Skip to content

Shadi A. - #1

Open
shmoonwalker wants to merge 9 commits into
HackYourAssignment:mainfrom
shmoonwalker:main
Open

shmoonwalker wants to merge 9 commits into
HackYourAssignment:mainfrom
shmoonwalker:main

Conversation

@shmoonwalker

Copy link
Copy Markdown

Task 1 and 2 for week 3 Completed .

@mvcatsifma mvcatsifma left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Solid implementation overall with correct enum pricing, clean Pizza/Order structure, a working generic Cache<T> implementation, and good use of streams and final fields.

Main improvement areas:

  • Package declarations do not match the directory structure (com/pizzacompany/... vs package pizzacompany...).
  • printReceipt() is missing the required receipt header and separator line from the specification.
  • CacheTests misses several important edge cases, especially overwrite behavior, removing missing keys, and validating size() after removal.
  • One test is redundant and there is a small typo (dobleCache).
  • Cache is package-private instead of explicitly public.

For future assignments:

  • Always verify package declarations match the actual directory structure.
  • Compare CLI/program output carefully against the exact specification.
  • Expand unit tests to cover edge cases and state transitions.
  • Remove redundant tests and keep naming/typos clean.
  • Prefer explicit visibility modifiers for public APIs/classes.

Good overall structure and implementation quality with a few important correctness/details issues remaining.

@mvcatsifma mvcatsifma added Reviewed This assignment has been reivewed by a mentor and a feedback has been provided and removed Review in progress labels May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed This assignment has been reivewed by a mentor and a feedback has been provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants