Append a note to one of three files based on user choiceMaking kexec reboots less painfulProcess files in all subdirectories and save output to new files based on their current pathText files: Copy, Rename, Append/Merge togetherSort and source configuration files from one of two directoriesCopy recently modified files from one directory to anotherOne to One network file transfer code C# socket basedGo: Generate color palettes for set of files in a directory based on a specific field property in each of themUser-friendly script for searching through log filesScript to clean up old files that should only run one instanceSSMTP configuration in Ubuntu 18.04 with Bash in my own system (one user)
2×2×2 rubik's cube corner is twisted!
Virginia employer terminated employee and wants signing bonus returned
BitNot does not flip bits in the way I expected
Could you please stop shuffling the deck and play already?
What wound would be of little consequence to a biped but terrible for a quadruped?
Unreachable code, but reachable with exception
Grey hair or white hair
Why is this plane circling around the Lucknow airport every day?
Do f-stop and exposure time perfectly cancel?
Why is there a voltage between the mains ground and my radiator?
Accountant/ lawyer will not return my call
Should QA ask requirements to developers?
Good allowance savings plan?
Why would a jet engine that runs at temps excess of 2000°C burn when it crashes?
The bar has been raised
Best approach to update all entries in a list that is paginated?
show this identity with trigometric
Why does the negative sign arise in this thermodynamic relation?
Replacing Windows 7 security updates with anti-virus?
infinitive telling the purpose
A question on the ultrafilter number
Should I take out a loan for a friend to invest on my behalf?
Should I tell my boss the work he did was worthless
How do I express some one as a black person?
Append a note to one of three files based on user choice
Making kexec reboots less painfulProcess files in all subdirectories and save output to new files based on their current pathText files: Copy, Rename, Append/Merge togetherSort and source configuration files from one of two directoriesCopy recently modified files from one directory to anotherOne to One network file transfer code C# socket basedGo: Generate color palettes for set of files in a directory based on a specific field property in each of themUser-friendly script for searching through log filesScript to clean up old files that should only run one instanceSSMTP configuration in Ubuntu 18.04 with Bash in my own system (one user)
$begingroup$
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
beginner bash linux shell
New contributor
$endgroup$
add a comment |
$begingroup$
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
beginner bash linux shell
New contributor
$endgroup$
add a comment |
$begingroup$
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
beginner bash linux shell
New contributor
$endgroup$
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
beginner bash linux shell
beginner bash linux shell
New contributor
New contributor
edited 8 hours ago
200_success
130k17153419
130k17153419
New contributor
asked 9 hours ago
Ryan RRyan R
162
162
New contributor
New contributor
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
A couple of points:
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" >> ...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 thatif [ ...
will work.)There is a program named
true
and a program namedfalse
. 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 writewhile true
. This is important becausewhile false
will do something different fromwhile [ false ]
. You may be surprised ...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 "nNoten> " note
echo "$date: $note" >> "$destfile"
echo "Note saved to $destfile"
fi
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Ryan R is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215297%2fappend-a-note-to-one-of-three-files-based-on-user-choice%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
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.
$endgroup$
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.
answered 8 hours ago
Vogel612♦Vogel612
21.8k447130
21.8k447130
add a comment |
add a comment |
$begingroup$
A couple of points:
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" >> ...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 thatif [ ...
will work.)There is a program named
true
and a program namedfalse
. 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 writewhile true
. This is important becausewhile false
will do something different fromwhile [ false ]
. You may be surprised ...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 "nNoten> " note
echo "$date: $note" >> "$destfile"
echo "Note saved to $destfile"
fi
$endgroup$
add a comment |
$begingroup$
A couple of points:
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" >> ...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 thatif [ ...
will work.)There is a program named
true
and a program namedfalse
. 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 writewhile true
. This is important becausewhile false
will do something different fromwhile [ false ]
. You may be surprised ...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 "nNoten> " note
echo "$date: $note" >> "$destfile"
echo "Note saved to $destfile"
fi
$endgroup$
add a comment |
$begingroup$
A couple of points:
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" >> ...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 thatif [ ...
will work.)There is a program named
true
and a program namedfalse
. 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 writewhile true
. This is important becausewhile false
will do something different fromwhile [ false ]
. You may be surprised ...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 "nNoten> " note
echo "$date: $note" >> "$destfile"
echo "Note saved to $destfile"
fi
$endgroup$
A couple of points:
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" >> ...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 thatif [ ...
will work.)There is a program named
true
and a program namedfalse
. 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 writewhile true
. This is important becausewhile false
will do something different fromwhile [ false ]
. You may be surprised ...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 "nNoten> " note
echo "$date: $note" >> "$destfile"
echo "Note saved to $destfile"
fi
answered 7 hours ago
Austin HastingsAustin Hastings
7,0921233
7,0921233
add a comment |
add a comment |
Ryan R is a new contributor. Be nice, and check out our Code of Conduct.
Ryan R is a new contributor. Be nice, and check out our Code of Conduct.
Ryan R is a new contributor. Be nice, and check out our Code of Conduct.
Ryan R is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215297%2fappend-a-note-to-one-of-three-files-based-on-user-choice%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown