On Fri, 5 Nov 1999, Alexander Larsson wrote:
> While I'm in complain mode i also like to say that i still *don't* like
> the idea of having very generic base elements and then limiting the
> visiblility or editability of their properties in base classes. This makes
> for overly complex base classes and is contrary to typical OO. Having a
> zigzagline that does rounded corner might look ok, but then someone wants
> it to support wavy lines too, which is possible after some work, but the
> code gets horribly comples (waved curves anyone?). Then people want more
> features leading to hard to understand unmaintainable code.
Hmmm... Perhaps you have missed one thing I said : just like I feel we
should not have distinct implementation every time a pixel is rendered
differently (see 'UML generalisation' vs 'UML realises'), we should not
reduce dia's object set down to four or five implementations ; when the
difference boils down to just one line type, I think it does make sense to
share the implementation. On the other hand, I agree with you that swiss
army knifes are unmaintainable ; typically, RoundedZigZagLine's draw
routine should be totally different from ZigZagLine's, and likewise for
WavyZigZagLine, so these three objects must be *at least* partially
forked. However, if I look at newzzline.cpp (cpp ?!?), there's still
plenty of code I think is useless -- useless because of C's lack of
built-in inheritance mechanism.
For instance, we'll find this, verbatim, in every orthconn-derived object :
static void
zigzagline_select(Zigzagline *zigzagline, Point *clicked_point,
Renderer *interactive_renderer)
{
orthconn_update_data(&zigzagline->orth);
}
Or this :
static real
zigzagline_distance_from(Zigzagline *zigzagline, Point *point)
{
return orthconn_distance_from((OrthConn *)zigzagline, point,
zigzagline->line_width);
}
Since zigzagline inherits from orthconn, we should not need to mention
that code at all, period. The second routine is a bit trickier, but that's
probably because orthconn ought to own the line_width property.
Same for zigzagline_add_segment_callback : this routine adds no value
(it's not the case for, say, bus_add_dhandle_callback() ; I'm not saying
that the rightclick menu callback system is wrong, just that it should be
possible to simply refer to a orthconn_add_segment_callback() directly
wherever an object doesn't change this part of orthconn's behaviour.
Now, I don't say we must use C++ ; the fixup mechanism I proposed happily
does the job (though at run-time instead of at compile-time).
Another way would be to have objects describing their routines public
rather than static, and let them refer another object's routines as they
need ; to solve dependencies, the library loader would simply attempt to
load each library until there's no unloaded library besides the libs with
broken dependencies.
However, I strongly disagree with you with your statement that '''if an
object type is to be a base type, then it belongs to lib/ .''' (my
reformulation) ; first, there's the case of object re-branding : I want to
re-use the current Ethernet bus object, and call it "OR vergence" (to draw
IEC848 grafcet sheets). I should be able to just provide a new ObjectType
structure, giving the bus_type_ops and bus_type_properties and a new name.
Then I need to have a "bus"-like object, with two base lines drawn at a
line width interval, instead of a single base line,
and call it "AND vergence". However, I add no new state, no new
properties, no nothing to the OR vergence, except a new draw routine. I
should be able to provide only this new draw routine (it wouldn't make
sense to clutter the ethernet bus' draw routine with the possibility to
draw two base lines instead of one, so here it makes sense to fork *this*
routine and keep most the rest[maybe a few tweaks to bus_distance_from]).
> Essentially you describe the layout of your object, and then you put this
> description in the ObjectType. Then you just replace the properties and
> load/save methods with generic functions that uses the object
okay, I proposed to do this job in XML, externally to the code, but this
makes sense too.
I'd add a way to specify min, max value and increment step to the
Properties structure (maybe a union { void *na; /* N/A = NULL */
struct { real min,max,step } bounds;
} as an additional parameter);
and of course, macros in the line of
#define LINE_COLOR_PROPERTY(objecttype) \
{ "line_color", PROPERTY_COLOR, G_STRUCT_OFFSET(objecttype,
line_color), &color_black, N_"Line Style:", NULL }
to shorten the definition and encourage use of common property
definitions.
However, I totally don't get the point of having this :
struct _ZigzaglineState {
ObjectState obj_state;
Color line_color;
LineStyle line_style;
real dashlength;
real line_width;
Arrow start_arrow, end_arrow;
};
and this :
typedef struct _Zigzagline {
OrthConn orth;
Color line_color;
LineStyle line_style;
real dashlength;
real line_width;
Arrow start_arrow, end_arrow;
} Zigzagline;
which, save the first member, are identical. Now, you have to provide this
really senseful, value-adding routine :
static void
zigzagline_set_state(Zigzagline *zigzagline, ZigzaglineState *state)
{
zigzagline->line_color = state->line_color;
zigzagline->line_style = state->line_style;
zigzagline->dashlength = state->dashlength;
zigzagline->line_width = state->line_width;
zigzagline->start_arrow = state->start_arrow;
zigzagline->end_arrow = state->end_arrow;
g_free(state);
zigzagline_update_data(zigzagline);
}
Isn't there a way to use plain but hidden versions of objects wherverer
object state is needed ? Then, it could be possible to replace the current
foo_copy(Foo *foo), foo_set_state(), foo_get_state() by a single
foo_copy_state(Foo *dest, Foo *src).
Or better, when objects are simply lib/ base objects (eg orthconn) modulo
new properties and a draw routine, have the base object's routine handle
the copying of all properties (that is, zigzagline could refer to
orthconn_copy_state and be confident that orthconn_copy_state copies all
properties described in zigzagline_properties ; that means the prototype
should be foo_copy_state(Foo *dest, Foo *src, const ObjectType *type);
Of course, more complex objects such as "bus" should be allowed to provide
their own, more compled bar_copy_state(), as needed.
> Some objects have properties that will never be possible to handle with
> generic helper functions. These will still have to define their properties
> so that they might be used in scripts and multi-selected-object properties
> editing, but the properties that are non-describable has to be marked of
> type PROPERTY_OPAQUE or something.
yeah, this makes sense.
> > 2 Eliminate register_sheets(), handle the filling of sheets
> > through improved index.sheet (built so that a user-defined sheet takes
> > precedence over dia-supplied ones).
> Yes, this should really be done. This removes some extra code from object
> libraries and make it easier to change the sheets and even make your own.
Okay, I'll tackle this first.
> > 6 de-couple object implementation from object type definition ;
> > put object type definition in XML files (define a suitable format, similar
> > to the way shapes are defined). Property list contents and midclick
> > menu appearance are defined in the object type definition file
> Why? I don't understand this.
Hmmm, I don't know on which sentence the "Why?" applies. The first
sentence : because, in my view, object re-branding and could have been
done outside the binary (more flexibility for the power-user) (I won't
fight on this : lightweight inheritance and table-based property
definitions do fit the bill).
The second sentence has a dependency on the first one : because object
types would have been defined in XML, two different object types with
possibly different semantics (see Bus object vs. OR Vergent object) could
have been created on a single implementation. In that case, there's a need
to define the appearance of the midclick menu at the same level as the
object type definition, that is, at the XML level.
Oh, I noticed that a lot of strings in newzzline.cpp are defined N_()
though they seem to appear in the UI (see property_list and object menu).
How's i18n to be handled ?
> > 8 While doing 7, possibly try to merge object implementations
> > wherever practical.
> Well, if it is possible and really wanted yes.
> By the way, sharing object implementations for object that are much alike
> is actually possible right now by using the user_data argument of
> object_create(). You can place different arguments in different sheet
> objects refering to the same object. This is used for example in the class
> and template class case.
Indeed. I didn't see that (feared that user_data was meant for shapes, or
something like that). With a little difference : in the current mechanism,
when objects are "much alike", then they have the same object type (and a
discriminating property). Now, in the current objects, you can put a weak
entity (with the weak entity sheet object), and later turn it into a
normal entity (and vice-versa), since the property is (to be) declared (so
it is saved *and* appears in the property editing dialog). I know you
don't like this, but there really should be a PROPERTY_INVISIBLE flag in
the Properties structure : though it may make sense to have the ability to
turn an entity into a weak entity, in Grafcet it makes absolutely no sense
to turn a OR vergent into a AND one. And it wouldn't make sense to fork
the whole object type definition just for that...
Now, since I'll get rid of compile-time SheetObject, to have it described
in XML, I'll have to loose the flexibility of giving a "void *" user_data.
Would two arguments (int_user_data and string_user_data) do the trick ?
-- Cyrille
------------------------------------------------------------------------------
Grumpf.
