Skip to content

ITP_GLASGOW_APR | HANNA_MYKYTIUK | MODULE__DATA_\GROUPS | SPRINT_2 #492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
10 changes: 10 additions & 0 deletions Sprint-1/implement/max.test.js
Copy link

Choose a reason for hiding this comment

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

Please remove unrelated files when you request a code review.

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ We have set things up already so that this file can see your function from the o

const findMax = require("./max.js");

describe("findMax", () => {
[
{ input: [], expected: ['a','b','c'] },
{ input: [5, 1, 1, 2, 3, 2, 5, 8], expected: [5, 1, 2, 3, 8] },
{ input: [1, 2, 1], expected: [1, 2] },
].forEach(({ input, expected }) =>
it(`deduplicated array for [${input}]`, () => expect(findMax(input)).toEqual(expected))
);
});

// Given an empty array
// When passed to the max function
// Then it should return -Infinity
Expand Down
2 changes: 1 addition & 1 deletion Sprint-2/debug/address.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ const address = {
postcode: "XYZ 123",
};

console.log(`My house number is ${address[0]}`);
console.log(`My house number is ${address.houseNumber}`);
5 changes: 3 additions & 2 deletions Sprint-2/debug/author.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const author = {
alive: true,
};

for (const value of author) {
console.log(value);
for (const key in author) {
console.log(key, author[key]);
}

6 changes: 4 additions & 2 deletions Sprint-2/debug/recipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ const recipe = {
};

console.log(`${recipe.title} serves ${recipe.serves}
ingredients:
${recipe}`);
ingredients: `);
for (let i=0; i<recipe.ingredients.length; i++){
console.log(recipe.ingredients[i]);
}
10 changes: 9 additions & 1 deletion Sprint-2/implement/contains.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
function contains() {}
function contains(obj, property) {
try {
return Object.hasOwn(obj, property);
}
catch(e ){
console.log(e);
return false;
}
}

module.exports = contains;
52 changes: 34 additions & 18 deletions Sprint-2/implement/contains.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,39 @@ as the object doesn't contains a key of 'c'

// Acceptance criteria:

// Given a contains function
// When passed an object and a property name
// Then it should return true if the object contains the property, false otherwise
describe("contains", () => {
// Given a contains function
// When passed an object and a property name
// Then it should return true if the object contains the property, false otherwise
it("check if object has property", () => {
expect(contains({name: 'Hanna'}, 'name')).toEqual(true);
expect(contains({name: 'Hanna'}, 'surname')).toEqual(false);
});
// Given an empty object
// When passed to contains
// Then it should return false
it("if object is empty it should return false", () => {
expect(contains({}, 'name')).toEqual(false);
});
// Given an object with properties
// When passed to contains with an existing property name
// Then it should return true
it("if object has multiple properties and passed existing property name it should return true", () => {
expect(contains({property1: 'exist1', property2: 'exist2'}, 'property1')).toEqual(true);
});
// Given an object with properties
// When passed to contains with a non-existent property name
// Then it should return false
it("if object has multiple properties and passed non-existing property name it should return false", () => {
expect(contains({property1: 'exist1', property2: 'exist2'}, 'property3')).toEqual(false);
});
// Given invalid parameters like an array
// When passed to contains
// Then it should return false or throw an error
it("in case of invalid parametr function contains should throw an error", () => {
expect(contains( [1,2,3], 'property')).toEqual(false);
expect(contains( "test", 'property')).toEqual(false);
});
Comment on lines +45 to +48
Copy link

Choose a reason for hiding this comment

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

1st Issue:
Did you run this test script? I don't think your current implementation can pass this test.

Array is a kind of objects in JavaScript where its indexes are treated as key. So
contains([4, 4, 4], '0') or contains([4, 4, 4], 1) would return true.
Object.hasOwn("ABC", 1) would also return true.

If you want to reject any type of value that is not an object, you will need to explicitly write code to check. (You can try asking ChatGPT how to properly check if a value is an object in JS)

2nd Issue:
Your function is not throwing any error. If you modify your function to throw error, you would need to prepare the test code in the following manner:

    it("in case of invalid parameter function contains should throw an error", () => {
      // Need to wrap the function call in a function
      expect( () => contains([1,2,3], 'property') ).toThrow();
      expect( () => contains("test", 'property') ).toThrow();
    });  

});

// Given an empty object
// When passed to contains
// Then it should return false
test.todo("contains on empty object returns false");

// Given an object with properties
// When passed to contains with an existing property name
// Then it should return true

// Given an object with properties
// When passed to contains with a non-existent property name
// Then it should return false

// Given invalid parameters like an array
// When passed to contains
// Then it should return false or throw an error
9 changes: 7 additions & 2 deletions Sprint-2/implement/lookup.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
function createLookup() {
// implementation here
function createLookup(complexArray) {
let object ={};
for (let i=0; i<complexArray.length; i++){
object[complexArray[i][0]] = complexArray[i][1];
}
return object;

}

module.exports = createLookup;
4 changes: 3 additions & 1 deletion Sprint-2/implement/lookup.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
const createLookup = require("./lookup.js");

test.todo("creates a country currency code lookup for multiple codes");
it("function recieve array of country code and currency code pairs should return an appropriate object", () => {
expect(createLookup([['US', 'USD'], ['CA', 'CAD']])).toEqual({'US': 'USD', 'CA': 'CAD'});
});

/*

Expand Down
14 changes: 11 additions & 3 deletions Sprint-2/implement/querystring.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,19 @@ function parseQueryString(queryString) {
const keyValuePairs = queryString.split("&");

for (const pair of keyValuePairs) {
const [key, value] = pair.split("=");
queryParams[key] = value;
//console.log(pair.split("="));
const array = pair.split("=");
let value = '';
for(let i = 1; i<array.length; i++){
value += array[i];
if (i != array.length - 1){
value += '=';
}
}
Comment on lines +11 to +17
Copy link

Choose a reason for hiding this comment

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

You can also write this as array.slice(1).join("="). Return an array without the first element and then join the elements using '='.

queryParams[array[0]] = value;
Copy link

Choose a reason for hiding this comment

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

Please note that in real querystring, both key and value are percent-encoded or URL encoded in the URL. For example, the string "5%" will be encoded as "5%25". So to get the actual value of "5%25" (whether it is a key or value in the querystring), you should call a function to decode it.
May I suggest looking up any of these terms, and "How to decode URL encoded string in JS"?

}

return queryParams;
}
console.log(parseQueryString("equation=x=y+1"));

module.exports = parseQueryString;
7 changes: 7 additions & 0 deletions Sprint-2/implement/querystring.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,10 @@ test("parses querystring values containing =", () => {
"equation": "x=y+1",
});
});

test("parses querystring values containing = and &", () => {
expect(parseQueryString("equation=x=y+1&something=something")).toEqual({
"equation": "x=y+1",
"something": "something"
});
});
30 changes: 29 additions & 1 deletion Sprint-2/implement/tally.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
function tally() {}
function tally(array) {
try {
if (typeof(array) == 'string'){
Copy link

Choose a reason for hiding this comment

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

There is a way to check if a value is an array. (Ask ChatGPT)
You can just return false or throw an error if array is NOT an array, instead of just check for "string". There are other types of value that is not array.

console.log('String is invalid input');
return false;
}
let copy = [];
for (let i=0; i<array.length; i++){
if (copy.indexOf(array[i]) === -1) {
copy.push(array[i]);
}
}
let object = {};
for (let i=0; i < copy.length; i++){
let count = 0;
for (let j=0; j < array.length; j++){
if (array[j] == copy[i]){
count = count + 1;
}
}
object[copy[i]] = count;
}
Comment on lines +7 to +22
Copy link

Choose a reason for hiding this comment

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

Would you try implement the following approach (which is more efficient by using object as a lookup table):

  let object = {};
  for each element, ele, in the arrays
    if object[ele] is undefined
      set object[ele] to 1
    else 
      increment object[ele] by 1

return object;
} catch(e){
console.log(e);
return false;
}

}

module.exports = tally;
16 changes: 15 additions & 1 deletion Sprint-2/implement/tally.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,30 @@ const tally = require("./tally.js");
// Given a function called tally
// When passed an array of items
// Then it should return an object containing the count for each unique item
it("it should return an object containing the count for each unique item", () => {
expect(tally(['a'])).toEqual({ a: 1 });
expect(tally(['a', 'a', 'a'])).toEqual({ a: 3});
expect(tally(['a', 'a', 'b', 'c'])).toEqual({ a : 2, b: 1, c: 1 });
});

// Given an empty array
// When passed to tally
// Then it should return an empty object
test.todo("tally on an empty array returns an empty object");
it("empty array should return an empty object", () => {
expect(tally(['a'])).toEqual({ a: 1 });
});

// Given an array with duplicate items
// When passed to tally
// Then it should return counts for each unique item
it("array with duplicate items should return an object containing the count for each unique item", () => {
expect(tally(['a', 'a', 'b', 'b', 'c', 'c'])).toEqual({ a : 2, b: 2, c: 2 });
});


// Given an invalid input like a string
// When passed to tally
// Then it should throw an error
it("Given an invalid input it should throw an error", () => {
expect(tally('string')).toEqual(false);
});
Comment on lines +46 to +48
Copy link

Choose a reason for hiding this comment

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

Your function is not throwing any error when the parameter is not an array (the description of the test is not quite correct).

11 changes: 9 additions & 2 deletions Sprint-2/interpret/invert.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,27 @@ function invert(obj) {
const invertedObj = {};

for (const [key, value] of Object.entries(obj)) {
invertedObj.key = value;
invertedObj[value] = key;
//invertedObj.key = value;
}

return invertedObj;
}

module.exports = invert;

// a) What is the current return value when invert is called with { a : 1 }
//current return value is: { key: 1 }

// b) What is the current return value when invert is called with { a: 1, b: 2 }
// current return value is: { key: 2 }

// c) What is the target return value when invert is called with {a : 1, b: 2}
// target output value is: { '1': 'a', '2': 'b' }

// c) What does Object.entries return? Why is it needed in this program?
// It returns an array of key/values of object. It is needed because in order to invert key and values we have to know them.

// d) Explain why the current return value is different from the target output
// because .key create one object paramenter with name 'key' and assign to it value(each of values shoud be a key)

// e) Fix the implementation of invert (and write tests to prove it's fixed!)
6 changes: 6 additions & 0 deletions Sprint-2/interpret/invert.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
const invert = require("./invert.js");

it("it should return an object with reverce key and values", () => {
expect(invert({ a : 1 })).toEqual({ '1': 'a' });
expect(invert({ a: 1, b: 2 })).toEqual({ '1': 'a', '2': 'b' });
});
Loading