Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

London| Elhadj Abdoul Diallo| Module-Tools | WEEK1 - Individual-shell-tools #16

Open
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

eediallo
Copy link

@eediallo eediallo commented Feb 28, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

[Elhadj Abdoul Diallo] added 30 commits February 26, 2025 10:19
…es in helper-files directory in script-04-stretch.sh
… files in helper-files directory in script-03.sh
…dless of case) from dialogue.txt in script-02.sh
…nsensitive) in dialogue.txt in script-03.sh
…se insensitive) from dialogue.txt in script-04.sh
…preceding line from dialogue.txt in script-05.sh
@eediallo eediallo added the Needs Review Participant to add when requesting review label Feb 28, 2025
@eediallo eediallo changed the title London| Elhadj Abdoul Diallo| Module-Tools | WEEK1 - Individual shell tools London| Elhadj Abdoul Diallo| Module-Tools | WEEK1 - Individual-shell-tools Feb 28, 2025
Copy link

@mjpeet mjpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're doing great with your scripts! Keep experimenting and practicing. Every script you write improves your skills. Don't hesitate to ask for help and keep up the fantastic work!

@@ -5,3 +5,5 @@ set -euo pipefail
# TODO: Write a command to output just the names of each player along with the score from their first attempt.
# Your output should contain 6 lines, each with one word and one number on it.
# The first line should be "Ahmed 1".

awk '{print $1, $3}' scores-table.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would your script look like if you used printf instead of print? what would be the advantage (if any) to use printf?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question! I've never tried printf before with awk but I guess it would allow me to format the result but as the content of the file is alreay in text format print is enough in this case

# The first line should be "Ahmed 4".
# Output the names of each player in London along with the score from their last attempt.
# For lines with fewer than 5 columns, print the last column.
awk '{if (NF >= 5 && $2 == "London") print $1, $5; else if (NF < 5 && $2 == "London") print $1, $NF}' scores-table.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, very nice work! (especially, because if you look at the current the latest changes for this in the repo, the condition for "For lines with fewer than 5 columns, print the last column." was taken out)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right!

@@ -7,3 +7,4 @@ set -euo pipefail
# TODO: Write a command to output just the names of each player along with the total of adding all of that player's scores.
# Your output should contain 6 lines, each with one word and one number on it.
# The first line should be "Ahmed 15". The second line should be "Basia 37"
awk '{ for(i = 3; i <= NF; i++) sum += $i; print $1, sum; sum = 0 }' scores-table.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i noticed you set initial value for sum at the end of the block - would it be better to initialise sum in another way? (regardless, your solution does print out the correct solution!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I could have done it at the biginning like this:

awk '{ sum= 0; for(i = 3; i <= NF; i++) sum += $i; print $1, sum}' scores-table.txt

@@ -9,3 +9,4 @@ set -euo pipefail
# 1 It looked delicious.
# 2 I was tempted to take a bite of it.
# 3 But this seemed like a bad idea...
cat -n ../helper-files/helper-3.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would this script look like if used the | (pipe) ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would have piped it to the nl command like this:

cat  ../helper-files/helper-3.txt | nl

@@ -4,3 +4,4 @@ set -euo pipefail

# TODO: Write a command to output every line in dialogue.txt that contains the word Doctor (regardless of case).
# The output should contain 9 lines.
grep doctor -i dialogue.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you look at the grep command man page, where are the grep options "positioned" compared to the parameters? (your solution works, but there may be commands where the order of the options and input(s) matters)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, going to fix it by following the correct order: grep [OPTION...] PATTERNS [FILE...]

@@ -4,3 +4,4 @@ set -euo pipefail

# TODO: Write a command to output, for each `.txt` file in this directory, how many lines of dialogue the Doctor has.
# The output should show that dialogue.txt contains 6 lines, dialogue-2.txt contains 2, and dialogue-3.txt contains 0.
grep ^Doctor -icH *.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you need to use the minimum of the options, how would your solution look like? (right now you're using 3)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove the -H option and the result will still look the same

@@ -5,3 +5,5 @@ set -euo pipefail
# TODO: Write a command which _recursively_ lists all of the files and folders in this directory _and_ all of the files inside those folders.
# The output should be a list of names including: child-directory, script-01.sh, helper-1.txt (and more).
# The formatting of the output doesn't matter.
cd ../ls
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you consider where this script (script-03.sh) is located, and where it would be executed, do you think this directory change is necessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not neccessary in this case. going to remove it.

@@ -16,8 +16,12 @@ echo "First exercise (sorted newest to oldest):"
# TODO: Write a command which lists the files in the child-directory directory, one per line, sorted so that the most recently modified file is first.
# The output should be a list of names in this order, one per line: helper-3.txt, helper-1.txt, helper-2.txt.

cd ../ls/child-directory
ls -1t
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you modify this script if using the command cd is not an option? (do you "need" to use cd ?)

Copy link
Author

@eediallo eediallo Mar 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make it work without cd-ing

@@ -5,3 +5,4 @@ set -euo pipefail
# TODO: Write a command to output input.txt with all occurrences of the letter `i` replaced with `I`.
# The output should contain 11 lines.
# The first line of the output should be: "ThIs Is a sample fIle for experImentIng with sed.".
sed s/i/I/g input.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would the option -e mean for this script?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it specifies multiple editing commands in a single sed invocation. But is not needed in this case because there is only one command being executed.

@@ -6,3 +6,4 @@ set -euo pipefail
# If a line starts with a number and a space, make the line instead end with a space and the number.
# So line 6 which currently reads "37 Alisha" should instead read "Alisha 37".
# The output should contain 11 lines.
sed -E 's/^([0-9]+) (.*)$/\2 \1/' input.txt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(optional) how would you change this script if you need to use basic regular expressions? (congrats on using ERE ! )

@mjpeet mjpeet added Reviewed Volunteer to add when completing a review Complete Volunteer to add when work is complete and review comments have been addressed 📅 Sprint 1 Assigned during Sprint 1 of this module and removed Needs Review Participant to add when requesting review labels Mar 2, 2025
Copy link

@mjpeet mjpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for getting back on the comments!

@eediallo
Copy link
Author

eediallo commented Mar 3, 2025

Thank you @mjpeet for the review and constructive feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and review comments have been addressed Reviewed Volunteer to add when completing a review 📅 Sprint 1 Assigned during Sprint 1 of this module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants