5
\$\begingroup\$

I have to print the number of stars that increases on each line from the minimum number until it reaches the maximum number, and then decreases until it goes back to the minimum number. After printing out the lines of stars, it should also print the total number of stars printed.

I have tried using shell scripting and worked. But is there any other simplified and generic way to achieve this. Or to be precise any better code to achieve this

echo "enter the mininum number of stars"
read min
echo "enter the maximum number of stars"
read max
for (( i=$min;i<=$max;i++))
do
   for (( j=$max;j>=i;j-- ))
   do
   echo -n " "
   done
   for (( c=1;c<=i;c++ ))
   do
   echo -n " *"
   sum=`expr $sum + 1`
   done
echo ""
done
d_max=`expr $max - 1`
for (( i=$d_max;i>=$min;i--))
do
   for (( j=i;j<=$d_max;j++ ))
   do
   if [ $j -eq $d_max ]
   then
   echo -n " "
   fi
   echo -n " "
   done
   for (( c=1;c<=i;c++ ))
   do
   echo -n " *"
   sum=`expr $sum + 1`
   done
echo ""
done
echo "Total No. of stars : "  $sum 

Output:

      *
     * *
    * * *
   * * * *
  * * * * *
   * * * *
    * * *
     * *
      *
\$\endgroup\$
4
\$\begingroup\$

Shell programs should start with a shebang, so that when executed, the OS can select the correct interpreter:

#!/bin/bash

With Bash, we can read prompted input using read -p:

read -p "Enter the mininum number of stars: " min

We should check that we have a valid positive integer before using $min.

The same comments also apply to $max. For both variables, consider accepting arguments instead of prompting for interactive input.

Prefer printf to echo -n - that's a good practice for portable scripting.

Most of those for loops can be eliminated with suitable printf formatting. We can produce n stars with printf '%.${n}s' '' | tr ' ' '*', and we can put that into a space that's m wide, with printf '%${m}s' "$(printf '%.$ns' '' | tr ' ' '*')". If we did keep the loops, we don't need $ for variable expansion within the arithmetic context (( )).

It seems strange to use arithmetic evaluation but also to invoke expr to increment $sum - it's more consistent to simply write ((++sum)).

The echo command with a single empty argument is equivalent to echo with no arguments, so no need to write "" there.

\$\endgroup\$
3
\$\begingroup\$

My Bash complains about sum=`expr $sum + 1`, and it ain't pretty:

  *expr: syntax error

What to make of that? Good ol' Bash won't treat you nicely if you don't treat it nicely! If I use set -u, things become clearer:

  *script.sh: line 13: sum: unbound variable

Aha! Don't forget to initialize variables, sum=0 in this case.


Speaking of sum=`...`, use sum=$(...) in the future, it's better in every way.


In arithmetic context you don't always need to use $ for variables. Instead of this:

for (( i=$min;i<=$max;i++))

You could write:

for ((i = min; i <= max; i++))

Notice that I adjusted the spacing around operators, to follow common conventions of many languages.

Also, I suggest to indent the loop body between do and done always, to make it easier to see the commands that are part of the loop. And it's common practice to indent by 4 spaces, not by 3.


Instead of printing fragments of a line of text, it would be better to build the content of each line in a variable, and then echo the line. For example like this:

for ((i = min; i <= max; i++)); do
    line=
    for ((j = i; j <= max; j++)); do
        line+=' '
    done
    for ((j = 1; j <= i; j++)); do
        line+=' *'
        ((sum++))
    done
    echo "$line"
done

Btw, all this is a really silly use of Bash :-)

\$\endgroup\$
2
\$\begingroup\$

First I will propose only some simple changes to make the code more readable.
1. add #!/bin/bash as first line then it is guaranteed that bash is used
2. use consistent indentation
3. when you do arithmetic evaluation you don't need the $ character to evaluate the variables,
4. use consistent spacing to structure expressions, e.g. (( j=max; j>=i; j-- ))
5. instead of expr use $((...))
this is more readable but has other advantages, too.
6. echo "" hase the same effect as echo, so use the latter, it is more clear
7. echo "Total No. of stars :" $sum is the same as echo "Total No. of stars : $sum", but the latter is more clear, so use the latter.
8. Initialize variables. They may be set outside the script to a wrong value.

#!/bin/bash
echo "enter the mininum number of stars"
read min
echo "enter the maximum number of stars"
read max
sum=0
for (( i=min; i<=max; i++ )); do
    for (( j=max; j>=i; j-- )); do
        echo -n " "
        done
    for (( c=1; c<=i; c++ )); do
        echo -n " *"
        sum=$(( sum+1 ))
        done
    echo
    done
d_max=$(( max-1 ))
for (( i=d_max; i>=min; i-- )); do
    for (( j=i; j<=d_max; j++ )); do
        if (( j==d_max )); then
            echo -n " "
            fi
        echo -n " "
        done
    for (( c=1; c<=i; c++ )); do
        echo -n " *"
        sum=$(( sum+1 ))
        done
    echo 
    done
echo "Total No. of stars : $sum"

This loop is executed d_max-i+1 mal, and d_max-i+2 times " " is printed.

for (( j=i; j<=d_max; j++ )); do
    if (( j==d_max )); then
        echo -n " "
        fi
    echo -n " "
    done

So use the simpler loop to print d_max-i+2 times " ".

for (( j=i; j<=d_max+1; j++ )); do
    echo -n " "
    done

Instead of

for (( c=1; c<=i; c++ )); do
    echo -n " *"
    sum=$(( sum+1 ))
    done

use the simpler

for (( c=1; c<=i; c++ )); do
    echo -n " *"
    done
sum=$(( sum+i ))

You have 4 times a similar block of code. Is it possible to replace it by a function? Yes!

print_from_to(){
    str=$1
    from=$2
    to=$3
    for (( t=$from; t<=$to; t++ )); do
        echo -n $str
        done
}       

So we have the following code now

#!/bin/bash

print_from_to(){
    str=$1
    from=$2
    to=$3
    for (( t=$from; t<=$to; t++ )); do
        echo -n $str
        done
}       

echo "enter the mininum number of stars"
read min
echo "enter the maximum number of stars"
read max
sum=0
for (( i=min; i<=max; i++ )); do
    print_from_to " " $i $max
    print_from_to  " *" 1 $i 
    sum=$(( sum+i ))
    echo
    done
d_max=$(( max-1 ))
for (( i=d_max; i>=min; i-- )); do
    print_from_to " " $i $((d_max+1))
    print_from_to " *" 1 $i 
    sum=$(( sum+i ))
    echo 
    done
echo "Total No. of stars : $sum"

All in all you do a lot of echo commands, I think

$$2\cdot max\cdot (max-min+1).$$

You can simplify your program. Note that for the first half of your lines the next line to be print can be constructed out of the previous line by removing ' ' from the beginning and adding ' *' at the end.

line=${line# }   # remove leading blank
line="$line *"   # add blank and asterisk at the end

The second half of the lines can be constructed by adding a ' ' at the beginning and removing ' *' from the end.

line=" $line"     # add blank at the beginning
line=${line% *}   # remove trailing blank and asterisk 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.