aspose file tools*
The moose likes Game Development and the fly likes Refactoring Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Game Development
Bookmark "Refactoring" Watch "Refactoring" New topic
Author

Refactoring

Scotty Young
Greenhorn

Joined: Oct 26, 2006
Posts: 10
I'm trying to develop a simple java version of pacman. Im wondering if anyone can take a look and give me some advice on splitting it up into proper OO classes. I'm not sure where to start. I've attached my classes so far...

Main JFrame class -





The pacman/paint class:

Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

Hi Scotty. A couple of important points:

Don't let your main class extend JFrame and you especially shouldn't let your PacMan class implement Runnable and any listeners. Always prefer composition over inheritance. This means the frame should be a part of your main class, your main class shouldn't be a frame. Your PacMan class should have something that runs the game and listens for events, it shouldn't take that responsibility itself. All it should be responsible for is drawing. Use according names too. Pacman sounds like it would be part of the game model. The graphical component should be called PacmanPanel or something.

When you call methods that fiddle with Swing components, make sure they are called on the Event Dispatch Thread. Your TuPacMan class is currently being constructed on the main thread.

Make all your fields private. *Always*. Seriously. There are moments where it can be beneficial to make fields package private or protected, but if you don't know for sure if such a moment has presented itself, it probably didn't. So just make your fields private.

Do not start new threads in constructors. Threads should be started by calling a method after the object has been constructed. *Never* pass a "this" reference from the constructor. This is very dangerous.

Try to keep members as private as possible. paintComponent() was protected, and should remain so.

Why are you drawing to an image, and then copying the image to the component? Just draw to the component directly. I also think Swing is double buffered by default. No need to call the method.

Use the @Override annotation when you're overriding methods, such as paintComponent().

Don't use Thread.sleep(). You should use a Swing timer instead.

When implementing listeners, use adapters. KeyAdapter is a great starting point if you want to implement a KeyListener. The same for WindowListener/WindowAdapter and MouseListener/MouseAdapter.

Here's an example demonstrating some of these concepts:
Scotty Young
Greenhorn

Joined: Oct 26, 2006
Posts: 10
That's a great reply, thank you. I'm going to take all that in consideration and go back and change the code. The reason I was drawing to an image first is becuase I encountered problems with flickering when drawing to the screen. That was my attempt to double buffer to remove the flickering.

Przemek Boryka
Ranch Hand

Joined: Dec 06, 2011
Posts: 51

Some time ago, I made game similar to PacMan, below is source code of main class, where you can find methods that are responsible for:
- initial settings game
- game update //gameUpdate() (states of the game, collision detection)
- game render//gameRender() (render menu, render game-play,
- input actions (keyboard actions)
- mouse actions

and a special method "initFullScreen" which provide all actions to show game in selected resolution in fullscreen.

I did not say that this is the best solution, that you can use in your game, but it may be helpful to start.


Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4347
    
    2

Stephan gave some great advice, and i don't mean to step on toes, but the idea of not extending or implementing is his opinion. not everyone agrees.


SCJP
Visit my download page
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

Yes, of course. I tend to present my points as if they are hard rules, but I hope everyone realizes they are merely (strong) suggestions, heavily influenced by my personal coding style.

[edit]

To expand on the extend/implement discussion, the reason I'm discouraging it *in this particular case* is because main classes should never extend some sort of graphical component. It means your main program is forever destined to be a JFrame, even if later on something better might come along, or you would just like to make an ASCII version of your game.

The display also shouldn't implement Runnable, because the display simply isn't something that should be runnable. It displays something. Conceptually it can't be run. It doesn't model a task. Graphical components also shouldn't implement listeners. This is the most subjective point. Many people disagree. However, if you let them implement a listener, it means some other code could take your graphical component, and register it as a listener for events to *other* graphical components. That hardly seems like it makes any sense.

The problem is that many programmers make choices because those choices work. The computer doesn't care. There are a lot of conceptual matters they fail to consider though. Whenever you find yourself using the extends/implements keywords, you should first ask yourself: Is my class *really* something I can consider to be of type X?

For instance, Fruit has a color. So if I want to create a Car class that also has a colour, should I let it extend Fruit? The compiler won't complain, and the program will run perfectly. However, conceptually it makes no sense at all.
The same is true with JPanel and ActionListener for instance. A graphical component is *not* a listener. It may work, but it doesn't make sense. The difference seems harder to see than with the obvious example of a Fruit and a Car, but (in my opinion) the issue is the same.
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

Actually, I made a mistake in my design above. I made the display responsible for timing the updates to the model. The model (Maze in this case) should handle the timing itself, and notify the display when it has updated.
Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4347
    
    2

i should have read the code(or at least part of it) before commenting. this is the first time i have seen a class implement runnable then create a thread to call its own run method.

i still fail to see the problem with having a JPanel implement mouseListener or mouseMotionListener for example. i have a painting program that implements both to draw shapes. just my opinion though.
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

Alright. So what happens when I take your panel, and register it as a listener for my JButton? Does it start performing actions on itself? That seems weird.

Another problem is that it gives one class too much responsibility. You're not separating concerns. It decreases cohesion, and makes your classes harder to maintain.

I also assume that your graphical component is public. If you implement ActionListener, it will also make this part of the class public. This is completely unnecessary. No class other than the component itself cares about events. So having an anonymous or package private class would be superior.
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

Let's take a look at Przemek's code. While I'm quite sure it works as intended (assuming some omitted code was included); it's a big slab of code that takes bites it cannot chew; with respect to Przemek. The very first thing I would do to refactor that code was create 4 new source files and separate some of that code over some new classes.
Randall Twede
Ranch Hand

Joined: Oct 21, 2000
Posts: 4347
    
    2

i am beginning to see your point. the JPanel and the JFrame that contains it are public. the JFrame handles Item events and Action events. all the event handling code is public also(probably because the are declared that way in the two super classes).

i have started a thread more or less about this here

so i will just let you help this guy.
Przemek Boryka
Ranch Hand

Joined: Dec 06, 2011
Posts: 51

The game runs pretty well, although I had problems with the correct implementation of (my) collision detection algorithm, it may result from the fact that I did not use with any suggestions, books.

Below is a url to picture of the structure of source files in the game.

Source files structure

Here is url to video of game play JarMan game play video (low quality)
I do not know why, but the sound lags behind the image :/, in game everything is fine!

JarManPane is a JPanel (window version) version of the same game!

One of most important is World.java class. In this class are made:
- collision detection JarMan(main character) - walls
- JarMan - ghosts
- ghosts - walls
- JarMan - obstacles (yellow, blue, white, ghost icon)
- creating a virtual world (paths, walls, obstacles)

There is no implemented any intelligent algorithm of ghosts movement. I had no time and desire ;).

Sorry for my english ! I am still learning
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3646
    
  16

That's quite impressive. The game looks really smooth, and beautiful too.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Refactoring