Add the render method with specs and cleans up previous methods.#7
Add the render method with specs and cleans up previous methods.#7
Conversation
| end | ||
|
|
||
| def empty? | ||
| @ship.nil? |
There was a problem hiding this comment.
@ship.nil? does the same thing as
if @ship == nil
true
else
false
end
(its just a shorter way of writing it!)
|
|
||
| def fire_upon | ||
| @ship.hit | ||
| @fired_upon = true |
There was a problem hiding this comment.
Ok, I understand why you changed that and added @fired_upon = false to the cell's attributes. One way to clean up the syntax a bit would be to write:
@fired_upon = true
@ship.hit if !empty?
| else | ||
| "." | ||
| end | ||
| end |
There was a problem hiding this comment.
you can remove all of the "== true" and "== false" to clean up the syntax. Here is how:
if fired_upon? && empty?
"M"
elsif fired_upon? && @ship.sunk? && !empty?
"X"
elsif fired_upon? && !empty?
"H"
elsif !empty? && show_ship
"S"
else
"."
end
| def render(show_ship = false) | ||
| if fired_upon? == true && empty? == true | ||
| "M" | ||
| elsif fired_upon? == true && @ship.sunk? == true && empty? == false |
There was a problem hiding this comment.
isn't the empty? == false redundant? b/c for a cell to have a sunk ship on it, it cannot be empty
|
see the few comments I made above (mostly syntax things that will reduce the amount of code by quite a bit). The overall logic of the #render method looks good to me (although I could be wrong, and we will see as we move forward). Gonna go ahead and merge, and i'll make those few changes so we can keep moving |
2 participants
Added the #render method and specs. I needed to update some other methods for the conditional statement in the render method to work. We can refactor if if you have a more efficient way.