Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.gui > #3876 > unrolled thread

Re: Avoiding NPEs caused

Back to article view | Back to comp.lang.java.gui

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Avoiding NPEs caused "Royan" <royan@THRWHITE.remove-dii-this> - 2011-04-27 15:47 +0000
    Re: Avoiding NPEs caused "Mark Space" <mark.space@THRWHITE.remove-dii-this> - 2011-04-27 15:47 +0000
      Re: Avoiding NPEs caused "Royan" <royan@THRWHITE.remove-dii-this> - 2011-04-27 15:47 +0000
        Re: Avoiding NPEs caused "Mark Space" <mark.space@THRWHITE.remove-dii-this> - 2011-04-27 15:47 +0000

#3876 — Re: Avoiding NPEs caused

  To: comp.lang.java.programmer
On 3 Á×Ç, 19:45, Lew <com.lewscanon@lew> wrote:
> Royan wrote:
> > This is just an example:
>
> > public class Model extends AbstractModel {
>
> ...
>
> Your example is incomplete.
> <http://pscode.org/sscce.html>
>
> Also, you failed to cite (copy and paste) the exception message.
>
> We need more information.
>
> > PS
> > There is a an erroneous cross-post in java.gui, please ignore it
>
> That was not a cross-post, that was a multi-post. šA cross-post shows all
> addressed groups in one message. šA multi-post shows the same or similar
> message independently as several messages, one per group. šCross-posting is
> better than multi-posting.
>
> If clj.gui and clj.programmer had different readerships, it would have been
> pointless to tell clj.programmer to ignore clj.gui, wouldn't it?
>
> --
> Lew

Hi Lew,

There is not much sense in stack trace, it's classic problem, i'm only
looking for the best solution. If stack really matters here's slightly
improved example that you can even run yourself and stack trace:

package test;

import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;

public class Model extends AbstractModel {
    private final PropertyChangeSupport propertyChangeSupport;

    public Model (Object source) {
        propertyChangeSupport = new PropertyChangeSupport(source);
    }

    @Override
    public void addPropertyChangeListener(PropertyChangeListener
listener) {
        propertyChangeSupport.addPropertyChangeListener(listener);
    }

    @Override
    public void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
        propertyChangeSupport.firePropertyChange(propertyName,
oldValue, newValue);
    }

    public static void main(String[] args) {
	new Model(new Object());
    }
}

abstract class AbstractModel {
    public AbstractModel() {
	indirectCall();
    }

    private void indirectCall() {
	setSomeValue(new Integer(1));
    }

    public void setSomeValue(Integer value) {
        firePropertyChange("someProperty", null, value);
    }

    public void addPropertyChangeListener(PropertyChangeListener
listener) {
	// some code
    }

    protected void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
        // some code
    }
}

Exception in thread "main" java.lang.NullPointerException
	at test.Model.firePropertyChange(Model.java:20)
	at test.AbstractModel.setSomeValue(Model.java:38)
	at test.AbstractModel.indirectCall(Model.java:34)
	at test.AbstractModel.<init>(Model.java:30)
	at test.Model.<init>(Model.java:9)
	at test.Model.main(Model.java:24)

---
 * Synchronet * The Whitehouse BBS --- whitehouse.hulds.com --- check it out free usenet!
--- Synchronet 3.15a-Win32 NewsLink 1.92
Time Warp of the Future BBS - telnet://time.synchro.net:24

[toc] | [next] | [standalone]

#3881

  To: comp.lang.java.programmer
Royan wrote:

> abstract class AbstractModel {
>     public AbstractModel() {
> 	indirectCall();
>     }
> 
>     private void indirectCall() {
> 	setSomeValue(new Integer(1));
>     }
> 
>     public void setSomeValue(Integer value) {
>         firePropertyChange("someProperty", null, value);
>     }

As Lew said, the chained call from the constructor is the problem. Never 
ever do this.  Calling any "foriegn" method too (like "indirectCall()" ) 
is broken because you don't know what they will call.  If they call a 
public, overriden method (which happens here), then you could be in a 
lot of trouble.

Joshua Bloch describes almost the exact code you have with a big "Don't 
Do This!" sign next to it.  It's Item 17: Design and Document for 
Inheritance or Else Prohibit It, in Effective Java 2nd edition.

A couple of classic solutions:  use composition instead of inheritance. 
  Use a static factory (maybe with a Strategy Pattern to "plug in" the 
exact Model you want).

Composition would use something like the Decorator Pattern.  Use a 
concrete class, DefaultModel, instead of an abstract class 
AbstractModel, and wrap the new class around the default one.

But we kinda don't have enough info to help you out of this, it's very 
much a design issue.  "indirectCall" is the problem, you can NEVER do 
this and expect it to work well.  It must go.  What are you really 
trying to do here?

---
 * Synchronet * The Whitehouse BBS --- whitehouse.hulds.com --- check it out free usenet!
--- Synchronet 3.15a-Win32 NewsLink 1.92
Time Warp of the Future BBS - telnet://time.synchro.net:24

[toc] | [prev] | [next] | [standalone]

#3887

  To: comp.lang.java.programmer
On 4 Á×Ç, 05:29, Mark Space <marksp...@sbc.global.net> wrote:
> Royan wrote:
> > abstract class AbstractModel {
> > š š public AbstractModel() {
> > š šindirectCall();
> > š š }
>
> > š š private void indirectCall() {
> > š šsetSomeValue(new Integer(1));
> > š š }
>
> > š š public void setSomeValue(Integer value) {
> > š š š š firePropertyChange("someProperty", null, value);
> > š š }
>
> As Lew said, the chained call from the constructor is the problem. Never
> ever do this. šCalling any "foriegn" method too (like "indirectCall()" )
> is broken because you don't know what they will call. šIf they call a
> public, overriden method (which happens here), then you could be in a
> lot of trouble.
>
> Joshua Bloch describes almost the exact code you have with a big "Don't
> Do This!" sign next to it. šIt's Item 17: Design and Document for
> Inheritance or Else Prohibit It, in Effective Java 2nd edition.
>
> A couple of classic solutions: šuse composition instead of inheritance.
> š Use a static factory (maybe with a Strategy Pattern to "plug in" the
> exact Model you want).
>
> Composition would use something like the Decorator Pattern. šUse a
> concrete class, DefaultModel, instead of an abstract class
> AbstractModel, and wrap the new class around the default one.
>
> But we kinda don't have enough info to help you out of this, it's very
> much a design issue. š"indirectCall" is the problem, you can NEVER do
> this and expect it to work well. šIt must go. šWhat are you really
> trying to do here?

OK, sorry for making that too confusing, my only intention was to
create a small test case and get things crystal-clear. I have faced
that in two cases.

First was when I derived from DefaultTableModel and overridden a
couple of its methods that make use of internal vectors.

And the second one (which has made me to start this thread) was when I
derived from the JDialog and overridden addXxxx and fireXxx methods.
Because this case is somewhat easier to reproduce I've written a very
small test case that demonstrates my original problem:

import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;

import javax.swing.JDialog;

class SampleDialog extends JDialog {
    private final PropertyChangeSupport propertyChangeSupport;

    public SampleDialog(JDialog owner) {
        super(owner);
        propertyChangeSupport = new PropertyChangeSupport(owner);
    }

    @Override
    public void addPropertyChangeListener(PropertyChangeListener
listener) {
        propertyChangeSupport.addPropertyChangeListener(listener);
    }

    @Override
    public void firePropertyChange(String propertyName, Object
oldValue, Object newValue) {
        propertyChangeSupport.firePropertyChange(propertyName,
oldValue, newValue);
    }
}

public class Test {
    public static void main(String[] args) {
        SampleDialog sd = new SampleDialog(new JDialog());
    }
}


The stack trace here:

Exception in thread "main" java.lang.NullPointerException
	at SampleDialog.firePropertyChange(Test.java:21)
	at java.awt.Container.setFocusTraversalPolicy(Container.java:3220)
	at sun.awt.SunToolkit.checkAndSetPolicy(SunToolkit.java:501)
	at java.awt.Dialog.<init>(Dialog.java:665)
	at java.awt.Dialog.<init>(Dialog.java:499)
	at javax.swing.JDialog.<init>(JDialog.java:409)
	at javax.swing.JDialog.<init>(JDialog.java:361)
	at javax.swing.JDialog.<init>(JDialog.java:336)
	at SampleDialog.<init>(Test.java:10)
	at Test.main(Test.java:28)

As you can see the root of the problem is that
SunToolkit#checkAndSetPolicy(SunToolkit.java:501) calls
setFocusTraversalPolicy which eventually causes NPE

I'm afraid that nothing can actually be done except for things that
have already been mentioned, but you might have another idea :)

---
 * Synchronet * The Whitehouse BBS --- whitehouse.hulds.com --- check it out free usenet!
--- Synchronet 3.15a-Win32 NewsLink 1.92
Time Warp of the Future BBS - telnet://time.synchro.net:24

[toc] | [prev] | [next] | [standalone]

#3889

  To: comp.lang.java.programmer
Royan wrote:

> And the second one (which has made me to start this thread) was when I
> derived from the JDialog and overridden addXxxx and fireXxx methods.
> Because this case is somewhat easier to reproduce I've written a very
> small test case that demonstrates my original problem:

Your original test case was excellent, no worries.  I think however now 
that we know you base class is JDialog... I'm not sure there's much to 
be done.  Apparently, JDialog is poorly designed for the type of 
inheritance you want to do.  So, you can't do that.

I'm not sure what's up with the property change listener.  I've never 
used one (directly) and I don't know why it's firing in the constructor. 
  There might be a better way of using the property change listener.

Some things you might consider:

1. Wrap the JDialog instead of extending it.

class SampleDialog {
   private JDialog jd;
   private PropertyChangeSupport pcs;

   //... etc.
}

2. Extend java.awt.dialog instead of JDialog.

3. Look into other ways to use property change listeners.

#3 seems like the first thing you should do.  While JDialog might not be 
the best class to extend, I don't see why you can't extend 
DefaultTableModel (although it may not be designed for inheritance 
either.  AbstractTableModel seems to be the class to extend.)  So you 
might be doing something iffy with the property change listeners here. 
I'm not really sure because I don't have any experience with them.

---
 * Synchronet * The Whitehouse BBS --- whitehouse.hulds.com --- check it out free usenet!
--- Synchronet 3.15a-Win32 NewsLink 1.92
Time Warp of the Future BBS - telnet://time.synchro.net:24

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.java.gui

csiph-web