Re: [patch] changeDepth + updateCounters

[Alfredo Braunstein]

John Levon wrote:

> On Tue, Nov 25, 2003 at 09:57:01PM +0100, Alfredo Braunstein wrote:
> 
>> 1) it splits changeDepth into
>> 
>> bool changeDepthAllowed and
>> void changeDepth
>> 
>> and tries to use more understandable code. changeDepthAllowed is
>> potentiallty a bit faster than before.
> 
> So we have some slightly tricky logic duplicated ? Why is that a benefit ?

(because it's hard to see what the function does? This function breaks
clearly the "a function should do one single thing (and do it well)" rule.
And the code duplicated is a single loop. )

What about the following?

Thanks for looking at it.

Alfredo

namespace {

bool changeDepthAllowed(bv_funcs::DEPTH_CHANGE type,
                        ParagraphList::const_iterator pit,
                        int prev_after_depth)
{
        if (pit->layout()->labeltype == LABEL_BIBLIO)
                return false;
        int const depth = pit->params().depth();
        if (type == bv_funcs::INC_DEPTH && depth < prev_after_depth)
                return true;
        if (type == bv_funcs::DEC_DEPTH && depth > 0)
                return true;
        return false;
}


}


bool LyXText::changeDepthAllowed(bv_funcs::DEPTH_CHANGE type)
{
        ParagraphList::iterator beg, end; 
        getSelectionSpan(*this, beg, end);
        int prev_after_depth = 0;
        if (beg != ownerParagraphs().begin())
                prev_after_depth = boost::prior(beg)->getMaxDepthAfter();

        for (ParagraphList::iterator pit = beg; pit != end; ++pit) {
                if (::changeDepthAllowed(type, pit, prev_after_depth))
                        return true;
                prev_after_depth = pit->getMaxDepthAfter();
        }
        return false;
}


void LyXText::changeDepth(bv_funcs::DEPTH_CHANGE type)
{
        ParagraphList::iterator beg, end;
        getSelectionSpan(*this, beg, end);
        
        recUndo(parOffset(beg), parOffset(end) - 1);

        int prev_after_depth = 0;
        if (beg != ownerParagraphs().begin())
                prev_after_depth = boost::prior(beg)->getMaxDepthAfter();

        for (ParagraphList::iterator pit = beg; pit != end; ++pit) {
                if (::changeDepthAllowed(type, pit, prev_after_depth)) {
                        int const depth = pit->params().depth();
                        if (type == bv_funcs::INC_DEPTH)
                                pit->params().depth(depth + 1);
                        else
                                pit->params().depth(depth - 1);
                }
                prev_after_depth = pit->getMaxDepthAfter();
        }
        // this handles the counter labels, and also fixes up
        // depth values for follow-on (child) paragraphs
        updateCounters();
        redoCursor();
}