notion-devel

[Notion-devel] Object inheritance

[Arnout E. <no...@bz...>]

Hi,

I think the way inheritence and upcasting are done in Notion is a bit 
inconsistent and confusing, and would like to propose a change.

Classes in notion's OO are declared like this:

DECLCLASS(WScreen){
    WMPlex mplex;
    int id;
    ...
}

Basically this introduces a 'WScreen' type, and the first element of the 
struct holding its data is the supertype, in this case 'WMPlex'.

In code, whenever you have a WScreen* and need a WMPlex, we use either casts:

  mplex_map((WMPlex*)scr);
  ((WWindow *)(rw->scr))->win=None;

or look at the first field:

  mplex_map(&scr->mplex);
  rw->scr.mplex.win.win=None;

There are advantages and disadvantages to both calling conventions: the former
variation has the advantage that it is usually fairly short, and it's 
immediately clear that the called function operates on a WMPlex/WWindow. It 
has the disadvantage that you have to use a typecast, which the compiler 
cannot verify is correct.

The latter variation has the advantage that it's type-safe. The disadvantage is
that it is not immediately clear whether scr 'is an mplex' or 'has an mplex'. 
The second example makes this extra clear: the mplex 'is a' win which 'has a' 
win.

To mitigate the latter, I think it'd be neat to adopt the convention to always
call the first field 'super'. This will lead to:

  mplex_map(&scr->super);
  rw->scr.super.super.win=None;

The way I look at it now, we should pick one of the following conventions:

(a) always use typecasts, never use the first field of the struct explicitly
(b) always use the first field of the struct
(c) use the first field of the struct if we need the direct parent, use a 
  typecast when you need to go higher up the hierarchy.

in cases 'b' and 'c', we should decide whether to rename the first field to 
'super'.

What would you guys prefer? 

I'm still undecided - imho either of those choices is better than the current 
inconsistency though ;)


Kind regards,

Arnout

Re: [Notion-devel] Object inheritance

[<eb...@dr...>]

I like the .super idea for calling immediate ancestor. What is the
situation about other ion3 clones? Because mass renaming would harden
any porting of patches between ion3 clones (if there are any). I'm
not decided between (b) and (c). Maybe (c) is the most readable.

We may also consider to provide macros (with (runtime) checks in
future), similar to how objects are done in glib. This will be the
(a) solution, but wrapped into macros. With the option to implement
the checks later.


On Sat, 9 Apr 2011 17:15:25 +0200
Arnout Engelen <no...@bz...> wrote:
> Hi,
> 
> I think the way inheritence and upcasting are done in Notion is a bit 
> inconsistent and confusing, and would like to propose a change.
> 
> Classes in notion's OO are declared like this:
> 
> DECLCLASS(WScreen){
>     WMPlex mplex;
>     int id;
>     ...
> }
> 
> Basically this introduces a 'WScreen' type, and the first element of
> the struct holding its data is the supertype, in this case 'WMPlex'.
> 
> In code, whenever you have a WScreen* and need a WMPlex, we use
> either casts:
> 
>   mplex_map((WMPlex*)scr);
>   ((WWindow *)(rw->scr))->win=None;
> 
> or look at the first field:
> 
>   mplex_map(&scr->mplex);
>   rw->scr.mplex.win.win=None;
> 
> There are advantages and disadvantages to both calling conventions:
> the former variation has the advantage that it is usually fairly
> short, and it's immediately clear that the called function operates
> on a WMPlex/WWindow. It has the disadvantage that you have to use a
> typecast, which the compiler cannot verify is correct.
> 
> The latter variation has the advantage that it's type-safe. The
> disadvantage is that it is not immediately clear whether scr 'is an
> mplex' or 'has an mplex'. The second example makes this extra clear:
> the mplex 'is a' win which 'has a' win.
> 
> To mitigate the latter, I think it'd be neat to adopt the convention
> to always call the first field 'super'. This will lead to:
> 
>   mplex_map(&scr->super);
>   rw->scr.super.super.win=None;
> 
> The way I look at it now, we should pick one of the following
> conventions:
> 
> (a) always use typecasts, never use the first field of the struct
> explicitly (b) always use the first field of the struct
> (c) use the first field of the struct if we need the direct parent,
> use a typecast when you need to go higher up the hierarchy.
> 
> in cases 'b' and 'c', we should decide whether to rename the first
> field to 'super'.
> 
> What would you guys prefer? 
> 
> I'm still undecided - imho either of those choices is better than the
> current inconsistency though ;)
> 
> 
> Kind regards,
> 
> Arnout


-- 
                                 Tomáš 'ebík' Ebenlendr
                                 PF 2011.27372038305

Re: [Notion-devel] Object inheritance

[Etan R. <de...@un...>]

I think renaming to "super" is a pointless idea. Either you know what type
the parent is, in which case typing its name instead of super isn't a big
deal; or you don't, in which case you can't use super anyway since you
can't know what to cast it to.

It also doesn't tell you anything useful, since the immediate super
relationship isn't particularly helpful whereas the type of the member
actually is (especially if you are simply passing it to another function
which expects a certain type).

Consider:
    WScreen *scr=current_screen();
    mplex_switch_next(&(scr->mplex));
versus
    WScreen *scr=current_screen();
    mplex_switch_next(&(scr->super));

Which one tells you immediately that you've gotten the argument type to
mplex_switch_next right and which one leaves you having to remember and or
guess?

Also consider your given example. In what way is
> 'rw->scr.super.super.win=None;'
superior to
> 'rw->scr.mplex.win.win=None;'
?

The super version reads less well (to me), gives me less information, and
at that point I would really want a cast (or cast macro) or something to
hide it (you probably want that in either case really).

What is the current inconsistency exactly? I think there are actually
valid reasons to use both the specific cast and the &(obj->mem)
indirection.

To my mind the cast is more appropriate for assignment to local variables
for continued use (it says "I know this X is a Y and I intend to use it as
such from now on") whereas the member indirection is useful for examples
like the above (it says "I have an X, which I know is a Y, and you need
just that bit of it").

I'm not certain we need both, mind you. I'd be fine standardizing on the
cast (I wouldn't standardize the other way though). I'd also be fine
adding typecast macros (the way glib does), like what ebik said (though
I'm not sure we can reasonably get runtime tests the way glib sometimes
does).

    -Etan

Re: [Notion-devel] Object inheritance

[Arnout E. <no...@bz...>]

On Tue, Apr 12, 2011 at 09:28:51AM -0400, Etan Reisner wrote:
> I think renaming to "super" is a pointless idea. Either you know what type
> the parent is, in which case typing its name instead of super isn't a big
> deal; or you don't, in which case you can't use super anyway since you
> can't know what to cast it to.

Indeed it is not useful when writing code, but imho it is useful when reading 
code.

> It also doesn't tell you anything useful, since the immediate super
> relationship isn't particularly helpful whereas the type of the member
> actually is (especially if you are simply passing it to another function
> which expects a certain type).
> 
> Consider:
>     WScreen *scr=current_screen();
>     mplex_switch_next(&(scr->mplex));
> versus
>     WScreen *scr=current_screen();
>     mplex_switch_next(&(scr->super));
> 
> Which one tells you immediately that you've gotten the argument type to
> mplex_switch_next right and which one leaves you having to remember and or
> guess?

In this case, when reading this code, I actually do think the second variation
is more readable: you know the code compiles, you can easily guess 
'mplex_switch_next' will take an mplex argument, and the 'super' tells you 
you're calling 'mplex_switch_next' on the screen itself instead of on some 
mplex property of the screen.

> Also consider your given example. In what way is
> > 'rw->scr.super.super.win=None;'
> superior to
> > 'rw->scr.mplex.win.win=None;'
> ?
> 
> The super version reads less well (to me), gives me less information, and
> at that point I would really want a cast (or cast macro) or something to
> hide it (you probably want that in either case really).

Again, while I agree '.super.super' does not look very elegant, you *can* see
at a glance that 'win' is a property of the screen itself (via superclasses)
rather than a property of a property of a property of the screen. The types of
the superclasses in the second variation add confusion rather than information 
imho. 

I think I would prefer 'normal' casts over glib-style macro-casts, but both
of these throw the static typechecking out of the window, which the use of
members (named 'super' or otherwise) avoids.


Kind regards,

Arnout