Review Request: Introduce two new KCrash methods: void setLaunchDrKonqi(bool) and bool launchDrKonqi().

George Kiagiadakis kiagiadakis.george at gmail.com
Sun Oct 25 23:03:36 GMT 2009


On Mon, Oct 26, 2009 at 12:32 AM, Thiago Macieira <thiago at kde.org> wrote:
> Em Domingo 25. Outubro 2009, às 21.04.53, Chani escreveu:
>> On October 25, 2009 11:45:02 Thiago Macieira wrote:
>> > Em Domingo 25. Outubro 2009, às 17.33.09, George Kiagiadakis escreveu:
>> > > -----------------------------------------------------------
>> > > This is an automatically generated e-mail. To reply, visit:
>> > > http://reviewboard.kde.org/r/1970/
>> > > -----------------------------------------------------------
>> > >
>> > > Review request for kdelibs.
>> > >
>> > >
>> > > Summary
>> > > -------
>> > >
>> > > This patch introduces two new methods in the KCrash namespace: void
>> > >  setLaunchDrKonqi(bool) and bool launchDrKonqi(). The reason for
>> >
>> > The name "launchDrKonqi" is a bad one. "Launch" is a verb in the
>> >  imperative, so "launchDrKonqi" doesn't look like a setter, but an
>> > action.
>> >
>> > I recommend using "launchesDrKonqi" instead.
>> >
>> > Also, for boolean properties, prefixing the getter with "is" is
>> >  recommended, but then you need an adjective as the core of the property
>> >  name (as in "isDrKonqiLaunchEnabled"). If you can't come up with a good
>> >  name, don't use "is", it's not a problem.
>>
>> enableDrKonqui and isDrKonquiEnabled?
>
> No, those don't match property getter/setter naming conventions.
>
> Properties should be nouns, not verbs. What I said above about adjective was
> wrong: the property was still a noun (launch), but was disambiguated from the
> homograph verb by an adjective.
>
> If you call it "enable", it's an action. The opposite is "disable", not a
> true/false parameter.

/me is getting confused with all this...

Ok, I agree that launchDrKonqi() is not a good name for a getter. How
about these:

bool isDrKonqiEnabled()
void setDrKonqiEnabled(bool)

It misses the "launch" part though. Not sure if I am thinking clearly
at the moment; it's getting late here... I'll rethink about it in the
morning...




More information about the kde-core-devel mailing list