Conversation
|
I think the nested "if" structure (an if statement within an if statement -lines 4,5 and an elseif -line 10, etc.) can become difficult to track with these many boolean expressions. I would avoid nested "if" structure if possible and end the if statement when needed. :) |
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.
Good code for your first assignment.
| def array_equals(array1, array2) | ||
| raise NotImplementedError | ||
| if array1 == nil || array2 == nil | ||
| if array1 == nil && array2 == nil |
There was a problem hiding this comment.
This whole block takes the form of:
if (condition) return true else return false end
Simplify to: return (condition)
The latter is more readable and easier to type.
| if array1[i] != array2[i] | ||
| return false | ||
| end | ||
| i += 1 |
There was a problem hiding this comment.
The array.length.times method replaces the value of i with whatever comes next, so this statement is redundant and has no real effect. Remove.
| if array1.length == 0 | ||
| return true | ||
| else | ||
| i = 0 |
| else | ||
| return false | ||
| end | ||
| end |
There was a problem hiding this comment.
General comment: blank lines are your friend. Add some whitespace between logical blocks of code.
3 participants
No description provided.