Code review comment for lp://staging/~autopilot/autopilot-qt/update-for-1.3
Revision history for this message
| Thomi Richards (thomir-deactivatedaccount) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
9199653
demo site
(Get the code!)
> Given that this is a Qt project I think we should try to adapt Qt quidelines.
>
> Please use QString instead of std::string and one of Qt's shared pointer
> mechanisms.
Michael,
The GetName and GetPath methods are defined in xpathselect. We cannot use QStrings here since xpathselect does not (and should not) rely on Qt. Similarly, we use std::shared_ptr because that's what the interface defines.
> In fact, I'm not a big fan of shared pointers at all as in most
> cases they lead to bad design choices. So if not really necessary, try
> avoiding those.
I'm not sure what you mean - this is a classic case where shared_ptrs are necessary. Qt has it's fancy copy-on-write semantics which make passing objects around by value very cheap, but other toolkits don't have that. Since we're dealing with an interface that has to be toolkit-agnostic, we can't start using Qt's magic.