Inheritance, inner classes, and access

[Eric Sosman]

First, I apologize in advance for the length of this
message.

I'm building a model that exports data for display and
manipulation through a separate GUI. When the GUI changes
one of the model's inputs, the model recalculates its outputs
and fires ChangeEvent notifications for any that change;
the GUI registers listeners that tell it when to update its
JTextFields, JRadioButtons, and so on.

The model can also be told to load an entire set of
data en masse from a Reader, so the "input" data items
can also be "outputs" during a load operation, and the GUI
can register listeners on them, too. It turns out that the
only difference between inputs and outputs is whether their
setValue() methods are private to the model or accessible
outside, and whether changing the value sparks recalculation.
"Almost no difference" makes me think "inheritance," so my
design thus far looks like

class Model {

class Value {
private Object value;
private void setValue(Object newValue) {
if (! newValue.equals(value)) {
value = newValue;
fireChangeEvent();
}
}
private void fireChangeEvent() {
// ... notify listeners
}
// ... additional methods and fields
}

class Variable extends Value {
void setValue(Object newValue) {
super.setValue(newValue);
recalculate();
}
}

// data for this model instance
final Variable input1 = new Variable();
final Variable input2 = new Variable();
// ...
final Value output1 = new Value();
final Value output2 = new Value();
// ...

private void recalculate() {
output1.setValue( ... );
output2.setValue( ... );
// ...
}
}

So far, so good. Exporting fields directly, even final
fields, might not be suitable for public classes, but these
are all package-private and fairly tightly associated with
the GUI (although carefully divided from it), so I don't
propose to fret about that set of issues.

But let's look again at that en-masse load operation. If
the loader calls setValue() on each input Variable as it's
encountered, we'll do a whole lot of recalculations and fire
a whole lot of ChangeEvents that we really don't want. After
all, a Model that has some Variables newly-loaded while others
still hold old values may well be in an inconsistent state, and
I'd rather not take the chance of confusing the GUI. So I'd
like to let the loader just "poke" the `value' elements of all
the Variables without going through the setValue() methods
(which would fire ChangeEvents at each call), and just fire a
whole bunch of ChangeEvents after all the loading is finished.

... but this runs into a small snag: The load() method
cannot just do `input1.value = loadedThing' because, as javac
so eloquently puts it, "value has private access in Model.Value".
(There's somewhat of a paradox here, because the `value' fields
of `output1' and `output2' *are* freely accessible within Model;
Value is inside Model, so even its private fields are visible.
Variable is a subclass of Value and might be expected to have a
more "intimate" relation with it than some unrelated sibling
class would, but no: Variable can't see `value' directly, and
code inside Model can't see `value' if it looks at it through
the subclass' distorting lens.)

I've thought of two ways to solve the problem. One is to
up-cast each Variable reference inside the load() method and
write `((Value)input1).value = loadedThing'. The other is to
split the setValue() method into two pieces, one to do the
setting of the new value and the other to fire ChangeEvents as
needed; the load() method would call the "silent setter," which
would be inaccessible from outside Model. I'm not really happy
with either alternative, and wonder (at long last, here's the
question): is there a better way to design this whole thing?

Thanks for your patience, and I look forward to your ideas.

 

[Monique Y. Mudama]

... but this runs into a small snag: The load() method cannot
just do `input1.value = loadedThing' because, as javac so
eloquently puts it, "value has private access in Model.Value".
(There's somewhat of a paradox here, because the `value' fields
of `output1' and `output2' *are* freely accessible within Model;
Value is inside Model, so even its private fields are visible.
Variable is a subclass of Value and might be expected to have a
more "intimate" relation with it than some unrelated sibling
class would, but no: Variable can't see `value' directly, and
code inside Model can't see `value' if it looks at it through
the subclass' distorting lens.)

You might want to read about the 'protected' scope instead of using
'private' for this.

 

[Chris Uppal]

Thanks for your patience, and I look forward to your ideas.

Hmm, some suggestions, in no special order, and not all independent of each
other.

Have only Variables in your model, but pass out references of type Value for
the readonly ones for the GUI to see.

Have a "global" flag in your model that all the Variables check before firing
notifications. Turn that off before doing a bulk update.

After a bulk update either fire a single "model changed" event which is
distinct from all the individual events, and to which the GUI code responds by
refreshing everything, or make each value holder trigger its normal
notification then unconditionally.

Alternatively (nstead of messing with a flag), when you do a bulk update rerun
your initialisation code to create new Variable objects, fill in the new
values, and then fire "model changed". The GUI responds by rerunning /its/
initialisation code to Observe the new value holders.

Alternatively take responsibility for the implementation of notification away
from the value holders and give it back to your model. It can then suppress,
postpone, or batch up, notifications as it sees fit. Eg keep all the
notifications (reified as proper objectgs) in a Set and until the end of a
batch update, and then only send out that Sets contents -- thus avoiding
duplicate notifications.

Personally, my inclination would be to take as much advantage of pre-existing
initialisation code (which perhaps would need some refactoring) as possible.

-- chris

 

[Roedy Green]

I've thought of two ways to solve the problem. One is to
up-cast each Variable reference inside the load() method and
write `((Value)input1).value = loadedThing'. The other is to
split the setValue() method into two pieces, one to do the
setting of the new value and the other to fire ChangeEvents as
needed;

The way I would do it is invent a boolean perhaps called suppressFire=
true that suppress the change events. The fire methods check this
variable before firing. ( Encapsulate in the obvious way ).

When you globally load you do suppressFire = true; change everything,
set suppressFire = false; then fire up some very global change event
that triggers a complete reorg/repaint.

I feel like I am stating the obvious and I must have missed the point
of your question.

 

[Eric Sosman]

Eric Sosman wrote On 10/03/05 16:52,:

First, I apologize in advance for the length of this
message. [...]

... and won't quote the whole thing here; see up-thread.

Thanks to Monique Mudama, Chris Uppal, and Roedy Green
for their helpful suggestions.

Monique suggested making the wrapped field in Value be
protected, which allows the subclass Variable to have easy
access to it. Unfortunately, protected also allows access
to any other class in the package, which goes against the
intent to keep the model and the GUI separate. I suppose I
could put the model and GUI in different packages, but then
I'd need to make a lot of methods public instead of package-
private. I'm a relative beginner, and am reluctant to make
things public unless I'm sure they'll behave well even if
abused, and (between you and me) most of this code assumes
a "well-intentioned" client.

Both Chris and Roedy suggested using a flag to suppress
the firing of ChangeEvents at inconvenient times. That'd
work, but I have a distaste for such flags: they tend to
proliferate and then to develop strange interactions with
each other, and I use them only in extremis.

Chris suggested populating the model only with Variables,
but exporting Value references for the "read-only" items.
I don't see how that solves my problem, but I may not have
grasped his intent; gotta ponder some more.

Chris also had a couple other suggestions whose implications
are not yet entirely clear to me; yet more pondering.

Thanks again for the advice.

 

[Roedy Green]

Both Chris and Roedy suggested using a flag to suppress
the firing of ChangeEvents at inconvenient times. That'd
work, but I have a distaste for such flags: they tend to
proliferate and then to develop strange interactions with
each other, and I use them only in extremis.

I don't understand your extreme distaste. A flag with a method to
test it is the technique Sun itself uses.

 

[Eric Sosman]

Roedy Green wrote On 10/04/05 13:44,:

I don't understand your extreme distaste. A flag with a method to
test it is the technique Sun itself uses.

My distaste isn't extreme, just an everyday distaste.
I've seen too many programs that became breeding grounds
for little one-off flags, and the experiences haven't
always been pleasant. I've even seen flags introduced to
modify the effects of other flags; you go from

fireChangeEvent();

to

if (! suppressFire)
fireChangeEvent();

to

if (! suppressFire
|| (fireStringChangesAnyhow
&& (newValue instanceof String)))
fireChangeEvent();

.... and sometimes even beyond! IMHO, an abomination like
this last example is evidence that fireChangeEvent() is
being called from the wrong place, and that there's
something askew in the architecture.

Sometimes, the "obvious" use of a flag can be wrong.
Here's an obvious use pattern:

suppressFire = true;
doThingsThatWouldNormallyFire();
suppressFire = false;

.... and here's a rewrite to fix one of its possible
problems:

boolean oldSuppressFire = suppressFire;
suppressFire = true;
doThingsThatWouldNormallyFire();
suppressFire = oldSuppressFire;

.... and here's yet another rewrite to fix still another
possible bug:

boolean oldSuppressFire = suppressFire;
suppressFire = true;
try {
doThingsThatWouldNormallyFire();
}
finally {
suppressFire = oldSuppressFire;
}

In my experience, if you've got a lot of flags like this
you'll inevitably find that some programmer somewhere will
forget to do this dance, or will do it incorrectly and
tread on his partner's toes.

Of course, you and I are skillful dancers, and so are
Monique and Chris: we're all Pavlovas of programming whose
attention to the dance never, er, flags. (Pardon me a
moment; I seem to have sprained my arm trying to pat myself
on the back.) Nonetheless I hesitate before introducing a
flag of this kind; I try to figure out what it is about my
architecture that makes the flag necessary; I try to find
a rearrangement of the existing architectural pieces that
makes it unnecessary.

And sometimes I go ahead and use a flag anyhow. It's
something of a last resort, but sometimes expediency trumps
my non-extreme distaste. My misplaste distaste, if you like.

 

[Chris Uppal]

Chris also had a couple other suggestions whose implications
are not yet entirely clear to me; yet more pondering.

One other approach that I clean forgot to mention -- and I don't know why
because this is what I would usually use. Don't apply bulk updates to "the"
Model; create a new Model with the new data and tell the GUI "Oi! There's a
replacement model -- use it!"

-- chris