5 replies to this topic

[Cander]

I made a short tester program to illustrate this:

abstract class ThreadTest implements Runnable
{
    Thread thread;
    
    ThreadTest()
    {
        thread = new Thread(this);
        thread.start();
    }

    public void run()
    {
        while (true)
        {
            update();
            try { Thread.sleep(500); } catch (InterruptedException e) {}
        }
    }
    
    abstract void update();
    
    public static void main(String[] args)
    {
        new SubTest();
    }
}

class SubTest extends ThreadTest
{
    Person p = new Person();
    void update()
    {
        //fixes the NullPointerException.   
        //if (p != null) 
            p.update();
    }
}

class Person
{
    int age = 0;
    
    void update()
    {
        age++;
        System.out.println(age);
        if (age >= 5)
            System.exit(0);
    }
}

SubTest's update method will regernate a Nullpointerexception when running at this state because reference p is apperently null when runned. But how can reference p ever be null really? When a new object of SubTest is created reference p should directly be initialized a value (new Person) because its written in class definition?

I noted that this can be worked around by adding

if (p != null)

to check if p is null, and if so, skip updating this round. What am I missing to understand here? Is the update method called in SubTest before like the whole "creation" of the object is complete so for a short time p equals null instead a of a Person object?

A bigger project of mine is built up on this kind of thread-looping and I have to to put in if-statements everywhere to check if references really pointing at a object when they are used. This feels like a bad way doing it, how can I solve this in a more "corect way"?

Thanks in advance!

Same question discussed:
Nullpointerexception when using a looping thread (Java in General forum at JavaRanch)
OTN Discussion Forums : NullpointerException when using a ...

Edited by Cander, 25 November 2010 - 02:10 PM.

[wim DC]

I've read the response at JavaRanch. They are correct.
Either you put the if-statement back there.
or you don't do .start() in the constructor of ThreadTest. (Either in the main, or in the SubTest constructor)

Since the class is abstract and will always be extended, to me it's more likely to put the .start in the subclass.

[Cander]

oxano said:

I've read the response at JavaRanch. They are correct.
Either you put the if-statement back there.
or you don't do .start() in the constructor of ThreadTest. (Either in the main, or in the SubTest constructor)

Since the class is abstract and will always be extended, to me it's more likely to put the .start in the subclass.

Thank you, adding thread.start() at the end of SubTest's constructor seems to solve it nice and good without having to use a if-statements to check p.

Kalle Zander on JavaRanch said:

oxano on Codecall mentioned that due to Henry's good summary of how a object is instantiated its better to do the

thread.start();

in constructor of SubTest instead of the superclass ThreadTest.

Then if you make sure you put that start command of the thread in the very end of SubTest's constructor, you can be sure that step 1, 2 and 3 are done before the thread starting going, that in this case use a reference that might cause a NullpointerException if not initialized yet.

After testing that it worked fine. Just to make clear what changes I made:

abstract class ThreadTest implements Runnable
{
    Thread thread;
    
    ThreadTest()
    {
        thread = new Thread(this);
    }

    public void run()
    {
        while (true)
        {
            update();
            try { Thread.sleep(500); } catch (InterruptedException e) {}
        }
    }
    
    abstract void update();
    
    public static void main(String[] args)
    {
        new SubTest();
    }
}

class SubTest extends ThreadTest
{
    Person p = new Person();
    
    SubTest()
    {
        thread.start();
    }
    void update()
    {
        p.update();
    }
}

[Cander]

Paul Clapham on JavaRanch said:

Unfortunately the rule is: Don't call a method which can be overridden from a constructor. If you do, then you risk the problem you had here, and it can happen even with no threading involved. Generally that means removing those method calls from the constructor and putting them in some new method which you would call after creating the object.

Kalle Zander on JavaRanch said:

You mean that you don't risk suffer from NullpointerExceptions if you add

thread.start();

(which is putted in a method we can call "start") after

new SubTest();

in main? I understand that its not because of the threading in this case you do so, but generally calling methods from above in a constructor. Well, if I understand what you said correctly.

Like this then maybe? As I didn't need any constructor I removed them to save space, the compiler add empty ones invisible.

abstract class ThreadTest implements Runnable
{
    Thread thread = new Thread(this);

    public void run()
    {
        while (true)
        {
            update();
            try { Thread.sleep(500); } catch (InterruptedException e) {}
        }
    }
    
    abstract void update();
    abstract void start();
    
    public static void main(String[] args)
    {
        SubTest sub = new SubTest();
        sub.start();
    }
}

class SubTest extends ThreadTest
{
    Person p = new Person();
    
    void update()
    {
        p.update();
    }
    
    void start()
    {
        thread.start();
    }
}

As he says that maybe its a bad way doing it by adding thread.start() in the constructor of SubTest because it can cause other problems in other cases. Above I tried an other version of the program calling thread.start() completely after the SubTest object was instantiated in main. As you already mentioned oxano by the way yeah.

[wim DC]

If you don't need the thread object later, to stop it for example, you can write it on 1 line :)

New SubTest().start();

[Cander]

oxano said:

If you don't need the thread object later, to stop it for example, you can write it on 1 line :)

New SubTest().start();

Yes of course, that's smart

EJP on Oracle forums said:

I don't get the point of making it abstract in the base class and implementing it in the derived class when it is the base class that has the Thread member. That's just pointless complication. The main thing is to get Thread.start() away from the constructors and you've done that.

812278 on Oracle forums said:

Oh ok, so the abstract start method isn't really necessary?

abstract class ThreadTest implements Runnable
{
    Thread thread = new Thread(this);

    public void run()
    {
        while (true)
        {
            update();
            try { Thread.sleep(500); } catch (InterruptedException e) {}
        }
    }
    
    abstract void update();
    
    public static void main(String[] args)
    {
        new SubTest().thread.start();
    }
}

class SubTest extends ThreadTest
{
    Person p = new Person();
    
    void update()
    {
        p.update();
    }    
}

Optimized the code a bit I think.