[Nepomuk] Review Request 113188: Identifier: Give PersonContacts with a nco:contactUID special treatement

David Edmundson david at davidedmundson.co.uk
Thu Oct 10 18:46:08 UTC 2013



> On Oct. 10, 2013, 10:32 a.m., David Edmundson wrote:
> > services/storage/resourceidentifier.cpp, line 139
> > <http://git.reviewboard.kde.org/r/113188/diff/1/?file=200242#file200242line139>
> >
> >     could be const&
> >     same for newUri, and the ids later.
> 
> Vishesh Handa wrote:
>     Are you sure? The nieUrl method returns a temporarily constructed QUrl. Trying to create a reference to a temporary object does not seem like a good idea.
> 
> Vishesh Handa wrote:
>     Okay. I checked and it doesn't seem to make any difference. The constructor is only called once.
>     
>     Tested by doing -
>     Class TestClass {
>     public:
>         TestClass() {
>            cout << "Constructor\n";
>            a = 5; 
>         }
>     
>         int a;
>     }
>     
>     TestClass foo() { TestClass c; c.a = 99; return c; }
>     
>     int main() {
>         TestClass cl = foo();
>         cout << cl.a << endl;
>     
>         const TestClass& cl2 = foo();
>         cout << cl2.a << endl;
>     
>         return 0;
>     }
>     
>     The output is "Constructor\n 99\n Contructor\n 99\n". So, meh! Doesn't make any difference. Will investigate more.
>

Some people's rule of thumb is "if you expect something to be const, add the keyword". It can help, won't do any harm.
You're quite probably right it won't add anything. I didn't mark it as "an issue".

As for why it doesn't explode:
http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113188/#review41481
-----------------------------------------------------------


On Oct. 10, 2013, 10:20 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113188/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2013, 10:20 a.m.)
> 
> 
> Review request for Nepomuk and David Edmundson.
> 
> 
> Repository: nepomuk-core
> 
> 
> Description
> -------
> 
>     We check if another contact with the same identifier exists during the
>     identification phase. This way even if the nickname has changed, as long
>     as the contactUID is the same, a new contact is not created.
> 
> 
> Diffs
> -----
> 
>   services/storage/resourceidentifier.cpp c59bba355ba7cebfac8d79b642a3486993b58e26 
>   services/storage/test/identificationtests.h 4f2c0280ae4e090dfa307ad83188128ec6c4c267 
>   services/storage/test/identificationtests.cpp 0ed1343a0684418bfa499296f254e273cb8db4fc 
>   services/storage/test/qtest_dms.cpp d758a81ba66f0a86f12ac2b7a57454ab0bb66575 
> 
> Diff: http://git.reviewboard.kde.org/r/113188/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20131010/7be00b27/attachment-0001.html>


More information about the Nepomuk mailing list