• Post Reply Bookmark Topic Watch Topic
  • New Topic

Test doesn't stop running  RSS feed

 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello. My name is Dillon and this is my first ever post here at CodeRanch.
I am learning Java and I decided to build a Battleship game to practice and hopefully improve my skills. What I am trying to do is something like the "DotCom" game in "Head First Java" and as I didnt quite understand the entire code in the book, Im building my own version of the game.
Basically I want to place 3 ships (or more) made of 3 cells in a grid and have the game played. I am having a problem testing the Ship class when creating more than one Ship. When I click run(I am using Eclipse) it just keeps running and never ends. It seems that there's an infinite loop somewhere but I think(might be wrong) I got that covered with the while loops.
If someone cares to take a look at my "problem" I would apreciate.
Im sorry if my post has something wrong as Im a noob at posting stuff at forums.
Thank you and have a good day/evening.



 
Stevens Miller
Bartender
Posts: 1444
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch, Dillon.

I haven't tried to make sense of your entire program, but I bet part of your problem is at Lines 68 and 69, and Lines 72 and 73 in your first block of code, above. The scope (read that, "range of effect") of the "(int)" cast is smaller than I believe you think it is. Look at those lines carefully and see if you can figure out what they are doing.
 
Campbell Ritchie
Marshal
Posts: 55774
163
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome again. Stevens is correct; there is a long discussion about randoms here, where people have all sorts of misconceptions about random numbers. I recommend you create an instance of the Random class and use its nextInt method. I would also suggest you use an array instead of the switch:-If you use the right numbers you can be sure of not getiing an out of bounds exception.
You appear to be using the Strings again in another switch; I am sure there is a better way, e.g. using an enumerated type rather than Strings.
 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[UPDATE] Thanks guys I changed the Math.random()'s to Random objects and it does feel righter. The real problem was in line 16 of the Class Ship. There's an extra semicolon that shouldn't be there. Wasted like 2 hours just to find that out :/.
 
Stevens Miller
Bartender
Posts: 1444
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yup, I see your extra semi-colon, and you're right. But, you did have a problem with the scope of the cast where I pointed it out. Go back and write a super short program that just prints the values returned from those lines. You need to have that 100% understood, or it will plague you forever.
 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes you are right. I was doing (int) Math.random()*5 but the correct way is (int) (Math.random()*5). Thanks
 
Campbell Ritchie
Marshal
Posts: 55774
163
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I still think the correct way to do it is
myRandom.nextInt(4)
Where does the 5 come from, if you want 4 different possible results?

Look at our style suggestions which require braces after every if; that might not have prevented you writing the extra semicolon but it might have made it easier to find.
 
Knute Snortum
Sheriff
Posts: 4087
112
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch and thanks for using code tags correctly!

Some thoughts about your code:

* Package names should be lowercase.

* (minor) In Java 7 and up you can leave out the generic type on the initialization if it is not ambiguous. So...

becomes


* Only variables that act as constants should have names with all caps. So SHIPS should be ships. An example of a constant would be:


* This line from your code has two problems (three actually, but one has been addressed):

"7" is a magic number, that is, what does 7 mean? Create a constant with a meaningful name and use the constant instead of "7".
"l" is a poor choice of names. What is "l"? Use meaningful variable names or at least document them.

* Don't use raw types in new code. Add the type to the return type here:
 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you Mr.Snortum I updated my code with your suggestions.
I have realized that the exists() method in Ship class is not working properly. When I insert some coords with the same fields it returns false instead of true(I think it always returns false). In my view there's nothing so I must be missing something.
 
Stevens Miller
Bartender
Posts: 1444
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Knute Snortum wrote:* Don't use raw types in new code. Add the type to the return type here:

Since I'm not sure, I'll put this into the form of a question: Would Dillon be better of with this?
 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stevens I'm sure thats exactly what he meant. I tried that and it solved all the problems I had. Without the return type the objects would get out as type Object instead of Coord
 
Junilu Lacar
Sheriff
Posts: 11165
160
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The logic in the exists method seems a bit convoluted to me. You have a list of Ships, why don't you just iterate though it and compare each existing Ship's coordinates against the one you're trying to check for. I would also change the name to fit the idea: isOccupied(Coord a) or hasShip(Coord a) is really what you're trying to find out. I'm not particularly fond of how you did the SHIPS list though. I would have used a BattleField or Board object and defined an addShip() method to do what you're doing in Ship. Assigning responsibilities to the appropriate object/class is important in writing coherent and cohesive programs. Each Ship would have a list of coordinates that it occupies.



A Board has a list of Ships.
Each Ship will have a list of Coordinates that it occupies and you should be able to ask a Ship if it occupies a given coordinate


HTH
 
Dillon Colt
Greenhorn
Posts: 11
Chrome Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu, first of all thank you for the feedback. I've been thinking about that but I'm kinda lazy to change it all. I guess I going to change some things so it gets more organized.
Btw if I come up with a different problem regarding this "project" should I post it here or make a new thread?
 
Paul Clapham
Sheriff
Posts: 22521
43
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Different problem? Different thread.

(At least for this thread, which has gone on for quite a while in the usual Ranch-style convoluted way. If it was a short and simple thread then "Oh yeah, I have this other problem" might be okay.)
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!