Append a note to one of three files based on user choiceMaking kexec reboots less painfulProcess files in all...
Why does Deadpool say "You're welcome, Canada," after shooting Ryan Reynolds in the end credits?
How to pass a string to a command that expects a file?
A three room house but a three headED dog
How do I express some one as a black person?
How to create a hard link to an inode (ext4)?
PTIJ: How can I halachically kill a vampire?
What Happens when Passenger Refuses to Fly Boeing 737 Max?
Good allowance savings plan?
Accountant/ lawyer will not return my call
Is it true that real estate prices mainly go up?
Is there any way to damage Intellect Devourer(s) when already within a creature's skull?
Does "variables should live in the smallest scope as possible" include the case "variables should not exist if possible"?
How do I deal with a powergamer in a game full of beginners in a school club?
Can someone explain what is being said here in color publishing in the American Mathematical Monthly?
What does a stand alone "T" index value do?
Good for you! in Russian
Should I take out a loan for a friend to invest on my behalf?
Are babies of evil humanoid species inherently evil?
Is having access to past exams cheating and, if yes, could it be proven just by a good grade?
Who deserves to be first and second author? PhD student who collected data, research associate who wrote the paper or supervisor?
Why the color red for the Republican Party
How much stiffer are 23c tires over 28c?
Force user to remove USB token
Are the terms "stab" and "staccato" synonyms?
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 5 hours ago
200_success
130k17153419
130k17153419
New contributor
asked 5 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 4 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 3 hours ago
Austin HastingsAustin Hastings
7,0821233
7,0821233
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