On 04/18/2012 02:12 PM, E. Liddell wrote:
> On Wed, 18 Apr 2012 13:25:43 -0500
> "David C. Rankin" <drankinatty@...> wrote:
> 
>> On 04/17/2012 06:22 PM, David C. Rankin wrote:
>>> Darrell, all
>>>
>>>   I'm making an attempt at fixing k3b for gcc47. If anyone else has already done
>>> this please stop me. It is part of bug 958.
>>>
>>
>>   This is another redeclaration issue, but it is not the same type of simple
>> iterator issue I've seen earlier. The build of k3b fails on gcc47 with:
>>
>> k3baudioeditorwidget.cpp: In member function 'virtual void
>> K3bAudioEditorWidget::mousePressEvent(TQMouseEvent*)':
>> k3baudioeditorwidget.cpp:674:12: error: redeclaration of
>> 'K3bAudioEditorWidget::Range* r'
>> k3baudioeditorwidget.cpp:668:14: error: 'K3bAudioEditorWidget::Range* r'
>> previously declared here
>>
>>   It is complaining about 'r' being declared twice below. This looks more like a
>> reassignment than a redeclaration to me. How do we properly fix it?. If all we
>> care about here is 'r' having function scope, can't I just get rid of the
>> 'Range* ' typecast following the else { ? Or do I have to change 'r' to 'not_r'
>> following the else?
> 
> If I'm understanding the code correctly, I would probably move the declaration
> of r outside the condition for the if, that is:
> 
>  void K3bAudioEditorWidget::mousePressEvent( TQMouseEvent* e ) {
>    m_draggedRange = 0;
>    m_draggedMarker = 0;
>    bool end;
>    Range* r = findRangeEdge(e->pos(), &end);
> 
>    if (r) {
>      m_draggedRange = r;
>      m_draggingRangeEnd = end;
>    }
>    else {
>      r = findRange( e->pos() );
>      d->movedRange = r;
>      d->lastMovePosition = posToMsf( e->pos().x() );
>      m_draggedMarker = findMarker( e->pos() );
>    }
>    setSelectedRange( r );
>    TQFrame::mousePressEvent(e);
> }
> 
> To my eye, that's clearer than the original.  I would never declare a variable
> inside a condition for a branching construct, even if it's legal to do so (the
> technique has valid applications in loop constructs, but here it just serves to
> muddy the scoping). </opinionated>
> 
> Alternatively, basic scoping rules suggest:
> 
> void K3bAudioEditorWidget::mousePressEvent( TQMouseEvent* e )
> {
>   m_draggedRange = 0;
>   m_draggedMarker = 0;
> 
>   bool end;
>   if( Range* r = findRangeEdge( e->pos(), &end ) ) {
>     m_draggedRange = r;
>     m_draggingRangeEnd = end;
>     setSelectedRange( r );
>   }
>   else {
>     Range* r_two = findRange( e->pos() );
>     d->movedRange = r_two;
>     d->lastMovePosition = posToMsf( e->pos().x() );
>     setSelectedRange( r_two );
>     m_draggedMarker = findMarker( e->pos() );
>   }
> 
>   TQFrame::mousePressEvent(e);
> }
> 
> 

Darrell, E (don't know first name)

  I agree with all your comment and both proposals above, but like Darrell, I
don't have a c++ background to know the 'best' way to do this. I certainly agree
what declaration within the conditional looks bad, but I was not going to take
the liberty and re-write it given the conservative approach I take when
monkeying with the code (though I like the declaration outside the if statement
much better as suggested). I know they are not big changes, but I don't know
enough to know if they were declared in the if for some other reason.

  Of the two, I like the first suggestion the best, just from a style/clarity
standpoint:

 void K3bAudioEditorWidget::mousePressEvent( TQMouseEvent* e ) {
   m_draggedRange = 0;
   m_draggedMarker = 0;
   bool end;
   Range* r = findRangeEdge(e->pos(), &end);

   if (r) {
     m_draggedRange = r;
     m_draggingRangeEnd = end;
   }
   else {
     r = findRange( e->pos() );
     d->movedRange = r;
     d->lastMovePosition = posToMsf( e->pos().x() );
     m_draggedMarker = findMarker( e->pos() );
   }
   setSelectedRange( r );
   TQFrame::mousePressEvent(e);
}

  I'll change it any way you guys want it changed. The way it is currently
patched works fine (k3b runs fine as well), but it isn't the prettiest. Let me
know which option you like the best and I'll redo the patch. I'm just glad k3b
is building and working on gcc47 without and -fpermissive kludge :)

-- 
David C. Rankin, J.D.,P.E.