Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions implement-shell-tools/ls/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ It must act the same as `ls` would, if run from the directory containing this RE
Matching any additional behaviours or flags are optional stretch goals.

We recommend you start off supporting just `-1`, then adding support for `-a`.

Run the Python implementation from this directory with `python ls.py -1 sample-files`
or `python ls.py -1 -a sample-files`. Python 3 is required.
27 changes: 27 additions & 0 deletions implement-shell-tools/ls/ls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""List directory entries, one per line."""

import argparse
from pathlib import Path
import sys


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("-1", action="store_true", help="print one entry per line")
parser.add_argument("-a", "--all", action="store_true", help="include hidden entries")
parser.add_argument("directory", nargs="?", default=".")
args = parser.parse_args()

try:
entries = sorted(Path(args.directory).iterdir(), key=lambda entry: entry.name)
for entry in entries:
if args.all or not entry.name.startswith("."):
print(entry.name)
Comment on lines +16 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In main, the entries variable is created only to be used immediately in the for loop:

  • Line 16: entries = sorted(Path(args.directory).iterdir(), key=lambda entry: entry.name)
  • Line 17: for entry in entries:

Because entries isn’t reused elsewhere, one question to consider is whether introducing this temporary name actually helps readability for you, or whether you’d find it just as clear to loop directly over the sorted iterator. When a variable is introduced and only used once right away, it can sometimes add a tiny bit of cognitive overhead for readers who then need to remember that entries is simply the sorted directory listing.

How do you feel about the clarity here? If you find that the name entries meaningfully improves understanding, it can be worth keeping. If not, you might explore iterating directly over the sorted result to reduce the number of moving parts in the function.

To "like" or "dislike" this comment, please follow this link

except OSError as error:
print(f"ls: {error}", file=sys.stderr)
return 1
return 0


if __name__ == "__main__":
sys.exit(main())
5 changes: 5 additions & 0 deletions implement-shell-tools/wc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ It must act the same as `wc` would, if run from the directory containing this RE
Matching any additional behaviours or flags are optional stretch goals.

We recommend you start off supporting no flags for one file, then add support for multiple files, then add support for the flags.

Run the Python implementation from this directory with `python wc.py sample-files/1.txt`
or `python wc.py -l sample-files/3.txt`. Multiple filenames and combined count
flags are supported. Expand wildcards in your shell, or pass filenames explicitly
if your shell does not expand them. Python 3 is required.
47 changes: 47 additions & 0 deletions implement-shell-tools/wc/wc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Count lines, words, and bytes in files."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In wc.py, all the naming and text around the counts suggest that the third value is a byte count, but the implementation actually computes a character count, which can differ from the byte count for UTF-8 text containing non-ASCII characters.

Concretely:

  • The module docstring says: "Count lines, words, and bytes in files." (line 1)
  • The -c flag’s help says: "print byte counts" (line 17).
  • count_file reads the file as bytes (line 9), decodes it as UTF-8, and then returns len(text) as the third value (lines 9–11). After decoding, len(text) is the number of Unicode codepoints in the string, not the number of bytes in the original file.

So as a reader (or user of this script), you’d believe -c and the third count represent raw bytes, but for multi-byte UTF-8 characters the number you get will actually be smaller than the real byte size. That mismatch between the names/descriptions and the actual value is what makes this confusing.

You might find it useful to ask yourself: if someone else tried to reuse count_file because they needed byte counts, what assumptions would they reasonably make based on the function name, the docstring, and the option help text? How could you adjust either the naming/documentation or the implementation so that the third count and the "byte" wording are in clear agreement?

To "like" or "dislike" this comment, please follow this link


import argparse
from pathlib import Path
import sys


def count_file(filename):
text = Path(filename).read_bytes().decode("utf-8")
return len(text.splitlines()), len(text.split()), len(text)


def main():
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("-l", action="store_true", help="print line counts")
parser.add_argument("-w", action="store_true", help="print word counts")
parser.add_argument("-c", action="store_true", help="print byte counts")
parser.add_argument("files", nargs="+")
args = parser.parse_args()
selected = [args.l, args.w, args.c]
if not any(selected):
selected = [True, True, True]

def print_counts(counts, label):
values = [str(value) for value, enabled in zip(counts, selected) if enabled]
print(" ".join([*values, label]))
Comment on lines +24 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Inside print_counts, the values list is created and then immediately used once in the print call:

  • Line 25: values = [str(value) for value, enabled in zip(counts, selected) if enabled]
  • Line 26: print(" ".join([*values, label]))

Since values isn’t reused, you might ask yourself whether naming this intermediate result makes the code easier for you (and others) to follow, or whether it could be inlined into the print call without losing clarity. Temporary variables that are only used once can sometimes obscure the main intent of a small helper like this, though they can also make complex expressions easier to read.

Would an inlined version still feel readable to you, or does having values as a named step make the logic clearer in your eyes? Thinking about that trade-off can help you decide when a one‑use temporary is pulling its weight.

To "like" or "dislike" this comment, please follow this link


totals = [0, 0, 0]
successful = 0
status = 0
for filename in args.files:
try:
counts = count_file(filename)
except (OSError, UnicodeError) as error:
print(f"wc: {filename}: {error}", file=sys.stderr)
status = 1
continue
print_counts(counts, filename)
totals = [total + value for total, value in zip(totals, counts)]
successful += 1
if successful > 1:
print_counts(totals, "total")
return status


if __name__ == "__main__":
sys.exit(main())
Loading