-
Notifications
You must be signed in to change notification settings - Fork 47
Sockets - Carla #40
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
base: master
Are you sure you want to change the base?
Sockets - Carla #40
Conversation
CheezItMan
left a comment
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.
Your code definitely works, but things are nested very deeply and it could be written much cleaner.
consider
if array1.length != array2.length
return false
else
array1.each_with_index do |element, index|
return false if element != array2[index]
end
return true # must have gone through the entire list and found them all equal
endThere 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.
Tests pass! A few minor style fixes and you should be good to go. 🙂
(Sorry about the duplicate reviews, just realized Chris already covered this.)
| end | ||
|
|
||
| if array1.length == array2.length | ||
| loop_count = array1.length |
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.
Tiny style nit: is this extra variable necessary?
| loop_count.times do |i| | ||
| if array1[i] != array2[i] | ||
| return false | ||
| else |
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.
Style nit: do we need this else block? I have a feeling we can delete it, if we put a return true statement after it.
(returning exits the function - so any code after an executed return statement won't run.)
| return false | ||
| end | ||
|
|
||
| if array1.length == array2.length |
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.
Style nit: using a "guarding if" that checks if array1.length does not equal array2.length is probably more readable here. (If nothing else, more programmers are used to that pattern - so it's easier for your coworkers to recognize.)
No description provided.