From Buzztard

[edit] compiling

• add test sources in todo check (e.g. via splint)
• override default glib log handler for unit tests (there is a new methods in glib-head now)
• add status change signal to song-io
• the signal reciver should be able to get more informations about the loading progress

[edit] error handling

• g_assert() vs. g_return_if_fail()
  • precondition checks
    • each public method needs a g_return_(val_)if_fail() block at the begin to check incomming parameters
    • private methods can use g_assert(self) here (as this is an internal fault)
    • never use g_assert() for public API precondition checks as this is always fatal and thus e.g. aborts test-casse without letting us intercept the message
  • post condition checks
    • use g_assert as a failing post-condition check signals an internal error
  • other checks
    • when e.g. verifying the internal state of self->priv (own object) then also use g_assert()
• if we need -1 return values in methods, we should use better gulong and g_error for returning
• do tests runs with --g-fatal-warning to find untested code that fails

• GError usage
  • remove GError from machine.c and wire.c
  • use GError for funtion that are called by e.g. UI and that require human readable errors (like song-loading)

[edit] design

[edit] machines and wires should be bins, we need to retest if this now works

advantages:

• allows to use the ghostpags for machine->{src_elem,dst_elem}
• simplifies linking
• enhances the readabillity of the graph, when imported in gst-editor

[edit] step resolution need better visualisation

• choosing step>1 can cause pattern-events to be hidden
  • we could try to paint fractional rectangles into the back of the cells to show patterns
  • we could add elipsis to the label (or some other mark)

[edit] source code

• try to fix all splint stuff
• add macro to each sizeof(CLASS) in every class GTypeInfo struct
• care about song->unsaved (call bt_song_set_unsaved(); )
  • call that in some _set_property() methods of song-subclasses
  • see Undo/Redo handling
• turn: g_object_try_ref(g_value_get_object(value)); into g_value_dup_object(value);
• disconnect "notify::" handler when disposing the object
• swap comparissions with constants (result==TRUE becomes TRUE==result and result==5 becomes 5==result)
  • the advantage of doing so is that the compiler can detect a missing '=', because one can't assign something to a constant
  • what to do with '!=' as here it is not a problem
• replace comparissions with plain numbers with symbolic constants
  • event->button.button== and event->button==
  • grid_density==
  • type == 0 -> !type
• add G_GNUC_CONST; to the end every _get_type function prototype in header files (see gtkdoc)
• use something like G_DEFINE_TYPE for object construction to save typing
• in bt-edit settings-dialog.c uses weak-ref on app, other -dialog.c files use hard-refs, which is better in this case?
• use G_GNUC_INTERNAL in libs to mark internal helpers

[edit] fix unrefrerenced object returns

change methods the return object pointer to use g_object_ref()

[edit] collaboration hierarchy and lists

Several of our classes have children. Doing bt_child_new() adds it to the list. The new methods should get the parent to avoid bt_child_new() looking it up often.

What about making the parent parameter optional (if NULL it will be looked up):

bt_child_new(BtParent *parent, ...) {

  child=g_object_new(...);

  // add child to parent
  if(parent) {
    bt_parent_add_child(parent,child);
  }
  else {
    g_object_get(xxx,"parent",&parent,NULL);
    bt_parent_add_child(parent,child);
    g_object_unref(parent);
  }
}

This applies to:

• bt_wire_new(const BtSong *song, const BtMachine *src_machine, const BtMachine *dst_machine)
• bt_machine_new(BtMachine *self)
  • bt_processor_machine_new(const BtSong *song, const gchar *id, const gchar *plugin_name, glong voices)
  • bt_sink_machine_new(const BtSong *song, const gchar *id)
  • bt_source_machine_new(const BtSong *song, const gchar *id, const gchar *plugin_name, glong voices)
• bt_pattern_new(const BtSong *song, const gchar *id, const gchar *name, glong length, glong voices,const BtMachine *machine)
• bt_wave_new(const BtSong *song,const gchar *name,const gchar *file_name)
• bt_wavelevel_new(const BtSong *song,const BtWave *wave)

[edit] handling id attributes

Elements that can be referenced have an id in the file. Currently that is machines (processor, source, sink). To make the XML well-formed the ids need to be unique. For machines we have:

gchar *bt_setup_get_unique_machine_id(const BtSetup *self,gchar *base_name);

For patterns we concat machine-id and the pattern name.

gchar *bt_machine_get_unique_pattern_name(const BtMachine *self);

Shouldn't we generate pattern ids when saving ? E.g. by using the memory addresses of the objects. When loading object with ids, we use a hashmap in the song to resolve them. (get_object_by_id). Then its totally up to the user how to name items.

It should be enough to generate uniqe id on save time.

[edit] generate setter and getter methods

We not use g_object_set/get. This is short and needs no extra work, but has that disadvantage that we lack compiler-type checking. Apart sometimes we need atomic setters, where we update thing when n-properties have been changed together. Writing setters and getters by hand is tedious though (they also need docs) If would be cool to have something like:

/*
 * *
 * fullname: lastname,firstname
 */
#include "person_accessors.h"

A generator would parse the comment above the include and generate the setters and getters including gtk-doc comments for them.

[edit] problems

• we like to influence the generated docs (add a comment thats gets included)
• we need to point the tool to the gtk-doc sections-file so that it an add the prototypes there
• setters might need some code to be executed after all items have been set.

[edit] testing

• try Diota
• disable all negative tests with we don't have debug enabled

[edit] make tests harder

• do a checked_unref(song) first and the do checked_unrefs of the machines, wires, ...
• will detect more ref-count issues

[edit] docs