LinuxQuestions.org
Register a domain and help support LQ
Go Back   LinuxQuestions.org > Forums > Linux Forums > Linux - Newbie
User Name
Password
Linux - Newbie This Linux forum is for members that are new to Linux.
Just starting out and have a question? If it is not in the man pages or the how-to's this is the place!

Notices


Reply
  Search this Thread
Old 11-12-2015, 06:38 PM   #31
chrism01
LQ Guru
 
Registered: Aug 2004
Location: Sydney
Distribution: Centos 6.8, Centos 5.10
Posts: 17,260

Rep: Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328Reputation: 2328

@kjeska: a couple of minor tips;

1. do always use indentation in your code; much easier to read and debug for us & you.
2. for the 'if' test, use [[ ]] instead of [ ] see http://tldp.org/LDP/abs/html/testcon...ml#DBLBRACKETS
 
Old 11-12-2015, 07:39 PM   #32
kjeska
LQ Newbie
 
Registered: Nov 2015
Posts: 15

Original Poster
Rep: Reputation: Disabled
Quote:
Originally Posted by grail View Post
So I am not sure if you are handing these scripts in to be marked, but if you are, you will lose marks on the first task as it does not meet the brief:

At no time do you accept or test for any parameters at the command line, so if I were to run:
Code:
#!/bin/bash

echo "Choose a number between 1-9"
close() {
  for((i=1;i<=n;i++)) 
  do  
    for((k=i;k<=n;k++)) 
    do  
      echo -ne " ";
    done
    for((j=1;j<=i;j++))
    do  
      echo -e "$i \c"
    done  
    echo ""
  done
}
read n
if [[ n -le 9 && n -ge 1 ]]; then
  close
fi
And ran it with an argument:
Code:
$ ./pyramid.sh 5
Choose a number between 1-9
So instead of printing my pyramid up to 5 I get asked again what number I would like to choose.

As a learning exercise, here is a list of things I would mark you down on if grading the work:

1. Script has no help option in case the user does not know what choices there are when running it, ie. like the fact you can pass an argument at the command line

2. Command line arguments are ignored (so does not meet the brief)

3. Question to user and reading of the answer are so far apart that following the code is confusing

4. Users response is not tested at all to see if it is a number, ie. I can enter the letter 'a' and the code simply finishes

5. As with point 4, if the user enters a number outside the range the script simply finishes and you have no message to advise why (even though in a simple script like this it is obvious, it is a good practice to get into)

6. When testing for numbers, like in all your for loops, it is better to use (()) as you can then use standard arithmetic signs like <= or >=

7. 'close' as a function name doesn't seem to make much sense?? Maybe draw_pyramid or write_nums, but something more on what the function does so later when you look at it (maybe 5 years later) you don't have to read all the code to discover that the function does not 'close' anything

8. You go to the trouble of creating a function but then use global variables. Look up the reserved word local

9. No message to advise the script has completed successfully (not always required but might have given you extra marks )

10. No comments at all throughout code to advise what is happening (again thinking of looking at the code in the future)

On the positive side:

1. Code adequately tests number range

2. Correct data is shown once number provided

As a tip, maybe look into the printf command as it could replace the need for your first internal if (ie. where you print the spaces before printing the first number on the line)

You may wish to apply some of the above to your next task as well
Wow! Much appreciated. This will definitely help me further. Later on I will re-do the hole script and make it more advanced.
After reading some of your tips, im sure ill get a much better result at the end.

PS: Some of the tips are out of curriculum, so we don't really have to worry about very deep details. But the more you get to understand them
the better you get at the general scripting.

We are almost finished with the tasks.

Again, thanks to everyone.
 
Old 11-12-2015, 07:40 PM   #33
kjeska
LQ Newbie
 
Registered: Nov 2015
Posts: 15

Original Poster
Rep: Reputation: Disabled
Quote:
Originally Posted by chrism01 View Post
@kjeska: a couple of minor tips;

1. do always use indentation in your code; much easier to read and debug for us & you.
2. for the 'if' test, use [[ ]] instead of [ ] see http://tldp.org/LDP/abs/html/testcon...ml#DBLBRACKETS
Thanks! I'll keep that in mind for the future.
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off



Similar Threads
Thread Thread Starter Forum Replies Last Post
[SOLVED] How to make a diamond with progressing & rectracting numbers andrew.comly Programming 13 12-02-2014 11:55 AM
making a number pyramid imran042 Programming 20 06-16-2012 12:02 PM
gcc and linux pyramid board shabi Linux - Embedded & Single-board computer 3 06-24-2010 12:42 AM
How do i make a script that checks that my input is letters or numbers ? ministeren Programming 13 05-20-2010 10:36 AM
how do I make lpr always print page numbers fakie_flip Linux - Software 2 11-30-2007 08:03 PM


All times are GMT -5. The time now is 08:28 PM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration