[Bug 27271] API review: TplLogEntry, TplLogEntryText

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=27271





--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2010-03-23 12:44:24 PST ---
Only a partial review so far...

In TplLogEntryClass:

>  void (*dispose) (GObject *obj);
>  void (*finalize) (GObject *obj);

These seem unused... which is good, because they make no sense.

I'm not sure that the rest of the virtual methods (apart from equal()) make
sense either: why would a subclass ever want to override the generic
get_timestamp method? The timestamp hopefully isn't going to wander off into a
different database table for certain events?

(In reply to comment #0)
> In Bug #27178, Cosimo writes:
> > There are several methods duplicated, my intention is to drop most of them and
> > keep only
> > tpl_log_entry_<method> for what is implemented in TplLogEntry and
> > tpl_log_entry_text_<method> for what is implemented in TplLogEntryText
> > 
> > dropping things similar to:
> > 
> > const gchar* (*get_log_id) (TplLogEntry *self);
> > 
> > in TplLogEntryClass and the related
> > 
> > const gchar *tpl_log_entry_text_get_log_id (TplLogEntryText *self);
> > 
> > which create only confusion and keeping only
> > 
> > const gchar* tpl_log_entry_get_log_id (TplLogEntry *self);

Yes, please do (or I could, or whatever).

There are basically three useful patterns for a GLib method:

* non-virtual (C++: no special keyword, Java: final)

  The base class has a method. Subclasses can't override it.

  GLib example: tp_proxy_get_object_path() (note that there
  is no tp_channel_get_object_path() - that wouldn't be
  useful)

* virtual (C++: virtual, Java: no special keyword)

  The base class has a method which just calls a function
  pointer in the class struct. The base class also has a
  default implementation in *its* class struct, usually.

  GInterfaces often have these; base classes can have
  them too.

  GLib example: g_initable_init() just calls
  the object's implementation of GInitableIface.init.

* "virtual and protected" (analogous to C++ virtual protected)?

  The base class struct has a function pointer in it
  which is called in order to to implement some functionality.
  It doesn't correspond 1:1 to any public method.

  GLib example: tp_base_connection_change_status() is a
  non-virtual method which calls private function pointers
  TpBaseConnection.connecting, TpBaseConnection.shut_down
  etc.

I'm not sure which one TplLogEntry[Text] is trying to use, tbh...


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.