• Post Reply Bookmark Topic Watch Topic
  • New Topic

can somone point out any major errors or mistakes I've made?  RSS feed

 
zane ouimet
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'd like it if someone could scan through what I've written and see if there are any majorly dumb things I've done so far. I know this is a lot to ask but it might help me the code is correct in syntax but it is incomplete its jsut for practice. thanks in advance :P I tried asking some questions on stackoverflow but I think that I'm a bit out of my depth over there.
The program is the begining of what I *hope* is a simple text based game the files are as follows

Game1:


Player


Object


Firstroom (this is a test to see how I might build the rooms and such)



If anyone replies thanks already and be gentle I'm new :P
 
Stephan van Hulst
Saloon Keeper
Posts: 7976
143
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to CodeRanch!

I haven't looked too closely yet, but there are a couple of things that spring out immediately. First of all, you're not using encapsulation. Make your fields private, and let other classes access them through methods. Right now, anybody can set a Player's playerHealth to any value they please.

I also notice you initialize all your fields to default levels. Initializing to 0 or null is useless, because all fields are always initialized to these values at first. And there's not much point in using default values if you're going to overwrite them in the constructor anyway.

Don't call what should be called something like "Item" "Object". Object is an identifier that's already used for the most important class of all. The one that every other class inherits from. Try to avoid naming your classes the same as well known classes in the standard API.

Don't use an array of items. Use a List of items. And types should not be Strings. Types should be represented by enums.

It also appears you're going to make a separate class for each room, and provide it with a bunch of static methods. You *really* shouldn't do this. Make one Room class, and give it instance methods. When you construct all the rooms for your game, you can let them read their narrative from a file; that way you prevent mixing data and code.

 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephan van Hulst wrote:Don't call what should be called something like "Item" "Object". Object is an identifier that's already used for the most important class of all. The one that every other class inherits from. Try to avoid naming your classes the same as well known classes in the standard API.

I'd say that a little more strongly: Never name a class that yourself "Object"! All classes in Java have a common superclass called Object, which in the package java.lang. If you name your own class "Object", then it is going to be very confusing if you mean java.lang.Object or your own Object class in the source code and you can easily get many hard to understand errors.
 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And I shall complain about there being so many static members. you should use instance methods, unless you have good reasons, maybe using the Campbell Ritchie classification of methods, to make them static.
 
zane ouimet
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you for the replies this is indeed the help I need all over the place :P What is the benefit of using a list over an array? is there any reason why the other classes shouldn't be allowed to just access the other classes besides the obvious?
 
Stephan van Hulst
Saloon Keeper
Posts: 7976
143
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Lists can have variable length. You can easily add and remove objects and they eliminate null checks. They can do anything an array can do, and more.

Classes shouldn't be allowed to access fields defined in other classes, because this makes it extremely difficult to maintain class invariants. Invariants are like "promises" you make regarding the state of an object. For instance, your Player class could promise that health never reaches levels higher than 100. If the field isn't private, it simply cannot promise this because it doesn't know if other classes exist that modify the field. These promises are important, because they allow you to prove correctness of your program, up to a certain point.

Doesn't it give you more confidence knowing that objects of the String class, for example, won't suddenly change without you knowing about it? You should do the same for your own classes.
 
zane ouimet
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you for explaining +1
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!