Re: [Audacity-devel] Patch for the Bug 181

[Chandranshu G. <gar...@gm...>]

yes you are right.

Thanks for clearly pointing out my mistakes.
There are lot of things i need to learn in programming practices.

Chandranshu Garg

On Fri, Aug 17, 2012 at 5:12 AM, Vaughan Johnson
<va...@au...>wrote:

> Thanks for trying to help, but your patch is not acceptable.
>
> * It's generally a bad idea to copy and paste code rather than
> encapsulate it.
>
> * It's a worse idea to copy and paste from one class to another. You
> copied code from TrackPanel::HandleClosing() into
> AudacityProject::OnRemoveTracks(), and that's why you had to make those
> convoluted calls. For example, one line in your patch:
>
>    mTrackPanel->GetListener()->TP_RedrawScrollbars();
>
> shows you did not analyze the relationships between TrackPanel and
> AudacityProject, because mTrackPanel->GetListener() is the
> AudacityProject the TrackPanel is hooked up to. So you could have just
> called this->TP_RedrawScrollbars()... if this were the right solution.
>
> * It's a TrackPanel functionality, so the code should be on TrackPanel.
> So I made a new method on TrackPanel::UpdateViewIfNoTracks(), with the
> code that was in TrackPanel::HandleClosing(), and called it from
> TrackPanel::HandleClosing() and AudacityProject::OnRemoveTracks().
>
>
> Thanks,
> Vaughan
>
>
>
> On 8/16/2012 4:57 AM, Chandranshu Garg wrote:
> > Hi
> >
> > Attached is the patchfile for the Bug 181
> >
> > Bug 181 - Tracks > Remove Tracks leaves timeline cursor or selection at
> > non-zero http://bugzilla.audacityteam.org/show_bug.cgi?id=181
> >
> > Regards
> > Chandranshu Garg
>
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> audacity-devel mailing list
> aud...@li...
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
>