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

List:       kde-core-devel
Subject:    Re: Review Request: Introduce two new KCrash methods: void
From:       George Kiagiadakis <kiagiadakis.george () gmail ! com>
Date:       2009-10-25 23:03:36
Message-ID: ba4a6e220910251603o3d49304crfbfe61dc3251e647 () mail ! gmail ! com
[Download RAW message or body]

On Mon, Oct 26, 2009 at 12:32 AM, Thiago Macieira <thiago@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...

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