Skip to main content

API design experiments: "close-in" Highlighter/Predicate api

12 replies [Last post]
Anonymous

Beware, this is going to be one of those thinking-while-writing posts,
getting longer and longer. Just skip if you don't feel like it :-)

Reading Practical API design (Tulach), there are two rules that get
hammered into the reader's brain (apart from the "published api is like
a star - once discovered it's there foreever", which we ... ehem ...
don't follow, excusing ourselves with the "labs" character)

- api is communication
- hide and close and make final whatever you can get away with

Remembering that occasionally developers seem to have problems with the
Highlighter/Predicate pair (which does what?) it looks like the "api
message" might not be clear enough. The message is:

- a highlighter changes visual properties ("decorates") of a component
- a highlightPredicate decides whether or not a highlighter should be
turned on, based on the state of an ComponentAdapter

The confusion might come from the fact that both take the same
parameters (component and adapter) in the methods which clients must
implement (yeah, Karl, I know you nudged me to change that for ages -
some things take a bit longer to crack in :-)

<br />
// HighlightPredicate</p>
<p>public boolean isHighlighted(Component, ComponentAdapter)</p>
<p>// AbstractHighlighter</p>
<p>protected void abstract doHighlight(Component, ComponentAdapter)<br />
protected boolean canHighlight(Component, ComponentAdapter)<br />

Okay, so I started a little experiment (the old physicist inside
rejoycing ) about how that could be better focused. And at the same
time fixing it down by making more methods final.

<br />
// HighlightPredicate</p>
<p>public boolean isHighlighted(ComponentAdapter);</p>
<p>// AbstractHighlighter</p>
<p>// nail it down<br />
public final Component highlight(Component, ComponentAdapter)</p>
<p>// focus attention:<br />
// the only methods subclasses can/must override<br />
// main responsibility: component<br />
protected abstract void doHighlight(Component)<br />
protected boolean canHighlight(Component)<br />

All our pre-defined (and all examples) happily live with the slimmer
method signature, no problem there. First thingy that barked was
ColorHighlighter - now exposing its slight dirtyness (as Karl never
ceased to point out :-) when it comes to selection : if all decisions
should be handled by the predicate, selected should be handled there as
well. Then client code would have to care about it explicitly, like

<br />
// unselected, same as now with null selection colors<br />
unselectedHL= new ColorHighlighter(HighlightPredicate.UN_SELECTED,<br />
   mybackground, myforeground);<br />
// select as well<br />
selectedHL= new ColorHighlighter(HighlightPredicate.SELECTED,<br />
  selectionBackground, selectionForeground);<br />
highlighterClient.setHighlighters(unselectedHL, selectedHL)<br />

This is easier to communicate as well, as now we have no exception to
the "predicate decides" rule, one thing less to learn. So we are on the
right path ... until ... CompoundHighlighter: it delegates the actual
highlighting to its contained highlighters. This is done in the
doHighlight (we want super's check for a constraint from the
predicate/or self), which now no longer has the adapter as parameter.
And painted ourselves into a corner, as the highlight method which has
it is final.

At this point, the options are

- remove the final and re-implement in the subclass
- add the adapter again to the doHighlight

Gut feeling is that the first is the less-invasive, so do it:

<br />
// AbstractHighlighter</p>
<p>public Component highlight(Component, ComponentAdapter) {<br />
   // duplicate logic<br />
    if (canHighlight(component) &&<br />
      getHighlightPredicate().isHighlighted(adapter)) {<br />
      for each:<br />
         component = highlighter.highlight(component, adapter);<br />
   }<br />
   return component;<br />
}</p>
<p>// empty implementation, not used<br />
public Component doHighlight(Component) {<br />
   return component;<br />
}<br />

a slight smell is the empty implementation of the "normal" highlight
implementation method. Let's see ...

Next problem was the PainterHighlighter: it overrides highlight for
internal bookkeeping (needs to set a flag to ignore painter's property
changes during the highlighting process), so it again looks as if a
final there wouldn't be such a good idea. Which leaves the "reduced"
signatures in doHighlight/canHighlight. Unfortunately, they bumped
against a wall with value-based highlights (which I consider one of the
nicest assets in highlighting): their whole point is that they must know
the _value_ Again, the only option would be to override the main
highlight method:

</p>
<p>    @Override<br />
    public Component highlight(Component, ComponentAdapter) {<br />
        // duplicate logic<br />
        if (canHighlight(component) &&<br />
          getHighlightPredicate().isHighlighted(adapter)) {<br />
             if (adapter.getValue() instanceof Number) {<br />
                component.setBackground(mapToColor(adapter.getValue());<br />
            }<br />
       }<br />
       return component;<br />
    }</p>
<p>// empty implementation, not used<br />
public Component doHighlight(Component) {<br />
   return component;<br />
}<br />

And again sniffing at the empty implementation. Turns out that code
similar to that is necessary for all decoration effects which are
relative to _adapter_ state. And many in my more advanced examples are.
Which leaves as the last candidate the canHighlight - guess like you
most probably already did - which is more useful with both params as
well: that's where the instanceof Number check above could be done.
Arguably doable by a predicate - but wouldn't rely on that, as the
highlighter must protect itself.

My conclusion:

- the AbstractHighlighter api is about where it should be (with adding a
final to the get/set predicate)
- maybe change the semantics of a colorHighlighter and add a un/selected
predicate? Could do by adding a new class to not break anything.
- the predicate could do without the component. Which would break any
many apps out in the wild, so I would tend to not do it.

Comments, please?

Jeanette

PS: thanks for your patience :-)

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
kschaefe
Offline
Joined: 2006-06-08
Points: 0

> > - hide and close and make final whatever you can
> get
> > away with
> >
> Don't like this.
> I would say "hide and close and make final only what
> you have to".
>
> Why make something final if you don't have to? I
> thought one of the main points of
> languages like Java was for classes to be
> extendible.

This sounds distinctly, like "why even have final?" final is another form of encapsulation that ensures that the abstractions that you provide are used as you intended. it's also something that you can remove without causing any issues, but something you can never add (without breaking the API). It's always best to make methods and classes final and let use-cases expand the API in a reasonable way. There is no way to know how other people are going to use your code, or even how you're going to use your code, but you do know (when you create ie) how you want it to be used. final helps express that design intention.

Personally, I feel that final is highly underused.

> I sometimes wonder even why we make a field private
> then supply getters and setters
> when there is no processing done in them. (I know
> javabeans needs them but thats a
> historical accident ;) )
> I guess you could argue for consistency, as sometimes
> there is processing done in the
> getters and setters.

What if you need to be able to veto some changes? What if that requirement comes late? If the field is public, how are you going to do that? getters and setters provide a defined abstraction for interacting with the object. As the object evolves, it allows you to alter the object without changing anything outside of the class.

Karl

rturnbull
Offline
Joined: 2005-08-27
Points: 0

> final is another form of encapsulation that ensures
> that the abstractions that you provide are used as
> you intended. it's also something that you can
> remove without causing any issues, but something you
> can never add (without breaking the API). It's
> always best to make methods and classes final and
> let use-cases expand the API in a reasonable way.
> There is no way to know how other people are going
> to use your code, or even how you're going to use
> your code, but you do know (when you create ie) how
> you want it to be used. final helps express that
> design intention.
>
> Personally, I feel that final is highly underused.
>

What's wrong with people using something you provided in ways you never
intended (or even envisaged). You should be flattered :)

> > I sometimes wonder even why we make a field
> private
> > then supply getters and setters
> > when there is no processing done in them. (I know
> > javabeans needs them but thats a
> > historical accident ;) )
> > I guess you could argue for consistency, as
> sometimes
> > there is processing done in the
> > getters and setters.
>
> What if you need to be able to veto some changes?
> What if that requirement comes late? If the field
> is public, how are you going to do that? getters
> and setters provide a defined abstraction for
> interacting with the object. As the object evolves,
> it allows you to alter the object without changing
> anything outside of the class.
>
> Karl

Good point!

kschaefe
Offline
Joined: 2006-06-08
Points: 0

> > Personally, I feel that final is highly underused.
> >
>
> What's wrong with people using something you provided
> in ways you never
> intended (or even envisaged). You should be flattered
> :)

The problem is that the unexpected use may be accidentally closed off in a later version. It was never part of the designer's intent and therefore no care would be taken to ensure that functionality worked correctly. The communication that a closed off API causes allows the developer to expand the code in ways that he understands and can maintain. The issue is really one of mainenance and growth.

Karl

mrmorris
Offline
Joined: 2006-07-26
Points: 0

> apart from the
> "published api is like
> a star - once discovered it's there foreever", which
> we ... ehem ...
> don't follow, excusing ourselves with the "labs"
> character)

Thanks goodness! After reading the first chapters full of praise for backwards compatibility and statements like "simplicity and elegance is not a goal at all" I felt very depressed and was about ready to give up my profession.

/Casper

rturnbull
Offline
Joined: 2005-08-27
Points: 0

> - hide and close and make final whatever you can get
> away with
>
Don't like this.
I would say "hide and close and make final only what you have to".

Why make something final if you don't have to? I thought one of the main points of
languages like Java was for classes to be extendible.

I sometimes wonder even why we make a field private then supply getters and setters
when there is no processing done in them. (I know javabeans needs them but thats a
historical accident ;) )
I guess you could argue for consistency, as sometimes there is processing done in the
getters and setters.

Kleopatra

jdnc-interest@javadesktop.org schrieb:
>> - hide and close and make final whatever you can get
>> away with
>>
>>
> Don't like this.
> I would say "hide and close and make final only what you have to".
>
>

that's the application developer speaking - my quirk as well in that
role :-) With the hat of a framework developer it's different: whatever
is public must be maintained, now and forever. Preferably in a way that
doesn't break any code out in the wild. Which strictly speaking is
impossible for anything that's open to change, because the change is
unknown. Personally, I'm drawn between those opposing forces - with a
tendancy to open up more than I can possibly promise to maintain. So
giving developers enough rope to hang themselves if they use some part
in a way it isn't meant to be used.

One interesting aspect (loosely citing Jaroslav) of a more closed api is
that it can enhance communication between framework and application
developers: in a completely open api client developers just go and tweak
a given class in whatever way is necessary (legal or illegal in terms of
method/class contracts and invariants) until it suits their needs. In a
more closed environment, they are forced to talk back to the framework
about what they think is missing or not quite right to get it.
Obviously, they are still free to drop and go somewhere else, so
wouldn't try . A good communication infrastructure always is a must
(short response times, expert advise, seeing change actually happen
after reporting an issue ... ). Nevertheless, interesting thought, IMO.

> Why make something final if you don't have to? I thought one of the main points of
> languages like Java was for classes to be extendible.
>
>
Extension (aka inheritence) breaks encapsulation, so there are opposing
forces to weigh. Classes must be designed to be extensable, it doesn't
come for free but involves effort so that extenders don't hurt
themselves :-) And don't forget: extension is just one type of usage,
composition often is an alternative. One recurring theme in SwingX is to
reduce the necessity to subclass "big" units and instead support plug-in
with very lightweight "small" units. F.i. a XXComponentProvider - which
is implemented once per type of rendering component - is pluggable with
YYValues. The latter is so simple that implementing can't go much wrong.

> I sometimes wonder even why we make a field private then supply getters and setters
> when there is no processing done in them. (I know javabeans needs them but thats a
> historical accident ;) )

won't follow you there

Cheers
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08
Points: 0

Jeanette,

> - api is communication
> - hide and close and make final whatever you can get
> away with
>
> So we are on the
> right path ... until ... CompoundHighlighter: it
> delegates the actual
> highlighting to its contained highlighters. This is
> done in the
> doHighlight (we want super's check for a constraint
> from the
> predicate/or self), which now no longer has the
> adapter as parameter.
> And painted ourselves into a corner, as the highlight
> method which has
> it is final.
>
> At this point, the options are
>
> - remove the final and re-implement in the subclass
> - add the adapter again to the doHighlight

The other option is to not extend AbstractHighlighter. You seemed to have tripped over a common stumbling block in the Interface with Abstract Implementation paradigm. First, the interface defines the contract, not the abstract class. The abstract class is a stub for handling common cases. Most, but not all interface implementations should extend the abstract class. If everything can extend the abstract class, is there a need for the interface? (Not considering the testing angle for this.) Well, in a SPI, you typically define abstract classes because you want to define (at least part of) the actual behavior. Here, we expect that as long as the Highlighter implements the interface method that the contract is fulfilled. The abstract class, then, should just be for the common case. I would suggest that the CompountHighlighter is not the common cases and should be implemented as a one-off. This also gives outside developers and idea of when not to use AbstractHighlighter.

> Next problem was the PainterHighlighter: it overrides
> highlight for
> internal bookkeeping (needs to set a flag to ignore
> painter's property
> changes during the highlighting process), so it again
> looks as if a
> final there wouldn't be such a good idea.

After a quick look (and without producing code), it seems to me that PainterHighlighter could set the flag in the doHighlight method.

> Unfortunately, they bumped
> against a wall with value-based highlights (which I
> consider one of the
> nicest assets in highlighting): their whole point is
> that they must know
> the _value_

So, do we really need to pass in the entire adapter? Can we pass in just the value? Do we want to have two AbstractHighlighter chains: one that is value-aware and one that is not?

> Which leaves as the last candidate the canHighlight -
> guess like you
> most probably already did - which is more useful
> with both params as
> well: that's where the instanceof Number check above
> could be done.
> Arguably doable by a predicate - but wouldn't rely on
> that, as the
> highlighter must protect itself.

Again, this seems value-related, can't we just pass the value in?

Karl

Kleopatra

Karl,

how did I knew you would look into this, cool
> The other option is to not extend AbstractHighlighter. You seemed to have tripped over a common stumbling block in the Interface with Abstract Implementation paradigm. First, the interface defines the contract, not the abstract class. The abstract class is a stub for handling common cases. Most, but not all interface implementations should extend the abstract class. If everything can extend the abstract class, is there a need for the interface? (Not considering the testing angle for this.) Well, in a SPI, you typically define abstract classes because you want to define (at least part of) the actual behavior. Here, we expect that as long as the Highlighter implements the interface method that the contract is fulfilled. The abstract class, then, should just be for the common case. I would suggest that the CompountHighlighter is not the common cases and should be implemented as a one-off. This also gives outside developers and idea of when not to use AbstractHighligh!
> ter.
>
>

Hmm .. that's a good point. My reason to make it extend the
AbstractHighlighter was that I wanted it predicate-controlled as well,
so its "compoundness" is on that property too. This allows me a
coase-grained turn on/off a whole branch of highlighters in addition to
the smaller-grained per-highlighter on/off.

> After a quick look (and without producing code), it seems to me that PainterHighlighter could set the flag in the doHighlight method.
>
>

yes, that's true (and was the first solution when I stumbled over the
issue). As _all_ implementations (at least all I played with) must
think about that - and tend to forget it - I moved it up the hierarchy.
In this way its completely transparent to subclasses.

>> Unfortunately, they bumped
>> against a wall with value-based highlights (which I
>> consider one of the
>> nicest assets in highlighting): their whole point is
>> that they must know
>> the _value_
>>
>
> So, do we really need to pass in the entire adapter? Can we pass in just the value? Do we want to have two AbstractHighlighter chains: one that is value-aware and one that is not?
>
>

in my experiments, it's not just the value. See f.i. the
ToolTipHighlighter in the swinglabs-demos: it compounds a tooltip for a
column from content of cells in the same row. Or the per-row image
highlighter: it decorates the cell with a subimage which needs access to
the cell width. Or the ZoomHighlighter (in the demos hierarchy
BarRendererDemo) which adjusts the font depending on the rowheight of
the adapter's owner. Similar for the canHighlight.

As to multiple AbstractHighlighter chains: the semantic difference of
adapter-aware vs. adapter-agnostic feels a bit small for me. Would hate
to replicate something like the Painter hierarchy ;-)

Good brain food, keep it coming!

Thanks
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08
Points: 0

Jeanette,

> how did I knew you would look into this, cool
Well, I love these types of discussions. A couple of quick responses.

> Hmm .. that's a good point. My reason to make it
> extend the
> AbstractHighlighter was that I wanted it
> predicate-controlled as well,
> so its "compoundness" is on that property too. This
> allows me a
> coase-grained turn on/off a whole branch of
> highlighters in addition to
> the smaller-grained per-highlighter on/off.

Don't forget that you can have a package-private class that allows the highlight method to be overridden and only expose the AbstractHighlighter with a final. The API design is limited for the general case, but extensible for the implementors. We're likely to be the ones that need that flexibility of overriding highlight.

> in my experiments, it's not just the value. See f.i.
> the
> ToolTipHighlighter in the swinglabs-demos: it
> compounds a tooltip for a
> column from content of cells in the same row. Or the
> per-row image
> highlighter: it decorates the cell with a subimage
> which needs access to
> the cell width. Or the ZoomHighlighter (in the demos
> hierarchy
> BarRendererDemo) which adjusts the font depending on
> the rowheight of
> the adapter's owner. Similar for the canHighlight.

Will dig into those examples later, but it would be nice if we could limit the scope somewhat. Any thoughts on that, or do you believe that there's no way to limit any of the adapter's power?

> As to multiple AbstractHighlighter chains: the
> semantic difference of
> adapter-aware vs. adapter-agnostic feels a bit small
> for me. Would hate
> to replicate something like the Painter hierarchy ;-)

Well, it's probably not ideal unless we feel that there is enough semantic difference and that exposing that difference is worthwhile to the API users. I'm inclined to believe that the difference is insignificant, but worth mentioning nonetheless.

Karl

Kleopatra

Karl,

>
>
>> how did I knew you would look into this, cool
>>
> Well, I love these types of discussions.
>
>

and I love the fact that you love them as well :-)

> Don't forget that you can have a package-private class that allows the highlight method to be overridden and only expose the AbstractHighlighter with a final. The API design is limited for the general case, but extensible for the implementors. We're likely to be the ones that need that flexibility of overriding highlight.
>
>

hmm .. don't quite understand (getting tired shows after a longish day),
could you give me a short code-sketch?
> Will dig into those examples later, but it would be nice if we could limit the scope somewhat. Any thoughts on that, or do you believe that there's no way to limit any of the adapter's power?
>

(adapter's power? assuming you mean the highlighter's power - if not, just ignore) well, I started the day with being quite confident that we could limit the scope, hide some details - after spending the day with trying to do so, I tend to think that we shouldn't: looks like we need the full flexibility for everything that's not a plain one simple property decoration.

> Well, it's probably not ideal unless we feel that there is enough semantic difference and that exposing that difference is worthwhile to the API users. I'm inclined to believe that the difference is insignificant, but worth mentioning nonetheless.
>
>

okay, agreed.

Have a nice day, I'll switch off now (getting hungry :-)
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net

kschaefe
Offline
Joined: 2006-06-08
Points: 0

Jeanette,

> hmm .. don't quite understand (getting tired shows
> after a longish day),
> could you give me a short code-sketch?

Sure:
[code]
abstract class AbstractInternalHighlighter {
//protected methods etc.

public highlight(...) {
//calls other internal methods
}
}

public class AbstractHighlighter extends AbstractInternalHighlighter {
public final highlight(...) {
super.highlight(...);
}
}

public final CompoundHighlighter extends AbstractInternalHighlighter {
public highlight(...) {
//extra work
super.highlight(...);
//extra work
}
}
[/code]

This leverages code reuse (for us), but still provides a locked down API for developers.

> > Will dig into those examples later, but it would be
> nice if we could limit the scope somewhat. Any
> thoughts on that, or do you believe that there's no
> way to limit any of the adapter's power?
> >
>
> (adapter's power? assuming you mean the highlighter's
> power - if not, just ignore) well, I started the day
> with being quite confident that we could limit the
> scope, hide some details - after spending the day
> with trying to do so, I tend to think that we
> shouldn't: looks like we need the full flexibility
> for everything that's not a plain one simple property
> decoration.

Yep, I meant the Adapter. The Highlighter needs all of the Adapter and there's nothing that can be removed. You answered my question, even without understanding it. I wonder if we could seperate the context (cell-external data) and metadata from the ComponentAdapter? Hmm, still need to dig into those examples to formulate a better comment. Not enough time in the day....

Karl

Kleopatra

Karl,
> This leverages code reuse (for us), but still provides a locked down API for developers.
>
>

Thanks for the snippets - that's easiest to understand for me. But ...
honestly: nothing I would like to do. Probably too much for my app
developer's part of my soul :-)
>
>
> Yep, I meant the Adapter. The Highlighter needs all of the Adapter and there's nothing that can be removed. You answered my question, even without understanding it. I wonder if we could seperate the context (cell-external data) and metadata from the ComponentAdapter? Hmm, still need to dig into those examples to formulate a better comment. Not enough time in the day....
>
>

yeah, wondered about that as well - yet another story. Will summarize
what the adapter is/not doing one of these days (next week or so, will
be off-line for a while).

Cheers
Jeanette

---------------------------------------------------------------------
To unsubscribe, e-mail: jdnc-unsubscribe@jdnc.dev.java.net
For additional commands, e-mail: jdnc-help@jdnc.dev.java.net