Follow basic coding conventions and never forget the following quote when things start to look grim:

Quote

There's gotta be a better way.

To clarify: whenever you have to write something that looks ugly or just seems wrong there's usually a better way of doing it.

On a slightly unrelated note, I just managed to finish a skeleton animation test program, which probably is the most hackish program I've ever written that actually runs. My main() function is colossal, all variables are static and I've got more temporary variables than I can count, but it works. Sometimes it's okay that way though, since this abomination isn't going into the final game anyway. Maybe that's another good tip: Whenever you're going to implement something new (and decently big), create a small stand-alone test program before you start integrating things into your game. It keeps your game code cleaner, and you'll most likely discover any problems with your idea when making the test program. I've pretty much followed this idea since I started with OpenGL, since there are so many ways that things can go wrong and when they do you often just end up with a black screen. Simply leaking OpenGL state or forgetting a single method call can make your screen black, and it's very time consuming to debug since if you want to System.out.println() some data you usually have to allocate buffers e.t.c to read back data from OpenGL. Sometimes you also have way too much data (for example a render target texture) so printing it as text isn't going to help at all. Geh, now I'm just rambling again, but my point is: Go forward a little slower and catch problems earlier. It's easier to find bugs in a small test program than in your complete game.

Like Loom_weaver wrote, do you have any examples of when your code looks "unclean"?

For the most part, it doesn't look too bad. There are several broken conventions that I can see, however it doesn't look much worse than a lot of the 'good' code that gets put into the actual Java API.

For the most part, it doesn't look too bad. There are several broken conventions that I can see, however it doesn't look much worse than a lot of the 'good' code that gets put into the actual Java API.

That's something you need to fix right there. If you ignore exceptions, your failure at that point will be replaced by mysterious NPE's later. If you really need some boilerplate handler to shut up the whole checked exception thing, then fill it in with "throw new RuntimeException(e)". But fergodsake, don't silently ignore exceptions.

That's something you need to fix right there. If you ignore exceptions, your failure at that point will be replaced by mysterious NPE's later. If you really need some boilerplate handler to shut up the whole checked exception thing, then fill it in with "throw new RuntimeException(e)". But fergodsake, don't silently ignore exceptions.

throw new RuntimeException(e) will shut up Eclipse then? The only reason that's there is because it complains it's not. These errors don't tend to happen, so normally I just put a println command and use system.exit.

Yes, it will shut up Eclipse. It effectively turns a checked exception into an unchecked one. That unchecked exception should be allowed to propagate all the way up and kill your program with a traceback. For a GUI app, you should catch it, show an error screen with the traceback, then exit.

A pure style thing I see is that entityCollisionWith could use a foreach loop instead of a C-style loop (the other one uses the index, so just leave it as-is). As for code organization in general: it's rare you'll ever write code that's so perfect that you never feel tempted to tweak or trim or rearrange it some way. As a rule, programmers tend to be OCD cases, comes with the territory. The real skill to learn with experience is knowing when to stop.

Well, it's more that you're catching an error and then doing nothing with it than anything else. It's bad form to do that.

At the very least, you should put a comment there stating that it's intentional so that when you look back at it (Or others view it) they won't go "huh". Also, since SlickException covers such a large area of possible exceptions, it's not a good plan to use a blanket catch like that if you're not going to pass it on up. If you have some sort of logging functionality (Even just Java's basic Logger), then you should output some information about the exception so that you can figure out if it was actually an exception or just something you want to ignore (I admit I tend to use Integer.parseInt(String s) surrounded by a catch(NumberFormatException nfe) {} for the purposes of checking text input that's supposed to be an integer).

And here I might be a little mean, too specific in my picking, but...

Another issue is, you have several 'Magic Constants' sprinkled through out your code. I see a bunch of +16 (In your constructor), hard-coded values for your sprite size (Line 42), hard-coded guards in your iterator in your constructor, a for-loop iteration that starts at 1 (Which I've had to do {PreparedStatements, something for parameterizing SQL statements, are indexed at 1 instead of 0}, but since convention is that iterators tend to start with the 0th element, starting at any other number, especially hard coded in, you should put a note to ensure people know why). In and of themselves, these aren't necessarily bad things. They're just things that you should watch out for, since any changes in say... Whatever the +16 is, the size of your sprite sheet, the number of possible sprites in your sheet, will force you to search through your code to find these places and change them.

The only other thing that jumps out at me are other minor convention/readability issues. Such as is in line 58, the tick method. You have a class textBoxBase. While the language allows this, it goes against a major convention in Java that states that class names should be UpperCamelCase (TextBoxBase) and instance/variables should be lowerCamelCase (textBoxBase). This is primarily so that when skimming the code, it's easy to differentiate between different things. It's especially bad when the text processor colors things based on their caps (Look at the code chunk below). If you're skimming it, it looks like there are two variable names, one after another in that method declaration.

Well, it's more that you're catching an error and then doing nothing with it than anything else. It's bad form to do that.

At the very least, you should put a comment there stating that it's intentional so that when you look back at it (Or others view it) they won't go "huh". Also, since SlickException covers such a large area of possible exceptions, it's not a good plan to use a blanket catch like that if you're not going to pass it on up. If you have some sort of logging functionality (Even just Java's basic Logger), then you should output some information about the exception so that you can figure out if it was actually an exception or just something you want to ignore (I admit I tend to use Integer.parseInt(String s) surrounded by a catch(NumberFormatException nfe) {} for the purposes of checking text input that's supposed to be an integer).

And here I might be a little mean, too specific in my picking, but...

Another issue is, you have several 'Magic Constants' sprinkled through out your code. I see a bunch of +16 (In your constructor), hard-coded values for your sprite size (Line 42), hard-coded guards in your iterator in your constructor, a for-loop iteration that starts at 1 (Which I've had to do {PreparedStatements, something for parameterizing SQL statements, are indexed at 1 instead of 0}, but since convention is that iterators tend to start with the 0th element, starting at any other number, especially hard coded in, you should put a note to ensure people know why). In and of themselves, these aren't necessarily bad things. They're just things that you should watch out for, since any changes in say... Whatever the +16 is, the size of your sprite sheet, the number of possible sprites in your sheet, will force you to search through your code to find these places and change them.

The only other thing that jumps out at me are other minor convention/readability issues. Such as is in line 58, the tick method. You have a class textBoxBase. While the language allows this, it goes against a major convention in Java that states that class names should be UpperCamelCase (TextBoxBase) and instance/variables should be lowerCamelCase (textBoxBase). This is primarily so that when skimming the code, it's easy to differentiate between different things. It's especially bad when the text processor colors things based on their caps (Look at the code chunk below). If you're skimming it, it looks like there are two variable names, one after another in that method declaration.

It wasn't mean at all! It was very helpful ^^It's funny though, I went ahead and changed TextBoxBase with the refactor, but it's funny because it was the first class I made in the game. That's probably why it's the only one that starts with a lower cased character. I didn't know the importance prior.

I strongly advice against such code. It's so easy to make a mistake, and you end up with lots of 'magic' offsets. When you expand your code to 8 directions, you'd have to check all those indices and make the constants 'STILL' and 'MOVING' multiples of 8. When your project is growing, more and more patches are required to make it all work again.

I strongly advice against such code. It's so easy to make a mistake, and you end up with lots of 'magic' offsets. When you expand your code to 8 directions, you'd have to check all those indices and make the constants 'STILL' and 'MOVING' multiples of 8. When your project is growing, more and more patches are required to make it all work again.

but you should try to make it more readable... we're not coding in C after all.

Use enums for starters. Enum.ordinal() can be used to provide an index into an array.

Would you guys mind explaining the purpose of an Enum to me? I've read on it, and it reminds me a bit of constants without values. I read this at the Java doc, and honestly I hate their explanations, so if you could please, enlighten me. Haha.

Basically, it allows you to specify a set of possible inputs that will always be bounded and cannot be changed programmatically in anyway. Once you define your Enum its structure, and the number of elements in it, will stay constant the whole time. This allows you to remove a lot of code that you might otherwise need to catch certain types of errors and helps make your code more readable. Further, if you require the constants to be used in several different places, it allows you to simply provide the Enum instead of including like a Constants interface to hold all of these values. While it can be just about the same as using public final int CONSTANT_NAME = 1, it makes sure that you don't sometimes think "Oh, I know what this constant is supposed to be, so I'll just use the value instead of the variable".

However, they don't do anything magical. If your constant isn't meant to define a part of a logical set, then you probably won't want to use an Enum for it. Further, if your logical set ever changes (As in the elements change, not the sate of the elements) you don't want to use an Enum. There is also something of a trade-off in readability/understandability when the number of elements in your Enum gets too large.

Quick, silly example of some enums that form a logical set. Instead of having a bunch of integer values, that might end up being the same value (ACE = 1, HEART = 1), you allow the language to take care of it. Also, you never have to worry about another program attempting to interface with yours, that has Face.REVERSE_ORDER, Face.DRAW_TWO_CARDS, etc that might have had the same constant values are you Face.ACE and FACE.Two.

An Enum is a class whose instances are all statically enumerated (named) in the declaration of the class. They're most commonly used as symbol types, an object which has itself as its own value, but you can literally use them like any class, except that they can't participate in class inheritance (interfaces are still okay). Additionally you can use them in switch/case statements, and you get some extra methods for looking them up by name, storing them in EnumSets, and so on.

Basically, it allows you to specify a set of possible inputs that will always be bounded and cannot be changed programmatically in anyway. Once you define your Enum its structure, and the number of elements in it, will stay constant the whole time. This allows you to remove a lot of code that you might otherwise need to catch certain types of errors and helps make your code more readable. Further, if you require the constants to be used in several different places, it allows you to simply provide the Enum instead of including like a Constants interface to hold all of these values. While it can be just about the same as using public final int CONSTANT_NAME = 1, it makes sure that you don't sometimes think "Oh, I know what this constant is supposed to be, so I'll just use the value instead of the variable".

However, they don't do anything magical. If your constant isn't meant to define a part of a logical set, then you probably won't want to use an Enum for it. Further, if your logical set ever changes (As in the elements change, not the sate of the elements) you don't want to use an Enum. There is also something of a trade-off in readability/understandability when the number of elements in your Enum gets too large.

Quick, silly example of some enums that form a logical set. Instead of having a bunch of integer values, that might end up being the same value (ACE = 1, HEART = 1), you allow the language to take care of it. Also, you never have to worry about another program attempting to interface with yours, that has Face.REVERSE_ORDER, Face.DRAW_TWO_CARDS, etc that might have had the same constant values are you Face.ACE and FACE.Two.

java-gaming.org is not responsible for the content posted by its members, including references to external websites,
and other references that may or may not have a relation with our primarily
gaming and game production oriented community.
inquiries and complaints can be sent via email to the info‑account of the
company managing the website of java‑gaming.org