java-gnome-hackers

[java-gnome-hackers] Thoughts about TreeStore

[Vreixo F. L. <met...@ya...>]

Hi!

During my work on TreeStore I've faced some problems I
want to discusse with you.

First of all, there is the model field in TreeIter. It
has a problem. Signals like TreeView.ROW_EXPANDED
return a TreeIter, which is instantiated by generic
signal handler that, obviously, do not know how to
initialize the model field. With a simple
implementation, this converts the TreeView parameter
on the signal handled in a completelly useless
parameter. The solution I've taken is add a package
protected setModel to TreeIter, together with removing
of the "final" qualifier in such field. That way, we
can implement a custom signal handler (of course,
private and hidden to the user), that initializes the
missing model field (see attached treestore.patch).
Ok, solved in a quite good way. But a bit ugly, sure.
The model field increments the implementation effort.
Not a problem, actually, if such field really provides
a better API. In my opinion, this is the case. Having
an iterNext() without the need of a model parameter is
sure a good thing. However, given we need extra effort
to have the model parameter in TreeIter, why don't we
take more advantage for each. Of course, I'm
presenting again my old proposal for having the
setValue/getValue function in TreeIter instead of of
TreeModel (see attached itervalues.diff).
This allow to set values of a row without having the
model, and it is useful, for example, to access the
values of a row in a signal handler without needing to
take a reference to the TreeModel. It is already on
TreeIter? Why force users to add extra code to get it?

------

Another question is the way CellRenderers are created.
They always take a CellLayout (ie, a TreeViewColumn)
as a parameter. I'm not against this, but what I don't
like is that they're always added to the given
CellLayout with the expand property set to true. Maybe
I'm missing something (help me!), but it seems it
can't be changed later. If so, this is a design
pitfall. Reason? Think about the left panel of an app
like nautilus (the tree of directories). It is a
TreeView with a single TreeViewColumn, with two
CellRenderers, one for the icon, one for the directory
name. If we set the expand property of the icon to
true (as it seems forced by java-gnome), when the
TreeView gets expanded it appear an ugly white space
between the icon and the dir name. This is ugly. How
can I handle this properly? Is it possible with
current design? If not, we should change this, maybe
with an alternate constructor that takes an additional
"boolean expand" parameter.

Cheers,
Vreixo



      Abra sua conta no Yahoo! Mail, o único sem limite de espaço para armazenamento!
http://br.mail.yahoo.com/

Re: [java-gnome-hackers] Thoughts about TreeStore

[Andrew C. <an...@op...>]

I am subscribed to the java-gnome-hackers mailing list. You don't need
to send messages To me and Cc list. Just send it To the list.

AfC
Berlin

Re: [java-gnome-hackers] Thoughts about TreeStore

[Andrew C. <an...@op...>]

Hi Vreixo,

Sorry I didn't reply earlier. Thanks for politely nudging me to ask for
some design calls. Let's see:

On Sat, 2008-03-15 at 15:24 -0300, Vreixo Formoso Lopes wrote:
> First of all, there is the model field in TreeIter. It
> has a problem. Signals like TreeView.ROW_EXPANDED
> return a TreeIter, which is instantiated by generic
> signal handler that, obviously, do not know how to
> initialize the model field.

Right.

So we both know the usual way to engineer around this. Before we just
blindly head off down that road, it occurs to me to just copy in the
comment that's around the current TreeIter protected <init>(long)
constructor.

    /*
     * The protected constructor was originally removed entirely, but it turns
     * out we need it for signals like ROW_ACTIVATED. This is a problem,
     * because it means we'd have to jump through *many* hoops to populate the
     * model field. Instead, we just cripple this TreeIter as far as iterating
     * goes but that's ok because you never need to iterate from a TreeIter in
     * a signal callback - it's just used to point at a row, done.
     */
    protected TreeIter(long pointer) {
        super(pointer);

        model = null;
    }

So having thought about this some more, I kinda still feel that this
argument holds. TreeIter-as-thing-that-points-at-a-row and
TreeIter-as-iterating-mechanism are really two entirely different use
cases. It's unfortunate that the same struct is used for both.

More generally, I have just been burned by iterating via TreeIter and
changing something (which I thought harmless) causing the TreeIter to
fail. (solving this is either going to mean using a TreeRowReference[]
and looping over it, or TreeModelSort. Not sure yet). Anyway, I'm even
less impressed with TreeIter than I was.

> Ok, solved in a quite good way. But a bit ugly, sure.

No, it's what's necessary - if one assumes that we can't "cripple" the
TreeIters in such cases.

So here's the question: when you get a ROW_EXPANDED event, do you need
to point at a row [ie call model.getValue() with it] or do you then need
to iterate over the model?

I've never seen the later case **from that iterator** - I _have_
subsequently called getIterFirst() and started from the top - but not
from the original TreeIter supplied by the callback.

All in all, this is a case of C-API-convenience vs
essence-of-the-toolkit. The trick is to figure out which one.

Have a think about the question I asked, and we'll chat again in a few
days.

AfC
Vancouver

Re: [java-gnome-hackers] Thoughts about TreeStore

[Vreixo F. L. <met...@ya...>]

> So here's the question: when you get a ROW_EXPANDED
> event, do you need
> to point at a row [ie call model.getValue() with it]
> or do you then need
> to iterate over the model?

good question! I think iteration from a arbitrary row
is not an use case. Usually you catch ROW_EXPANDED
just to get some of the row values (i.e calling
model.getValue()). But the question is: is that a
problem for us? Can we assume the user is not going to
iterate, and thus leave model to null? In my opinion
this is a bad solution, because in that case we have a
set of TreeIter methods that sometimes work, sometimes
not! It is hard for the developer to figure out in
which situations it is valid or not, even if we
carefully document that. 

In my opinion, we should try to keep the same behavior
for all iterators, despite of where they come from.
Anyway, it is not hard to achieve, it is implemented
in my branch!

And note that other methods that need the model, such
as children(), are really useful in a ROW_EXPANDED
handler, so...

Cheers,
Vreixo




      Abra sua conta no Yahoo! Mail, o único sem limite de espaço para armazenamento!
http://br.mail.yahoo.com/