[Merge] lp:~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Thu Jun 16 09:00:44 UTC 2016


> I know, but I tend to prefer environment agnostic function when
> possible, hence my approach there, I dont like "introspective"
> function that dont have to be,

I’m not sure I’m following you here. isPrintHelpLaunch() is not used outside of the context of BrowserApplication (and indeed it’s a member function), so what’s wrong with making it non-static and accessing member variables?
It has a similar scope to BrowserApplication::inspectorPort() or BrowserApplication::urls(), which both read m_arguments.


> I am not sure if it is a cleaner approach, constructor work + state
> flag seem weird to me,

You’re right, it would really be cleaner if there was a way to bail out early in the constructor, but adding a flag sort of cancels the "elegance" of the solution.


> What bothers you is the double calls to isPrintHelpLaunch?

I guess the naming does :) Can it be renamed to e.g. "helpRequested", or maybe "shouldDisplayHelp"?
-- 
https://code.launchpad.net/~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid/+merge/297220
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~abreu-alexandre/webbrowser-app/container-print-help-when-no-appid into lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list