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)
|