2

This is the programme I use.

clear
echo enter a string
read string
length=`echo -n $string | wc -c`
v=0
cons=0
d=0
s=0
len=$length
while [ $len -gt 0 ]
do
h=`echo $string | cut -c $len`
case $h in
[AaEeIiOoUu]) v=`expr $v + 1`
;;
[BbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz]) cons=`expr $cons + 1`
;;
[0-9]) d=`expr $d + 1`
;;
' ') s=`expr $s + 1 `
;;
esac
len=`expr $len - 1 `
done
spl=`expr $len - $v - $cons - $d - $s`
echo "vowels = $v"
echo "consonants = $cons"
echo "digits = $d"
echo "spaces = $s"
echo "special character = $spl"

The program counts all other types of characters except special characters. The output shows a minus value even if there are special characters in the input value. How can I modify the programme to make it display correct number of special characters in the input?

2
  • So you want us to fix bug for you? What have you tried? Commented Sep 12, 2013 at 5:17
  • 1
    echo $string will lose information if the string contains irregular spaces. Always use double quotes unless you specifically require splitting on whitespace. echo "$string". Commented Sep 12, 2013 at 8:06

2 Answers 2

6

While your script is technically (almost) correct, it is not very pretty (both visually and in terms of code beauty).

The reason your version is not working as expected @devnull has already pointed out, but there is another bug that I will explain further down.

Since you are using bash you could rewrite the whole thing in a more idiomatic, readable and shorter way.

Here is a rewritten version of your script (comments and explanations follow below):

#!/bin/bash

clear
IFS= read -p "enter a string: " string

for ((i = 0; i < ${#string}; i++)); do
        case "${string:$i:1}" in
                [AaEeIiOoUu]) ((vowels++)) ;;
                [[:alpha:]])  ((consonants++)) ;;
                [[:digit:]])  ((digits++)) ;;
                [[:space:]])  ((whitespace++)) ;;
                *)            ((others++)) ;;
        esac
done

echo "vowels           = ${vowels-0}"
echo "consonants       = ${consonants-0}"
echo "digits           = ${digits-0}"
echo "whitespace       = ${whitespace-0}"
echo "other characters = ${others-0}"

indentation

First off, you should always indent your code blocks (if constructs, loops, switch (case) statements, ...) for readability, e.g.

while [ $len -gt 0 ]; do
    do_stuff
done

read, whitespace and the prompt

Since you are using bash, read is capable of displaying a prompt for you - the extra echo is not needed. Furthermore, read strips off leading and trailing whitespace which results in an incorrect calculation unless you set IFS to the null string:

IFS= read -p "this is my prompt: " string

iterating over characters in a string

You can use parameter expansion to both get the length of the string as well as slice out one character at a time using a for loop, getting rid of the unnecessary cut and avoiding a subshell.

# ${#string} = length of $string
for ((i = 0; i < ${#string}; i++)); do
    c=${string:$i:1} # c is set to the character at position $i in string $string
done

character classes

First off, your consonants statement still includes Ii for the match. Technically it doesn't matter since you cannot fall-through from the vowels match, but if this is an assignment you probably want to remove it.

That being said, you could just use the short character class for readability:

[AaEeIiOoUu]) vowel_stuff     ;;
[a-zA-Z])     consonant_stuff ;; # vowels already matched above, so superfluous characters don't matter here

To make your life even easier, there are so called character classes which you can use instead, e.g.

[:digit:] = [0-9]
[:space:] = tabs, newline, form feed, carriage return, space

etc. Note that your current locale influences certain character classes.

the special characters case

Just use the default case in the switch statement to count them, then you can skip calculating those afterwards:

case ... in
    vowels)     handle_vowel ;;
    [...]
    *)          handle_other_character ;;
esac

defaults

Using parameter expansion you can also get rid of initializing your variables with 0, you can on-the-fly expand the variables to 0 if they are not set, i.e. they have not been incremented during the loop.

backticks

Unless you are writing code that has to be super-portable and has to work in all kinds of old shells, use the $() syntax in stead of ``.

arithmetic expressions

Same as above, unless you really need it you can use (( )) for arithmetic expressions, e.g.

a=5
((a = a + 10))  # or even ((a += 10))
# $a is now 15

Google and the Advanced Bash-Scripting Guide as well as the bash sections of Greg's Wiki are your friends.

Sign up to request clarification or add additional context in comments.

2 Comments

Great article! Ii :)
Your answer was very helpful in improving my programme.
5

You are using the wrong variable while calculating spl. Using len is clearly wrong as you're decrementing it within the loop and it becomes 0 when the loop is done.

Instead of saying:

spl=`expr $len - $v - $cons - $d - $s`

say:

spl=`expr $length - $v - $cons - $d - $s`

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.