Append a note to one of three files based on user choice

Question

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.

It took me four hours to write this. I'm trying to learn and was wondering if this could be improved or cleaned up at all.

#!/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

Answer 1

A couple of points:

1. You determine the date outside your loop, then loop forever. Every note you make after the first one is going to have the same date stamp.

   A better approach would be to collect the date stamp just as you are writing to the file:

   datestamp=$(...)
   echo "$datestamp: $note" >> ...
   

2. When shells were originally written, almost everything was a program. The if statement takes an executable as its conditional element. (If you look, you will find a program named [ in /bin so that if [ ... will work.)

   There is a program named true and a program named false. You can run them, and they don't do anything except set their exit codes to appropriate values.

   You don't need to write while [ true ]. Instead, just write while true. This is important because while false will do something different from while [ false ]. You may be surprised ...

3. As @Vogel612 points out, you have duplicated code. I think you should keep the user feedback close to the user entry, so your checking should happen before typing the note. You can use another variable to hold the destination file path:

   read -p "What is this note for?
   Work
   School
   Shopping 
   > " topic
   destfile=''
   case $topic in
    "Work" ) destfile="$wsave" ;;
    "School" ) destfile="$scsave" ;;
    "Shopping" ) destfile="$shsave" ;;
    * ) echo "Error: Selection was not on list, try again.\n" ;;
   esac
   if [ "$destfile" ]; then 
    read -p "\nNote\n> " note
    echo "$date: $note" >> "$destfile"
    echo "Note saved to $destfile"
   fi
   

• Ryan R: Awesome, thanks! I added a quit option using a break command under shopping. I added a -e to the echo commands becuase the newlines weren't coming through. Read that printf is a subsitute, but that didn't display the way I wanted to. Moved the date variable to right before the ehco statement that calls it.

Answer 2

Extract common code into a common block. You only need the case to determine the save-location. The note itself can be read before switching:

#...
read -p "What is this note for?
Work
School
Shopping
> " topic
read -p "
Note 
> " note
save=""
case $topic in
 "Work")
 save=$wsave
 break
 ;;
 "School")
 save=scsave
 break
 ;;
 "Shopping")
 save=$shsave
 break
 ;;
 *) echo "Error: Selection was not on list, try again.
"
 ;;
esac
if [[ $save!="" ]]; then
 echo "$date: $note" >> "$save"
 echo "Note saved to $save"
fi
#...

this removes the duplication in the case-blocks and still allows you to clearly work with what you expect for every note.

---

The variable names could use a bit more... characters in general. You could even use snake_case to differentiate between words and all that. This allows you to make the code significantly more speaking.

---

The comment #list is really not adding any value. #get the date is already clearly outlined by the code, maybe a more human readable format explanation might be useful there. #save locations should be completely replaceable with proper variable names.

---

A word on the save locations you are using. As it stands, these are totally visible and will clutter the user's home directory. You should consider making these hidden by default by prefixing the filenames with a ., maybe even put them into a separate folder in the home-directory.

• Ryan R: Thanks for the ideas! I would like it though if the note only saved after the user selected an option from the predefined list. With your code if the user misspelled a topic name there potentially long note wouldn't be saved, right? I agree the comments I make aren't very helpful. I made a separate directory for the notes.