Re: setting the number of pages for a score

[Joe Neeman]

Han-Wen Nienhuys wrote:

> Joe Neeman wrote:
>
> > I've cleaned up most (all?) of the issues that you brought up with my 
>
>
> There's still some variable naming issues. I prefer not to postpone 
> those until the code is merged, because I tend to fix those asap, and 
> that makes it harder to sync your and my trees.
>
> > previous code. I've moved some things around to improve readability 
> > and I've addressed a few of the performance problems. Also, it is now 
>
>
> cool
>
> > #include "ly-smobs.icc"
>
>
> this is not necessary, is it?

No, and there are probably others -- I'd forgotten to clean the includes.

> > Optimal_breaking::best_page (vector<System_spec> const &sys,
> >                              Optimal_break_node &me)
>
>
> We never use non-const references, as they make it hard to see that an 
> argument is modified. Typically, this function should be a member of 
> Optimal_break_node, but if you want to keep this design, it has to be 
> a pointer.

OK, I'll revisit these.

> > /* unlike the default page breaker, we store our penalties in the 
> > system that
> >  * ends the page */
>
>
> is there a reason  for this, shouldn't we rather unify this

This is how both the line breakers do it and I think it makes more 
sense. The penalties belong to the system/column where the break occurs.

> >               /* TODO: support raggedness */
>
>
> I would like to integrate this code soonish, but the new line breaker 
> should support the same functionality as the old one, so we have to 
> have raggedness too.

Sure.

> >   if (!systems_) systems_ = 4; /* just to make this compatible with 
> > Gourlay */
>
>
> I don't understand. What's the meaning of 4 ?

Nothing. The proper usage of Constrained_breaking is to specify a number 
of systems with get_solution (). Previously, I had assert (systems_) 
there but it's still a subclass of Break_algorithm so I thought it 
should at least
not crash on
Break_algorithm *a = new Restrained_breaking ();
a->solve ();

> > vector<Column_x_positions>
> > Constrained_breaking::get_soln (int start, int end, int nsys)
>
>
> soln ? You mean solution ?
> nsys would be called system_count within lilypond code base.

Sorry, looks like I missed a bunch of names.

> > void
> > Constrained_breaking::prepare_solution (int start, int end, int nsys, 
> > int &rank, int &brk)
>
>
> once again: no &

Sure.

> > class Prob;
> > class Paper_score;
>
>
> these should not be necessary: just include lily-proto.hh.

Sure.

> >     /* move the given system from the left page to the right page,
> >      * updating all the forces. prev will now be the last system on the
> >      * left page */
>
>
>
> we do function documentation in the CC files exclusively. Can you move 
> these comments?  If it needs comments, you might want to reconsider 
> the naming anyway.

Ok, I can probably get rid of most of them and move the rest.