Sprint-2/debug/recipe.js (1)

17-19: The iteration works, but what do you think for...in actually iterates over?

When you use for...in on an array, you're iterating over the indices (as strings: "0", "1", "2"...), not the values themselves — which is why you need recipe.ingredients[ingredientIndex] to get the actual item.

Have you explored for...of? It iterates directly over the values, which would let you write something like:

for (const ingredient of recipe.ingredients) {
  console.log(ingredient);
}

What advantages might this have for readability and reducing the chance of bugs?

.gitignore (1)

2-4: I notice there might be some redundancy here — what do you think?

Looking at lines 2 and 4, you have both .DS_Store and **/.DS_Store. The **/.DS_Store pattern is actually more comprehensive since it matches .DS_Store files in any subdirectory. Would you consider whether the simpler .DS_Store on line 2 is still needed, or could you consolidate these into just the glob pattern?

🧹 Optional cleanup

 node_modules
-.DS_Store
 .vscode
 **/.DS_Store
 prep

Sprint-2/implement/lookup.js (1)

3-5: Have you considered for...of here?

I notice you're using for...in to iterate over the array. While this works, for...in is typically designed for iterating over object properties, and when used on arrays it gives you the indices as strings rather than the elements directly.

How might the code look if you used for...of with destructuring instead? It could make the intent clearer and avoid the need for index-based access.

♻️ Suggested approach using for...of with destructuring

 function createLookup(array) {
   const countryCurrencyObject = {};
-  for (const index in array) {
-    countryCurrencyObject[array[index][0]] = array[index][1]
-  };
-  return countryCurrencyObject
-};
+  for (const [country, currency] of array) {
+    countryCurrencyObject[country] = currency;
+  }
+  return countryCurrencyObject;
+}

Sprint-2/implement/lookup.test.js (1)

12-31: What if we simplified these tests?

I see you're testing Object.keys() and Object.values() separately to verify the structure. This works, but it relies on object property order being preserved. Have you thought about whether a single test comparing the entire object might be clearer and more robust?

test("creates a lookup object from country-currency pairs", () => {
  expect(createLookup([["US", "USD"], ["CA", "CAD"]])).toEqual({
    US: "USD",
    CA: "CAD",
  });
});

Also, what edge cases might be worth considering? For example:

• What should happen with an empty array?
• What about duplicate country codes — should the last one win?

Sprint-2/implement/tally.test.js (1)

22-22: Quick thought on the describe block naming

The describe name reads like a requirement: "Given a function called tally, return an object containing the count for each unique item". Typically, describe blocks name the unit being tested (like "tally" or "tally function"), while the individual test names describe specific behaviors.

How might reorganizing this improve readability when test output is displayed?

🧹 Optional naming adjustment

-describe("Given a function called tally, return an object containing the count for each unique item", () => {
+describe("tally", () => {

Sprint-2/interpret/invert.js (1)

40-45: Consider a prototype‑less target object to avoid key collisions.

I’ve noticed you’re assigning arbitrary values as keys; if a value is "__proto__" or "constructor", it can collide with object prototypes. How could you make the target immune to that? Using Object.create(null) is a simple fix.

♻️ Proposed refactor

 function invert(obj) {
-  const invertedObj = {};
+  const invertedObj = Object.create(null);
 
   for (const [key, value] of Object.entries(obj)) {
     invertedObj[value] = key;
   }

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}