-
Notifications
You must be signed in to change notification settings - Fork 47
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
Ports - Jillianne #45
base: master
Are you sure you want to change the base?
Conversation
lib/array_equals.rb
Outdated
raise NotImplementedError | ||
|
||
def array_equals(array_a, array_b) | ||
if (array_a == [] && array_b == [] || array_a == nil && array_b == nil) |
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.
oh wow! I like how you used a compound conditional to group all the true and false edge cases together. VERY NICE. ✨✨✨
else | ||
array_a.length.times do |item| | ||
if array_b[item] != array_a[item] | ||
return false |
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 returns false as soon as it finds two chars that don't match. Brilliant! Have a 🍬
return false | ||
end | ||
end | ||
return true |
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.
AND..... SHE STICKS THE LANDING! Returns true if all chars match. Perf. 🔥
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 works and it's readable! Only thing that I would change (and this doesn't affect how the code runs at all) is that you don't have to say "if array_a == nil". Since nil is a falsey value, you could just say "if !array_a"
Other than that this is great and I can't find anything wrong with your logic.
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.
Hello Jillianne, my name is Phoebe (c9) and I have been assigned to review your PRs. Please let me know if you have any questions on my comments/feedback going forward.
This PR looks great. It is concise and straightforward. Good job!
No description provided.