Randomly passing null references into objects

Question

I created a program which should display a spaceship (essentially two rectangles) and shoot a bullet from it every time I press the space button. It doesn't work, though because apparently, my program is passing in a null Ship reference into one of my other classes, the MoveAction class. I have absolutely no clue why it's like this because in my Main class, I instantiate my Ship object before my Shoot class (calling the constructor that uses the ship object). I'm sorry if this question is a little bit silly as I'm still a novice programmer who doesn't have a clue about what he's doing most of the time :) Here's my code:

public enum Direction {
    LEFT, RIGHT, SPACE
}

import javax.swing.JFrame;

public class Main {
    public static void main(String[] args) {
        JFrame frame;

        Ship s1;
        Shoot shoot;

        // Set the frame up
        frame = new JFrame();
        frame.setSize(400, 300);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setResizable(false);
        frame.setVisible(true);

        // Get some more necessary objects
        s1 = new Ship();
        shoot = new Shoot(s1);
        frame.getContentPane().add(shoot);
        s1.setShoot(shoot);

        // Threads
        Thread ship = new Thread(s1);
        ship.start();
    }
}

import java.awt.Graphics;
import java.awt.event.KeyEvent;

import javax.swing.Action;
import javax.swing.ActionMap;
import javax.swing.InputMap;
import javax.swing.JPanel;
import javax.swing.KeyStroke;

public class Shoot extends JPanel {

    Ship s1;

    public Shoot(Ship s1) {
        this.s1 = s1;

        addKeyBinding(KeyEvent.VK_LEFT, "left.pressed", new MoveAction(true, s1, Direction.LEFT), true);
        addKeyBinding(KeyEvent.VK_LEFT, "left.released", new MoveAction(false, s1, Direction.LEFT), false);

        addKeyBinding(KeyEvent.VK_RIGHT, "right.pressed", new MoveAction(true, s1, Direction.RIGHT), true);
        addKeyBinding(KeyEvent.VK_RIGHT, "right.released", new MoveAction(false, s1, Direction.RIGHT), false);

        addKeyBinding(KeyEvent.VK_SPACE, "space.pressed", new MoveAction(true, s1, Direction.SPACE), true);
        addKeyBinding(KeyEvent.VK_SPACE, "space.released", new MoveAction(false, s1, Direction.SPACE), false);

        setDoubleBuffered(true);
    }

    @Override
    public void paintComponent(Graphics g) {
        // Draw the ship
        super.paintComponent(g);
        s1.draw(g);
        g.fill3DRect(40, 50, 10, 10, false);
    }

    protected void addKeyBinding(int keyCode, String name, Action action, boolean keyPressed) {
        if (keyPressed) {
            addKeyBinding(KeyStroke.getKeyStroke(keyCode, 0, false), name, action);
        } else {
            addKeyBinding(KeyStroke.getKeyStroke(keyCode, 0, true), name, action);
        }
    }

    protected void addKeyBinding(KeyStroke keyStroke, String name, Action action) {
        InputMap inputMap = getInputMap(WHEN_IN_FOCUSED_WINDOW);
        ActionMap actionMap = getActionMap();
        inputMap.put(keyStroke, name);
        actionMap.put(name, action);
    }

}

import java.awt.Color;
import java.awt.Graphics;
import java.awt.Rectangle;

public class Ship implements Runnable {
    int x, y, xDirection, bx, by;
    boolean readyToFire, shooting = false;
    Rectangle bullet;
    Shoot shoot;

    public Ship() {
        x = 175;
        y = 275;
        bullet = new Rectangle(0, 0, 3, 5);
    }

    public void draw(Graphics g) {
        // System.out.println("draw() called");
        g.setColor(Color.BLUE);
        g.fillRect(x, y, 40, 10);
        g.fillRect(x + 18, y - 7, 4, 7);
        if (shooting) {
            g.setColor(Color.RED);
            g.fillRect(bullet.x, bullet.y, bullet.width, bullet.height);
        }
        shoot.repaint();
    }

    public void move() {
        x += xDirection;
        if (x <= 0)
            x = 0;
        if (x >= 360)
            x = 360;
        shoot.repaint();
    }

    public void shoot() {
        if (shooting) {
            bullet.y--;
            shoot.repaint();
        }
    }

    public void setXDirection(int xdir) {
        xDirection = xdir;
    }

    public void setShoot(Shoot shoot) {
        this.shoot = shoot;
    }

    @Override
    public void run() {
        try {
            while (true) {
                shoot();
                move();
                Thread.sleep(5);
            }
        } catch (Exception e) {
            System.err.println(e.getMessage());
        }

    }
}

import java.awt.Rectangle;
import java.awt.event.ActionEvent;
import java.util.HashSet;

import javax.swing.AbstractAction;

public class MoveAction extends AbstractAction {

    boolean pressed;
    Ship s1;
    Direction dir;
    private HashSet<Direction> movement;

    public MoveAction(boolean pressed, Ship s1, Direction dir) {
        System.out.println("moveaction class");
        this.pressed = pressed;
        this.s1 = s1;
        this.dir = dir;
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        try {
            if (movement.contains(Direction.LEFT)) {
                if (pressed) {
                    s1.setXDirection(-1);
                } else {
                    s1.setXDirection(0);
                }
            } else if (movement.contains(Direction.RIGHT)) {
                if (pressed) {
                    s1.setXDirection(1);
                } else {
                    s1.setXDirection(0);
                }
            } else if (movement.contains(Direction.SPACE)) {
                if (pressed) {
                    if (s1.bullet == null)
                        s1.readyToFire = true;
                    if (s1.readyToFire) {
                        s1.bullet.x = s1.x + 18;
                        s1.bullet.y = s1.y - 7;
                        s1.shooting = true;
                    }
                } else {
                    s1.readyToFire = false;
                    if (s1.bullet.y <= -7) {
                        s1.bullet = null;
                        s1.shooting = false;
                        s1.bullet = null;
                        s1.bullet = new Rectangle(0, 0, 0, 0);
                        s1.readyToFire = true;
                    }
                }
            }
        } catch (NullPointerException ex) {
            System.out.println("NullPointerException");
        }
    }

Stack Trace :)

at MoveAction.actionPerformed(MoveAction.java:24)
    at javax.swing.SwingUtilities.notifyAction(Unknown Source)
    at javax.swing.JComponent.processKeyBinding(Unknown Source)
    at javax.swing.KeyboardManager.fireBinding(Unknown Source)
    at javax.swing.KeyboardManager.fireKeyboardAction(Unknown Source)
    at javax.swing.JComponent.processKeyBindingsForAllComponents(Unknown Source)
    at javax.swing.SwingUtilities.processKeyBindings(Unknown Source)
    at javax.swing.UIManager$2.postProcessKeyEvent(Unknown Source)
    at java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(Unknown Source)
    at java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(Unknown Source)
    at java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(Unknown Source)
    at java.awt.DefaultKeyboardFocusManager.dispatchEvent(Unknown Source)
    at java.awt.Component.dispatchEventImpl(Unknown Source)
    at java.awt.Container.dispatchEventImpl(Unknown Source)
    at java.awt.Window.dispatchEventImpl(Unknown Source)
    at java.awt.Component.dispatchEvent(Unknown Source)
    at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
    at java.awt.EventQueue.access$500(Unknown Source)
    at java.awt.EventQueue$3.run(Unknown Source)
    at java.awt.EventQueue$3.run(Unknown Source)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
    at java.awt.EventQueue$4.run(Unknown Source)
    at java.awt.EventQueue$4.run(Unknown Source)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
    at java.awt.EventQueue.dispatchEvent(Unknown Source)
    at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
    at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
    at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
    at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
    at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
    at java.awt.EventDispatchThread.run(Unknown Source)

"Questions seeking debugging help ("why isn't this code working?") must include the desired behavior, a specific problem or error and the shortest code necessary to reproduce it in the question itself. Questions without a clear problem statement are not useful to other readers." — Andy Turner, Jan 25 '16 at 14:12
Also, given the tag: Possible duplicate of http://stackoverflow.com/questions/218384/what-is-a-null-pointer-exception-and-how-do-i-fix-it — Andy Turner, Jan 25 '16 at 14:13
Also a stack trace would be nice :) I do not know where nullpointer occurs — Neron, Jan 25 '16 at 14:13
I've copied your code to a local project to test it. The window opens, but doesn't show anything. Apart from that though, I don't get any NPE. As you've stated in your question, you instantiate the Ship in the Main: `s1 = new Ship();`; pass it to the Shoot instance: `shoot = new Shoot(s1);`; and use it in the Shoot-constructor: `this.s1 = s1; ... new MoveAction(true, s1, Direction.LEFT) ...`. So, as others have stated, please add the Stack Trace of the exception so we have a bit more to work with. I don't know which IDE you use, but in Eclipse I don't get a NRE, and your code seems fine to me. — Kevin Cruijssen, Jan 25 '16 at 14:28
Here is the stack trace... also I'm using Eclipse but I'm not sure what compiler I'm using... — Eames, Jan 25 '16 at 14:47

score 1 · Accepted Answer · answered Jan 25 '16 at 14:43

1

You never initialize your member variable HashSet<Direction> movement, that's why you always get a NPE.

Try this

private Set<Direction> movement = new HashSet<>();

By the way, declaring an implementation class HashSet as a variable type is a bad practice. You should use an interface Set instead.

answered Jan 25 '16 at 14:43

nolexa

2,392
2
19
19

I added it and it doesn't give me an NPE anymore, but my screen is still blank. – Eames Jan 25 '16 at 14:52
You asked about a null pointer and this question have been answered. The question "why screen is blank" is too broad. Try to debug or trace your application and come closer to the root cause. Then, if you still have problems - ask a more targeted question. No one knows what logic you're trying to implement except for you. It is hard and counter-productive trying to help you at this stage. – nolexa Jan 25 '16 at 15:21
Okay, thanks anyway, but can you explain why declaring a HashSet as a variable is bad? It seems to work better (but not completely) when using your suggestion. – Eames Jan 25 '16 at 15:41
Here is a good answer: http://stackoverflow.com/questions/147468/why-should-the-interface-for-a-java-class-be-prefered – nolexa Jan 25 '16 at 15:46

theDima · Answer 2 · 2016-01-25T14:29:37.360

-1

Inside your public Shoot(Ship s1) constructor you instantiate the new MoveAction(true, s1, Direction.LEFT), true);, making use of s1, which also is the name of the field.

The thing is that you are not allowed to reference fields until the constructor finishes it's run. The behavior in such cases in undefined. Probably your compiler thinks the s1 you refer to is the field. It's not the case in my compiler, but I don't know which compiler you are using.

In any case, try to give different names to the argument of the constructor and the field - s1, and pass to the MoveAction(..) constructor the argument.

edited Jan 25 '16 at 14:29

answered Jan 25 '16 at 14:24

theDima

746
4
12

It's not up to the compiler. The scope of the variable is defined in the language specification, and it's always the parameter `s1` in this context. Also, there's nothing that says that you're not allowed to use fields until the constructor is finished. If the field is properly initialized, there is no reason not to use it. So your answer is incorrect on both counts. – RealSkeptic Jan 25 '16 at 14:29
Probably you are right on the first thing about the compiler scope, and as I've stated, in my case this should work. But: except of that, the program provided here is correct and should work. And the second one - the instance's state is undefined before the constructor finishes its run, so one can't make any assumptions about the fields. – theDima Jan 25 '16 at 14:31
The program may be correct, but your answer is incorrect. The OP needs to provide more information as people asked in the comments, and then we may understand where the NPE is coming for. – RealSkeptic Jan 25 '16 at 14:34
My answer contains two statements and a general suggestion. The first statement is (probably) wrong, but the second one, and the suggestion (which can't be wrong, since it's a suggestion) are fine. – theDima Jan 25 '16 at 14:39
Statement 1: the compiler may be interpreting `s1` as `this.s1`. **Incorrect**. Statement 2: You are not allowed to reference fields until the constructor finishes its run. **incorrect**. Suggestion: replace variable name. But this variable has nothing to do with the problem. Therefore does not solve OP's problem, and the suggestion is therefore unhelpful and confusing. – RealSkeptic Jan 25 '16 at 14:56
@RealSkeptic Btw, I added the stack trace to my question if that might help. – Eames Jan 25 '16 at 15:12

Randomly passing null references into objects

2 Answers2