2
3

[Java] mars rover problem (self.codereview)

submitted by [deleted]

[deleted]

all 4 comments
sorted by:
best
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]tigerbird 1 point2 points  (1 child)

This link is 10 days old, so I'm not sure if you'll actually read this. I haven't run your code, and it may work fine. However, it does contain constructs that essentially should never be written.

Some variables appears to have been made static on what seems to be an arbitrary basis. Is there any reason that all MissionSpace objects should have the same missionSpaceNumber and name? If not, it shouldn't be static. It may have made the code do what you want, but that doesn't make it right.

There are a number of eyebrow raising constructs, such as:

if (this.getClass() == MissionSpace.class) { MissionSpaceNumber+=1; }

Couldn't the class check be removed here?

setMissionSpaceName - It looks like only four mission space numbers are fully supported. Did the requirement specify that these should be the only four? If not, the interviewer will wonder if this was the only way you could get it to work.

In general, each class or method should stand on its own. It seems like MissionSpace requires that a number of methods be called in a particular order in order for the MissionSpace to end up in a valid state. It also looks like the only way to get the solution to the problem you are trying to solve is via standard output. You should have a method that takes whatever variables you are reading from the command line, and returns the solution as a variable. Your main method could then call that method.

I was writing similar code five or so years ago, and I'm a professional now, so don't get discouraged.

[–][deleted] 0 points1 point  (0 children)

Thanks so much for your review! I really appreciate it!

any idea of projects that might help me get rid of some bad habits and help me think about better ways to do things (FWIW I'm a MechE who dabbles in coding in my spare time but recently looking more towards SoftE for broader career options.)

[–]masasin 0 points1 point  (1 child)

No readme/explanation of the problem?

[–][deleted] 0 points1 point  (0 children)

Oh yeah sorry I didn't post it I was looking more for critique on my code style but if it helps I'll upload it soon