Skip to content
18 changes: 16 additions & 2 deletions src/inverseRobot.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,25 @@
/*
* @param {object} robot
*
* @return {object}
* @return {object | null}
*/

function inverseRobot(robot) {

Choose a reason for hiding this comment

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

It's a good practice to add a function description using JSDoc comments. This helps other developers understand what the function does, its parameters, and its return value.

Choose a reason for hiding this comment

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

It's a good practice to add a brief comment describing what your function does. This will help other developers understand your code more easily.

Choose a reason for hiding this comment

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

Consider adding a check to ensure that the 'robot' parameter is indeed an object and not null or undefined. This can prevent potential runtime errors if the function is called with invalid arguments.

Choose a reason for hiding this comment

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

The function 'inverseRobot' is correctly named and describes its purpose well.

// write code here
const fixed = {};

Choose a reason for hiding this comment

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

You can call it just inversedRobot

Choose a reason for hiding this comment

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

You can call it just inversedRobot

const valuesSet = new Set();

for (const key in robot) {

Choose a reason for hiding this comment

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

It's recommended to use Object.keys() or Object.entries() to iterate over an object. This way, you don't have to worry about properties that come from the object's prototype.

Choose a reason for hiding this comment

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

Consider using Object.keys(robot) or Object.entries(robot) to iterate over the properties of the robot object. This approach avoids iterating over properties in the prototype chain and is generally considered a best practice when you only want to deal with the object's own properties.

const value = robot[key];

Choose a reason for hiding this comment

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

You don't need this variable. You can use robot[key] in the next line

Choose a reason for hiding this comment

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

You don't need this variable. You can use inversedRobot in the next lines

Choose a reason for hiding this comment

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

you are iterating through object in a for in loop, it is not the best way to go, especially if the object is large. Consider using Object.keys() or Object.values() to make the loop more efficient.


if (valuesSet.has(value)) {
return null;
}

fixed[value] = key;

Choose a reason for hiding this comment

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

Try to simplify your code, we don't need the valuesSet and the fixed object. We can check if the value already exists as a key in the fixed object using hasOwnProperty.

Choose a reason for hiding this comment

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

You are modifying the input object which is generally a bad practice. It is better to create a new object for the output.

valuesSet.add(value);
}

Choose a reason for hiding this comment

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

InversedRobot, use inversedRobot instead of robot.

Choose a reason for hiding this comment

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

try to avoid using nested for loops

Comment on lines +12 to +19

Choose a reason for hiding this comment

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

Consider using 'Object.hasOwnProperty()' to check if a property exists on an object. While using 'in' is not incorrect, 'hasOwnProperty()' does not check down the object's prototype chain, which can be safer for property existence checks.


return fixed;
}
Comment on lines 9 to 22

Choose a reason for hiding this comment

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

The function inverseRobot is doing more work than necessary. It first checks for duplicate values in the input object by comparing each value with every other value. This results in a time complexity of O(n^2), which is not efficient for large inputs. Then it creates the inverse object. These two steps can be combined into one, which would result in a time complexity of O(n).


module.exports = inverseRobot;

Choose a reason for hiding this comment

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

It's good practice to include a newline at the end of the file. Some tools and systems expect or require it for proper processing.