Birmingham | 25-ITP-Sept | Joy Opachavalit | Sprint 1 | Feature/destructuring #335

enjoy15 · 2025-11-29T14:36:43Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Exercise 1-3 updated

illicitonion

This is looking really good - I left a couple of comments to consider for how to make your code even more clean, but this is all looking great, well done!

illicitonion · 2025-12-10T18:35:20Z

Sprint-1/destructuring/exercise-3/exercise.js

 ];
+
+// Print the receipt header
+console.log("QTY     ITEM                TOTAL");


This works, but let's imagine we wanted to change the spacing between the columns. Right now I would need to change two bits of code - this line, and also line 23 where we say how much padding to add.

Can you think how to restructure this code so that I would only need to change one place if I wanted to change the spacing?

@illicitonion I define a variable for the column spacing and use it consistently in both places. This way, if I want to change the spacing, I nly need to update the variable.

illicitonion · 2025-12-10T18:37:39Z

Sprint-1/destructuring/exercise-3/exercise.js

+  );
+});
+
+// Log the total cost


These comments don't seem to be adding a lot of value to this code... Have a read of https://www.jackfranklin.co.uk/blog/code-comments-why-not-how/ and think if either these comments could be changed, or could be removed, or if there are any changes you could make to your code to make them unnecessary.

@illicitonion Comments reviewed and amended

I'm still not sure the comments are providing much value here.

console.log(\nTotal: ${totalCost.toFixed(2)}); is the last line of code. And from reading the code, it's quite clear what it's doing: printing (console.log) the total cost (totalCost), at the end (it's the last line).

Comment removed.

illicitonion · 2025-12-11T10:27:19Z

Sprint-1/destructuring/exercise-3/exercise.js

+const columnSpacing = 8;
+
+// Print the receipt header with aligned columns
+console.log("QTY".padEnd(columnSpacing) + "ITEM".padEnd(20) + "TOTAL");


You're still repeating the 20 across these lines :)

Also, rather than having two different lines of code where you need to refer to the same variable, can you think how a function could help you not even need that variable?

Function added.

illicitonion · 2025-12-11T10:28:40Z

Sprint-1/destructuring/exercise-3/exercise.js

+  );
+});
+
+// Log the total cost


I'm still not sure the comments are providing much value here.

console.log(\nTotal: ${totalCost.toFixed(2)}); is the last line of code. And from reading the code, it's quite clear what it's doing: printing (console.log) the total cost (totalCost), at the end (it's the last line).

illicitonion · 2025-12-12T11:31:50Z

Sprint-1/destructuring/exercise-3/exercise.js

+const COLUMN_SPACING = 8;
+const ITEM_COLUMN_WIDTH = 20;
+
+function formatColumn(content, width) {


This can be a helpful function on its own, but it doesn't solve the problem of repeating the constants at each call site.

But each line has exactly three columns with the same spacings - quantity, item, and total. So it feels like you could make a function formatColumns(quantity, item, total) which is responsible for the column widths, and then call that function twice?

Refactor column formatting function for receipt alignment

illicitonion

Looks good! But unfortunately your comment problem has come back, e.g. function formatColumns is clear enough about what it does - the comment "// Function to format multiple columns" doesn't add any extra information/value here. Similarly, the "Print the total cost at the end" comment came back.

enjoy15 · 2025-12-12T19:15:21Z

Redundant comments removed.

illicitonion · 2025-12-16T13:45:04Z

Sprint-1/destructuring/exercise-3/exercise.js

+  );
+}
+
+// Print the receipt header with aligned columns


I think there are still some redundant comments you could remove here :)

redundant comments removed

illicitonion

Looks great, well done!

enjoy15 added 3 commits November 29, 2025 14:10

excercise-1

59f3869

excercise-2

798da30

exercise-3

c586205

enjoy15 added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Flows The name of the module. labels Nov 29, 2025

illicitonion added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Dec 10, 2025

illicitonion reviewed Dec 10, 2025

View reviewed changes

illicitonion added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Dec 10, 2025

Refactor receipt printing for consistent formatting and alignment

3bb3554

enjoy15 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 10, 2025

illicitonion reviewed Dec 11, 2025

View reviewed changes

illicitonion removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 11, 2025

Add function for receipt formatting

579ae44

enjoy15 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 11, 2025

illicitonion reviewed Dec 12, 2025

View reviewed changes

illicitonion removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

Refactor column formatting function for receipt alignment

b172ead

enjoy15 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

illicitonion reviewed Dec 12, 2025

View reviewed changes

illicitonion removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

Remove redundant comments

fc413b7

enjoy15 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Dec 12, 2025

illicitonion reviewed Dec 16, 2025

View reviewed changes

Remove redundant comments

80f6444

illicitonion reviewed Dec 17, 2025

View reviewed changes

illicitonion closed this Jan 7, 2026

Uh oh!

Birmingham | 25-ITP-Sept | Joy Opachavalit | Sprint 1 | Feature/destructuring #335

Birmingham | 25-ITP-Sept | Joy Opachavalit | Sprint 1 | Feature/destructuring #335

Uh oh!

Conversation

enjoy15 commented Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Learners, PR Template

Changelist

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

enjoy15 commented Dec 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illicitonion left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

enjoy15 commented Nov 29, 2025 •

edited

Loading