Skip to content

Conversation

@codeblahblah
Copy link

@jwo Had to @another_battleship = Battleship.new as I could not assert_equal on a Mock object.
Also refactored TestBattleship by adding a #setup.
Am I green enough to continue with the rest of the assignment?

@jwo
Copy link
Member

jwo commented Feb 11, 2014

Sure, looks great to me. (Though you don't need to ask my permission to continue)

@codeblahblah
Copy link
Author

Thanks. Will keep that in mind in future.

@codeblahblah
Copy link
Author

@jwo Add syntax to confirm that the Battleship can request more ammunition

@codeblahblah
Copy link
Author

@jwo I used a Module instead of a Class for "Fate".

minitest/navy.rb Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see you ever use the "FATE" constant. Is there a significance around it?

@codeblahblah
Copy link
Author

@jwo Refactored code.

@codeblahblah
Copy link
Author

@jwo Is a Module the correct way to go about the assignment?

@jwo
Copy link
Member

jwo commented Feb 13, 2014

It could be -- I don't dislike the approach.

On Thu, Feb 13, 2014 at 1:51 PM, drammopo notifications@github.com
wrote:

@jwo Is a Module the correct way to go about the assignment?

Reply to this email directly or view it on GitHub:
#7 (comment)

@codeblahblah
Copy link
Author

@jwo Any other way? Can I consider the assignment complete?

minitest/navy.rb Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem complete, thoughts?

@codeblahblah
Copy link
Author

@jwo Having never played battleship...I had to read up on the rules.
Changed from

  def has_a_ship?
    true
  end 

to

   def has_a_ship?
    @battleship.max_hits > 0
  end 

@jwo
Copy link
Member

jwo commented Feb 14, 2014

hmm, well you're not using has_a_ship as a counter. You're using it in a test:

def test_have_a_battleship
  assert_equal true, @another_admiral.has_a_ship?
end

@codeblahblah
Copy link
Author

@jwo I'm now using the @max_hits of the Battleship for the test.

@jwo
Copy link
Member

jwo commented Feb 14, 2014

OK, sounds good. nothing else I have to comment on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants