Conversation
|
What does the line (!array1 or !array2) do? |
|
the 3rd line returns false if it’s not empty, essentially |
eric-andeen
left a comment
eric-andeen
left a comment
There was a problem hiding this comment.
I'm Eric. I'll be your volunteer code reviewer. I've been a software developer for about a zillion years. I'll be applying most of the same professional standards to your code as I do in the day job. Some of my comments may seem pedantic, persnickety, or irrelevant to the kinds of exercises you're doing in class, but once you start your internship, you'll get the same kind of feedback I'm giving you here.
Nice clean code. Good job!
| def array_equals(array1, array2) | ||
| raise NotImplementedError | ||
| return true if array1 == nil and array2 == nil | ||
| return false if !array1 or !array2 |
There was a problem hiding this comment.
These two lines correctly handle all the cases where one array is nil, but there are two readability/maintainability problems.
First, you use (array1 == nil) on one line and then (!array1) on the next line to mean the same thing. Pick one and be consistent, at least within the method. (!array1) is fine for a small codebase where there are no team members who object to terse code. So, basically, not a real-world team. (array1 == nil) is much clearer.
Second, these two lines look like two independent checks for error conditions, but in reality the second line depends on the first. It's the cleanest way to write the code, but makes it easy for a future maintenance programmer to muck it up. Maybe add a comment.
3 participants
No description provided.