*
The moose likes Game Development and the fly likes Suggestions to make code (Pong) better (from the code I have, not adding additional code) Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Android Security Essentials Live Lessons this week in the Android forum!
JavaRanch » Java Forums » Java » Game Development
Bookmark "Suggestions to make code (Pong) better (from the code I have, not adding additional code)" Watch "Suggestions to make code (Pong) better (from the code I have, not adding additional code)" New topic
Author

Suggestions to make code (Pong) better (from the code I have, not adding additional code)

john price
Ranch Hand

Joined: Feb 24, 2011
Posts: 495

Pong.java:

Any suggestions to make this work better or faster? Is there anything I should change? I don't want to add any extra features, just make my existing code better. All comments and help are welcome. I coded this from scratch by myself. You have permission to use this, edit this, and claim it as your own if you would like.
Thanks,
cc11rocks


“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” (Mosher's Law of Software Engineering)
“If debugging is the process of removing bugs, then programming must be the process of putting them in.” (Edsger Dijkstra)
john price
Ranch Hand

Joined: Feb 24, 2011
Posts: 495

updated Pong.java:
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3600
    
  14

- Separate your game logic from your graphical interface. Use a simple class to keep track of where all the stuff is. The only thing the JPanel should do is draw the stuff, nothing more.
- Don't let your panel implement any listeners. Make separate listener classes, or use anonymous classes.
- Instead of MouseListener, you can use MouseAdapter.
- Use meaningful field names. "pl" means nothing. In a few months, you will have forgotten what it stands for. The same is true for your other fields, to a lesser or greater extent.
- Why are you using protected access? Don't use protected, unless you have a *really* good reason to use it.
- Why are you using the Boolean class, instead of boolean primitives?
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

Instead of (int) ((Math.random() * 2) + 1) you might want to look at Random.nextInt(int n).

Instead of You might want to consider

Instead of if (something == true) you might want to simplify it: if (something)

Instead of constants such as "left", "right", "up", "down"; you might want to consider an enum for directions.

You have specified that Pong extends JPanel. Is that correct? Is Pong a JPanel that other people might want to put components in? Or should this be a "has-a" relationship: Pong has a JPanel that it puts it's own components into, but nobody else gets to see.


The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
john price
Ranch Hand

Joined: Feb 24, 2011
Posts: 495


Instead of constants such as "left", "right", "up", "down"; you might want to consider an enum for directions.

You have specified that Pong extends JPanel. Is that correct? Is Pong a JPanel that other people might want to put components in? Or should this be a "has-a" relationship: Pong has a JPanel that it puts it's own components into, but nobody else gets to see.


Could you give an example of an enum?
Could you explain the last paragraph? I don't understand. I know what the "has-a" relationship but I don't get what you are saying.
Thank you for the if (something) comment.
Thanks,
John Price
john price
Ranch Hand

Joined: Feb 24, 2011
Posts: 495

Stephan van Hulst wrote:- Separate your game logic from your graphical interface. Use a simple class to keep track of where all the stuff is. The only thing the JPanel should do is draw the stuff, nothing more.
- Don't let your panel implement any listeners. Make separate listener classes, or use anonymous classes.
- Instead of MouseListener, you can use MouseAdapter.
- Use meaningful field names. "pl" means nothing. In a few months, you will have forgotten what it stands for. The same is true for your other fields, to a lesser or greater extent.
- Why are you using protected access? Don't use protected, unless you have a *really* good reason to use it.
- Why are you using the Boolean class, instead of boolean primitives?

pl means player length, etc. originally had comments for saying p = player etc...will add them back
MouseListener is for Swing, MouseAdapter is for AWT. My code is Swing. Please explain further.
boolean comment taken into effect.
all protected accesses are now changed to private.
please explain first comment.
Thanks,
John Price
john price
Ranch Hand

Joined: Feb 24, 2011
Posts: 495

updated code:
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

john price wrote:Could you give an example of an enum?



john price wrote:Could you explain the last paragraph? I don't understand. I know what the "has-a" relationship but I don't get what you are saying.


This relates to what IS an object, and what other objects can you say your object HAS-A instance of? (sorry for the bad grammar). A simple example might be considering something like a specific type of fruit, such as an apple, and apple seeds. In this case, we can say that an Apple IS-A fruit, and it HAS-A seed (more than one really). It would be incorrect to say that an Apple IS-A seed, or than an Apple HAS-A fruit.

This is something you need to think about when developing Object Oriented code. Is your class a real sub-class of some other class (IS-A), or does it simply use that other class (HAS-A). In the case I was asking about, you said that Pong extends JPanel. This means that Pong IS-A JPanel. A JPanel IS-A container that anyone can put anything in. So in saying that your class extends JPanel, you are saying that your class is a container that anyone can put anything in. This means that someone could create their own class that either extends or uses Pong and put objects into it that obscure your application. This is almost certainly not want you want. Whereas if you say that Pong just happens to use a JPanel, (HAS-A) then you can keep everyone else from changing it.
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11404
    
  81

john price wrote:
Stephan van Hulst wrote:- Use meaningful field names. "pl" means nothing. In a few months, you will have forgotten what it stands for. The same is true for your other fields, to a lesser or greater extent.

pl means player length, etc. originally had comments for saying p = player etc...will add them back

Two points:
  • It is great that you know what pl stands for today. But if you come back to this same code in six months time will you still remember what it stood for? What about 5 years from now? (Speaking from experience, if you work on 2 other projects tomorrow, then 3 days from now you will probably have forgotten what pl stands for)
  • One of the major advantages we have gained in the last few years is that there is no problem with having longer variable names. It was not so long ago that a variable name like pl would have been considered great, since manually typing it 60 times in one file over a slow modem would be much better than trying to type playerLength 60 times over a slow modem. These days I start typing pl and the IDE suggests that it should be playerLength, so I get code that is much easier to read, with almost no sacrifice of speed.
  • john price
    Ranch Hand

    Joined: Feb 24, 2011
    Posts: 495

    So what do I need to do to fix it? For Java GUI's, I have only used Swing. It is useful and works for me. Should I be using a canvas (AWT) or something else. If my things are private, then no one can use them outside of my Application. So it would be pointless to extend Pong.
    Confused,
    John Price aka cc11rocks

    EDIT: Also, thank you for the variable suggestion. I will change it now.
    Stephan van Hulst
    Bartender

    Joined: Sep 20, 2010
    Posts: 3600
        
      14

    MouseListener is AWT as well. It doesn't matter. Swing and AWT play nice together. MouseAdapter is a class that implements MouseListener and MouseMotionListener with empty methods. This way, you won't have to implement them yourself, if you don't use them.

    You should have a JPanel subclass that has a Pong field. Pong keeps track of the location of everything. Your JPanel subclass looks at the Pong to determine where it should draw things.

    john price wrote:If my things are private, then no one can use them outside of my Application. So it would be pointless to extend Pong.

    Exactly. You don't want anything to extend Pong. You want other classes to use Pong. You should try to avoid extending classes whenever possible.
    john price
    Ranch Hand

    Joined: Feb 24, 2011
    Posts: 495

    Stephan van Hulst wrote:MouseListener is AWT as well. It doesn't matter. Swing and AWT play nice together. MouseAdapter is a class that implements MouseListener and MouseMotionListener with empty methods. This way, you won't have to implement them yourself, if you don't use them.

    You should have a JPanel subclass that has a Pong field. Pong keeps track of the location of everything. Your JPanel subclass looks at the Pong to determine where it should draw things.


    So make Pong() JLabel()?
    I don't get it.

    And anyway, since I already have MouseListener set up, shouldn't I leave it alone and just use this for future reference?

    Anyway, here is human friendly, easily "rememberal" code:

    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11404
        
      81

    john price wrote:So what do I need to do to fix it?

    Instead of saying:You should say
    (Except that, as noted by others, you really shouldn't have Pong implement listeners either - separate listener classes and/or use of adapter classes with anonymous classes will make your code so much easier to read).

    john price wrote:For Java GUI's, I have only used Swing. It is useful and works for me. Should I be using a canvas (AWT) or something else.

    Nope - this is completely avoiding trying to solve the problem. What you are doing at the moment is saying something like a dog is a tail. Whereas you should be saying that a dog has a tail. Simply changing the animal type does not stop the overall problem of trying to say that a dog and a tail are the same.

    john price wrote:If my things are private, then no one can use them outside of my Application. So it would be pointless to extend Pong.

    Oops - I missed that you had the constructor as private. This is also considered bad style unless you have a very specific reason for doing this.

    If you had not made your constructor private, I could have done some weird things to your project. For example, if I take the code you first posted, I can do a simple attempt at cheating by doing something as simple as:

    I should not have had access to any of those methods or variables.

    To put it another way - while you have protected your methods and variables, you have declared that Pong IS-A JPanel. Which means that I have access to all the fields and methods that JPanel has made public and that any ancestors of JPanel have made public.
    Stephan van Hulst
    Bartender

    Joined: Sep 20, 2010
    Posts: 3600
        
      14

    Here is some skeleton code to give you some ideas:

    john price
    Ranch Hand

    Joined: Feb 24, 2011
    Posts: 495

    Why don't I just add the word "final" to my code. Then no subclasses can extend Pong and the "bad things" I did don't matter...
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11404
        
      81

    Perhaps we misunderstood. Your original request was for suggestions on how to make the code better. Were you looking for something other than what we are providing? Performance suggestions perhaps?
    john price
    Ranch Hand

    Joined: Feb 24, 2011
    Posts: 495

    Andrew Monkhouse wrote:Perhaps we misunderstood. Your original request was for suggestions on how to make the code better. Were you looking for something other than what we are providing? Performance suggestions perhaps?

    Well you pointed out a problem. Wouldn't adding the keyword "final" fix it? How could I get better performance from this?
    I was thinking making the ball movement algorithm better and the AI skill level (algorithm). I want the ball to look like it's bouncing off of walls and paddles normally but be semi-random (not the same pattern every time; otherwise boring game because it's the same every time). I want the AI to be beatable but not too easy. This is what I was thinking. Also, I think my ball bouncing off of the AI's paddle algorithm is off. The ball seems to be hitting the backside of the AI paddle, the same way as the player paddle. This is what I was thinking of but any other suggestions, I would like too. I'm a noob Java programmer so I am having a tough time with your previous suggestions.
    Thanks and sorry,
    cc11rocks

    EDIT: I've also noticed an issue with the paddle moving up. I've had to change the original:

    to:

    While when the paddle goes to hit the ball down the code is:

    which works fine.
    It should work about the same moving up or down so I'm confused on this part.
    Mich Robinson
    Ranch Hand

    Joined: Jun 28, 2009
    Posts: 250
        
        1
  • Wouldn't this be better as an applet rather than an application?
  • This bit should be defined as a function (int) ((Math.random() *x)+1)
  • The comparisons against "right" "left" etc should really be with integer constants.
  • You wanted to know how to make it faster but surely if pong won't run fast on a modern computer then there's something very wrong somewhere.
  • You don't want to add new features? pong is 40 years old (which in dog years is as old as it gets) I think if you're going to spend the time to write pong then you should at least add a few new features!



  • Arcade : Alien Swarm
    Board : Chess - Checkers - Connect 4 - Othello
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11404
        
      81

    Heh - OK, so we were giving you answers that were completely at odds with what you were asking. Oops.

    John wrote:I want the ball to look like it's bouncing off of walls and paddles normally but be semi-random (not the same pattern every time; otherwise boring game because it's the same every time).

    There are books dedicated to how to do physics in games, so believe me when I say that anything I write here is only the tip of the iceberg.

    Think about what happens when you hit a ball with a bat in real life: if the ball is coming straight at you, and you hit the ball with the center of your bat, it will fly straight. If you hit it with the outer edge or the inner edge, it will go off on a different angle. Some bad graphics to demonstrate:
    (Imagine the vertical line is the ball heading towards your bat, and the angled line is the ball leaving your bat). The opposite can be considered to be true - if the ball came in on a an angle, and you hit it with the correct edge of your bat, then it might approximate a straight line leaving your bat.

    A variation on this is that if a ball is coming at you on an angle, and you hit it with your bat dead center, then the angle at which it leaves added to the angle at which it enters should equal 90° to which it came in:
    So if it came in on a 45° angle, it should leave at (90° - 45°) 45°. If it came in at 30° and you hit it with the center of your bat, it would leave at (90° - 30°) 60° In addition, you can mimic the other rules - you might decide that the very edge of your bat is the 180° angle point - any ball that hits it should leave at 180° minus the incoming angle.

    Since a human is likely to vary exactly what part of their bat hits the ball at any given time, the angle with which the ball leaves the bat will vary each time. This will naturally increase the apparent randomness of the game.

    To make it easier for the human, you can have the AI make it's best effort to hit the ball in such a way that it is returned almost straight on to the human (thereby giving them the easiest chance of hitting it). You will see this all the time if you watch a parent trying to teach their child tennis - the kid will hit the ball all over the place, inadvertently making the parent run for the ball, while the parent will do their best to get the ball back to the child. Of course you will want to have some randomness so that the AI does not do this perfectly every time, or else it will be boring for the human.

    Hopefully that has given you some ideas. I'll be offline for a bit, so hopefully some other people will chime in with some ideas as well.
    Emmanuel F. Borges
    Greenhorn

    Joined: Mar 25, 2011
    Posts: 12

    I think you should pay more attention to some concepts of Object Oriented Programming.

    For example, the game Pong has one ball and two paddles, so you could create a Ball class and a Paddle class. This way, all the logic concerning these two entities would be encapsulated within their classes.

    You could also create a manager class that maintains the game main loop and a class responsible for drawing entities on screen (Separation of Concerns). This way, your code would be more readable and maintainable.

    Of course, you will have to think a bit to figure out how to organize all the information, but it's worth it.
    Stephan van Hulst
    Bartender

    Joined: Sep 20, 2010
    Posts: 3600
        
      14

    Emmanuel F. Borges wrote:For example, the game Pong has one ball and two paddles, so you could create a Ball class and a Paddle class. This way, all the logic concerning these two entities would be encapsulated within their classes.

    I disagree. Object Oriented programming is a means, not a goal. In this case, one class would be more than enough to hold all the game logic. It only needs to keep track of the positions and dimension of two paddles and a ball, and that's it. Making separate classes would make the code much more verbose than it has to be, in my opinion.

    However, I agree that this one game class should be completely separate from everything else, like the GUI.
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Suggestions to make code (Pong) better (from the code I have, not adding additional code)
     
    Similar Threads
    keyboard arrow keys not working
    Need help in ball breaker game
    Code is not working
    non-static method nextBoolean() cannot be referenced from a static context error
    Zooming in and out - cursor not in the right place