Re: First attempt at new accidentals in git

[Han-Wen Nienhuys]

2007/11/16, Rune Zedeler <address@hidden>:
> >> +/*
> >> +FIXME!!!
> >> +Does it work with simple smobs when it contains a reference to a 
> >> pitchclass_?
> >> +*/
> >
> > Yes, however, you must make sure that the Pitchclass* does not come
> > from a SCM, ie.
> >
> >   pitchclass_ = unsmob_pitchclass (scm_obj)  // wrong, pitchclass_
> >   // may be deleted if scm_obj becomes unreachable
> >
> >   pitchclass_ = new Pitchclass (*unsmob_pitchclass (scm_obj))  // wrong
>
> Eh, do you mean that the lower possibility is /right/ (not wrong)...?

oops, yes you're correct. The other option BTW is to keep a SCM, so you do

  pitchclass_ = scm_obj

then you have to mark in the gc_mark function.

> >> +  SCM virtual_smobbed_copy () { // AAARGH!
> >> +    return smobbed_copy ();
> >> +  }

for this, we usually have a clone() method, see the
VIRTUAL_COPY_CONSTRUCTOR method; so I suggest to use a smobbed_clone()
function.

> >>    DECLARE_SIMPLE_SMOBS (Pitch);
> >
> > This looks bad. You have to remove Pitch as a smobtype, and then
> > create an unsmob_pitch, which does
> >
> >   dynamic_cast<Pitch*> (unsmob_pitchclass (obj))
>
> That won't work with simple smobs.

Why? It should work if have virtual methods.

> If you call Pitchclass::smobbed_copy() on an object of type Pitch then
> the smob will only contain the fields of Pitchclass, and therefore
> Pitchclass::unsmob will always return an object of class Pitchclass -
> even if the original object was of class Pitch.
>
> I really don't like moving to complex smobs for something as simple as a
> pitch.
> What is the problem with having both Pitch and Pitchclass as smobtypes?

There is no clear cut hard argument, but it is contrary to the
practice for all other types.

some soft arguments:

I prefer to have the inheritance relation to be encoded in only one
place, the C++ class declaration. By only smobbing the basetype, you
can always convert to complex smobtypes in the future, something which
impossible when there both are distinct smobs. Also, it clouds the
type signature of functions on the scheme side. Suppose you have x,
which is a pitch smob.  Then you have function f which takes a
pitchclass. From the scheme side of things, it's not clear that f can
be applied to x, since x has a different type.

> > Then, double check that all relevant functions distinguish between
> > unsmob_pitch()  and unsmob_pitchclass() correctly.
>
> Hmm. So what you say is that the programmer should basically
>
> Pitchclass * pc = unsmob_pitch (scm);
> if (!pc) pc = unsmob_pitchclass (scm);

no, there is one smob type, with 2 unsmob functions:

  unsmob_base (),

  unsmob_derived (SCM x) { return dynamic_cast<Derived*> (unsmob_base(x)); }

Each function calls the appropriate unsmob function, depending on the
arguments expected.

> > You should also modify the print routine for the Pitch(class) smob so that 
> > it
> > will print
> >
> >  #<Pitch c''>
> >
> > vs
> >
> >  #<Pitchclass c>
>
> This will afaics require the Pitchclass implementation to know about the
> Pitch implementation - which is very bad oo style.

the print routine should call a method, obviously.

> (The pitchclass print_smob should check whether the SCM contains a Pitch
> and if so, call Pitch::print_smob).
> IMO the right thing would be to add a virtual method that does the
> printing, and then print_smob can call that method.
>
> >> +INSTANTIATE_COMPARE (Pitchclass, Pitchclass::compare);
> >
> > that puzzles me: how can you compare pitchclasses, which are
> > essentially cyclic?
>
> My idea was to compare them based on the position in the key-signature:
>
> c f bes es ... g d a ...
>
> That way one could just sort the list of pitchclasses before generating
> the key-signature accidentals. Ofcourse this would need some sort of
> comment.

ok.


-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen