This is all discussion about how many angels can fit on the head of a 
pin, but I think you may have discounted the time/speed tradeoff 
involved here.  Most Simple portrayal classes take up only about 100 
bytes.  SimpleEdgePortrayal2D takes up substantially more because it 
allocates a Font instance; but it's still pretty dang small, probably 
well under 1K.  Your application is going to have only a small number of 
field portrayals (or "non-simple" portrayals) -- say at most 20 -- so 
you're talking about 20K.

On the other hand, there *is* a disadvantage to dynamically allocating 
the default portrayal: you have to do an extra if statement and an 
instance variable load each time.  That'd be minor too were it not for 
the fact that defaultPortrayal() is called a zillion times.  So it's 
slightly non-minor.  Still not *major*, but I imagine its value 
outweighs the 20K max you've lost.  :-)

At any rate, filling slots instead of leaving them null isn't bad coding 
style by any means: quite a number of major OO programming languages 
encourage it, because null checks are the spawn of the devil when it 
comes to debugging.  So unless there's a programmatic disadvantage 
incurred (making it hard to do X perhaps), my inclination is to leave 
well enough alone in this case.

Sean



Stuart Rossiter wrote:
> All,
> 
>    Nothing major, but just a flaw I came across when writing some custom
> portrayals. When they're instantiated, all the MASON non-simple
> portrayal classes instantiate default simple portrayal instance fields,
> rather than doing a lazy instantiation on request
> 
> e.g. NetworkPortrayal2D has:
> 
> SimpleEdgePortrayal2D defaultPortrayal = new SimpleEdgePortrayal2D();
> public Portrayal getDefaultPortrayal() { return defaultPortrayal; }
> 
> instead of, say:
> 
> SimpleEdgePortrayal2D defaultPortrayal = null;
> public Portrayal getDefaultPortrayal() {
>     if (defaultPortrayal == null) {
>         defaultPortrayal = new SimpleEdgePortrayal2D();
>     }
>     return defaultPortrayal;
> }
> 
> Not a biggie, but means that all non-simple portayals carry this
> unnecessary baggage if you're not using the default.
> 
> Note that there are some backwards compatibility issues with fixing it,
> since subclasses might use direct defaultPortrayal access to change the
> default portrayal (and all MASON's own subclasses do, such as
> HexaSparseGridPortrayal2D for SparseGridPortrayal2D). So
> getDefaultPortrayal as above wouldn't work properly for subclasses
> unless they overrode this accessor method as well.
> 
> A better (and more OO purist) alternative to force the change would be
> something like the below:
> 
> // Force subclasses to change if they were accessing this directly
> // before
> 
> private SimpleEdgePortrayal2D defaultPortrayal = null;
> 
> // Force this not to be overrideable (final)
> 
> public final Portrayal getDefaultPortrayal() {
>     if (defaultPortrayal == null) {
>         defaultPortrayal = createDefaultPortrayal();
>     }
>     return defaultPortrayal;
> }
> 
> // Subclasses should override this if using a new default portayal
> // class
> 
> protected SimpleEdgePortrayal2D createDefaultPortrayal() {
>     return new SimpleEdgePortrayal2D();
> }
> 
> Note that, with Java 1.5, you can override to return a subclass of
> SimpleEdgePortrayal2D explicitly, instead of only a
> SimpleEdgePortrayal2D (this is apparently called a covariant return).
> 
> e.g. this, in a subclass, wouldn't give a compile error on Java 1.5:
> 
> // Override with explicitly returned subtype: Java 1.5 only
> // (where SpecialSimpleEdgePortrayal2D extends SimpleEdgePortrayal2D)
> 
> protected SpecialSimpleEdgePortrayal2D createDefaultPortrayal() {
>     return new SpecialSimpleEdgePortrayal2D();
> }
> 
> cf.
> 
> // Version for Java < 1.5
> 
> protected SimpleEdgePortrayal2D createDefaultPortrayal() {
>     return new SpecialSimpleEdgePortrayal2D();
> }
> 
> This avoids callers of getDefaultPortrayal on the subclass having to do
> explicit casts if they need the functionality of the subclassed
> SimpleEdgePortayal2D (and makes subclass documentation much clearer).
> Sorry, a bit of a tangent (esp. for MASON itself as it's Java 1.3
> compatible), but an interesting feature of 1.5 that I hadn't come across
> until now, and was burning to communicate ;-)
> 
> Appreciate that the benefit may be (far!) outweighed by the
> compatibility issues, but don't know MASON's stance on compatibility vs.
> performance/correctness (for example, I assume that MASON's support down
> to Java 1.3 is a conscious compatibility design decision).
> 
> Regards,
> Stuart
> 
> (Stuart Rossiter - University of Strathclyde)