London | 25-ITP-SEP | Adnaan Abo | Sprint 2 | Coursework

[AdnaanA]

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

Completed Sprint-2 exercises covering JavaScript challenges, error fixing code interpretation and running test-cases with Jest.

[cjyuan]

What does this comment describe?

[AdnaanA]

The comments from line 3 -6 describe a function that looks inside an array and tells you whether a certain value is present.

[cjyuan]

What function would that be? Your function does not quite match what it describe:

• Its first parameter is not named arr.
• Its first parameter is not an array
• Its second parameter is not named value.

[AdnaanA]

you are absolutely right it was one of those rookie mistake from my side. I have re-written the function to match the JSDoc

[cjyuan]

• Is the arguments passed to contains an array?

• Object contains keys and values. Naming the 2nd parameter 'value' could be misleading.

[cjyuan]

The description in contains.test.js says,

/*
Implement a function called contains that checks an object contains a
particular property
E.g. contains({a: 1, b: 2}, 'a') // returns true
as the object contains a key of 'a'
E.g. contains({a: 1, b: 2}, 'c') // returns false
as the object doesn't contains a key of 'c'
*/

[AdnaanA]

No, we cannot assume the arguments passed to contains() are an array.

and i have also changed the parameter 'value' --> 'item'

[AdnaanA]

I have now redone the whole contains.js assignment.

[cjyuan]

This test cannot detect (for certainty) if a function can properly check if the first parameter is an array or not.
An implementation of contains() could also return false because the property "a" cannot be found in the array.

Can you address this shortcoming?

[AdnaanA]

updated the the test we now only return false for invalid types.

[cjyuan]

My point was, contains([], "a") will evaluate to false regardless of whether the function checks if its first parameter is an array or not. If you want to test if the function can reject array, you should write something like

contains(["A"], "0")

because "0" is a key of the array. If contains() does not reject array, then it would have returned true.

[AdnaanA]

After fixing the contains.js I re-wrote the test-cases

[cjyuan]

Almost done. One more question.

[cjyuan]

Consider the following two approaches for determining if an object contains a property:

  let obj = {}, propertyName = "toString";
  console.log( propertyName in obj );                // true
  console.log( Object.hasOwn(obj, propertyName) );   // false

Which of these approaches suits your needs better?
For more info, you can look up JS "in" operator vs Object.hasOwn.

[AdnaanA]

property in obj --> will return true for own properties and inherited properties. This will give false positives in my use-cases.

While:

Object.hasOwn(obj, property) --> will return true only for properties defined directly on the object

[cjyuan]

Could consider grouping these tests into one category.

[AdnaanA]

these have been grouped

// contains({ a: 1, b: 2 }, 'a') // returns true and contains({ a: 1, b: 2 }, 'c') // returns false
test("Checks if an object contains the specified property", () => {
const obj1 = { a: 1, b: 2 };
expect(contains(obj1, "a")).toBe(true); // The property "name" exists
expect(contains(obj1, "c")).toBe(false); // The property "c" does not exist
});

[cjyuan]

Changes look good, and responses were clear.

[cjyuan]

These two tests are quite similar.

It would be more useful to test samples with different characteristics. For examples

• set obj1 to {a : 'c', b: 2 } to ensure the function won't check the value of the properties.
• Use property names with different letter case to ensure the check is case sensitive.