-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
added initial solution #2676
base: master
Are you sure you want to change the base?
added initial solution #2676
Changes from 12 commits
efa2ecd
a946d53
6831bb9
0c9c947
b67f7e2
66f7d93
412eb24
2c949bd
07d32d3
727503c
a44140d
6320c61
b589d37
408ab78
fc864ed
297f3ed
c1b5609
0d59f06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,27 @@ | |
|
||
function inverseRobot(robot) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need |
||
let tempKyesRobot = {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable name 'tempKyesRobot' seems to be a typo. Consider renaming it to 'tempKeysRobot' for better readability. |
||
let tempValueRobot = {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable name 'tempValueRobot' could be more descriptive. Consider renaming it to 'robotValues'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable names 'tempKyesRobot' and 'tempValueRobot' contain typos and are not descriptive of their purpose. Consider renaming them to 'robotKeys' and 'robotValues' respectively for clarity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable names |
||
|
||
tempKyesRobot = Object.keys(robot); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to declare 'tempKyesRobot' as an empty object and then assign it with 'Object.keys(robot)'. You can directly assign it like this: 'let tempKeysRobot = Object.keys(robot);'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to initialize |
||
tempValueRobot = Object.values(robot); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, you can directly assign 'tempValueRobot' with 'Object.values(robot)'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to initialize 'tempKyesRobot' and 'tempValueRobot' with empty objects on lines 11 and 12 if you are going to assign them arrays on lines 14 and 15. You can declare and assign them in one step. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, there is no need to initialize |
||
|
||
for (let i = 0; i < tempValueRobot.length; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check can be simplified. Use |
||
if (tempValueRobot.indexOf(tempValueRobot[i]) !== i) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'indexOf' method has a time complexity of O(n), which makes your code less efficient. Consider using a 'Set' to check for duplicate values, as it has a time complexity of O(1). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
return null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check for repeated values can be optimized. Instead of using 'indexOf', which has O(n) complexity for each check, consider using an object to track occurrences of values. This will allow you to check for duplicates in O(1) time. |
||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. InversedRobot, use |
||
|
||
for (const key in robot) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using a more descriptive variable name instead of 'key'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modifying the input object |
||
delete robot[key]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deleting properties from the original 'robot' object is not a good practice. It's better to create a new object and return it. This way, the original object remains unchanged. |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try to avoid using nested for loops There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modifying the input object 'robot' is not a good practice as it can lead to side effects. Instead, create a new object and return it without altering the original 'robot' object. |
||
|
||
for (const i in tempKyesRobot) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try to avoid using nested for loops |
||
robot[tempValueRobot[i]] = tempKyesRobot[i]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line of code is not clear. It's better to use meaningful variable names instead of 'i'. Also, consider using a single loop to check for duplicates and create the new object at the same time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable 'i' in the for...in loop is misleading because it suggests an index, but it's actually a key. Use a more descriptive variable name, such as 'keyIndex' or 'key'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This loop is unnecessary. You can invert the key-value pairs in the same loop where you check for duplicates to make the function more efficient. |
||
} | ||
|
||
return robot; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning the modified input object 'robot' does not align with the task description, which asks for a new object to be returned. Create and return a new object instead. |
||
} | ||
|
||
module.exports = inverseRobot; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
There was a problem hiding this comment.
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.