[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-devel
Subject:    Re: Review Request 119459: Added some Unittest for FileMonitor
From:       Marc Schmitzer <marc () marc-schmitzer ! de>
Date:       2014-07-26 12:16:25
Message-ID: 2329820.dEn66RF8Bz () lenny
[Download RAW message or body]

On Saturday 26 July 2014 11:58:32 Felix Eisele wrote:
> > On Juli 25, 2014, 12:04 nachm., Vishesh Handa wrote:
> > > src/file/autotest/filemonitortest.h, line 37
> > > <https://git.reviewboard.kde.org/r/119459/diff/1/?file=292598#file292598
> > > line37>> > 
> > >     Just out of curiosity, is there any advantage of making it a member
> > >     variable instead of just initializing it in the function?
> you save lines of code and unittests focusing what he realy do
In my experience, that is not a good idiom. Test cases should be self-
contained and isolated from all other cases. If anybody ever has to debug that 
test, he will first wonder where the value comes from, and if something may 
have modified it. If the object is created locally in the function, that is 
immediately clear.
Also, the idiom breaks if you ever add a test case that requires the object to 
be instantiated with (different) parameters.

Thanks,
Marc


> 
> 
> - Felix
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119459/#review63120
> -----------------------------------------------------------
> 
> On Juli 25, 2014, 10:41 vorm., Felix Eisele wrote:
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://git.reviewboard.kde.org/r/119459/
> > -----------------------------------------------------------
> > 
> > (Updated Juli 25, 2014, 10:41 vorm.)
> > 
> > 
> > Review request for Baloo and Vishesh Handa.
> > 
> > 
> > Repository: baloo
> > 
> > 
> > Description
> > -------
> > 
> > Added some Unittest for FileMonitor
> > 
> > 
> > Diffs
> > -----
> > 
> >   src/file/autotest/filemonitortest.h 9a72813
> >   src/file/autotest/filemonitortest.cpp 19d8f85
> > 
> > Diff: https://git.reviewboard.kde.org/r/119459/diff/
> > 
> > 
> > Testing
> > -------
> > 
> > 
> > Thanks,
> > 
> > Felix Eisele


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread]