0

I'm wondering if this could be improved or cleaned up at all. The idea is the user selects one of the three categories then makes a note on that selection. Then the note is appended and saved to the appropriate list.

#!/bin/bash

#get the date
date=$(date +%d-%B-%Y)

#save locations
wsave="${HOME}/worknotes.txt"
shsave="${HOME}/shoppingnotes.txt"
scsave="${HOME}/schoolnotes.txt"


#list
while [ true ]
do
read -p "What is this note for?
Work
School
Shopping 
> " topic
case $topic in

    "Work" )
        read -p "
Note
> " wnote
        echo "$date: $wnote" >> "$wsave"
        echo "Note saved to $wsave"
            break
            ;;
    "Shopping" )
        read -p "
Note
> " shnote
        echo "$date: $shnote" >> "$shsave"
        echo "Note saved to $shsave"
            break
            ;;
    "School" )
        read -p "
Note
> " scnote
        echo "$date: $scnote" >> "$scsave"
        echo "Note saved to $scsave"
            break
            ;;
    *) echo "Error: Selection was not on list, try again.
"
            ;;
esac
done
3
  • 6
    Requests for improvement of working code usually belong on codereview.stackexchange.com . You might get better suggestions there.
    – John1024
    Commented Mar 11, 2019 at 21:46
  • From reading the code I'd say the only thing that is actually affected by the user's choice is the filename the note gets appended to. That is one thing. But you have three statements in your case specific code...
    – Bananguin
    Commented Mar 11, 2019 at 23:21
  • Thanks for the suggestions. @John1024 I didn't know that was a thing, I'll check it out, thanks!
    – Ryan R
    Commented Mar 12, 2019 at 20:54

1 Answer 1

0

Yes, you can improve the code.

The select statement in bash provides a way of showing a menu, and it also provides a basic input loop.

#!/bin/bash

menu=(
    Work
    School
    Shopping
)

PS3='What is this note for? '

while true; do
    unset outfile

    select word in Exit "${menu[@]}"; do
        case $REPLY in
            1)
                echo 'Bye!'
                exit ;;
            [2-4])
                outfile="$HOME/${word,,}-notes.txt" ;;
            *)
                echo 'Invalid choice, try again' >&2
        esac

        if [ -n "$outfile" ]; then
            break
        fi
    done

    IFS= read -r -p 'Enter note: '
    printf '%s:\t%s\n' "$(date +%d-%B-%Y)" "$REPLY" >>"$outfile"

    printf 'Note added to "%s"\n' "$outfile"
done

Another change here is that I only ask for the actual note in one place. This makes the code easier to read and easier to maintain.

The case ... esac may obviously be replaced by a series of if ... then ... elif ... fi statements, if that feels better. The inner break statement breaks out of the select loop if a valid choice was made. This takes us to the outer loop which then asks the user for the text and saves it into the file before looping back to showing the menu. The Exit choice in the menu provides a way of exiting the script.

A minor note is that the

while [ true ]

in your code has the same effect as

while [ false ]

or, indeed as

while [ bumblebee ]

This is because when the [ ... ] contains a single word, that word is interpreted as a string. If the string is non-empty, the test is true.

In my code, I use

while true

which actually executes the true utility, which always returns with a true value.

2
  • Just a minor remark: $ type true: true is a shell builtin. Commented Mar 13, 2019 at 18:04
  • @Vlastimil Yep, true is a built in utility in bash and most other shells.
    – Kusalananda
    Commented Mar 13, 2019 at 18:07

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .