[Ctk-developers] Libs/DICOM/Core/ctkDICOMModel.cxx : variables names with same naming as method names

[Steve Pieper]

Hi Guys -

+1 on good naming conventions for variables (I like Julien's suggestions).

I also echo Julien's point that the coding convention should follow the 
conventions of the parent classes.  So code that inherits from vtkObject 
should be VTK-style, QObject code should be Qt-style, etc.  This will 
follow the 'principle of least surprise' for a developer who picks up a 
ctk Qt widget and wants to use it in their Qt code.

-Steve


On Apr/8/10 6:43 PM, Luis Ibanez wrote:
> Julien,
>
> I like your suggestion better than mine.
>
> The three approaches that you mentioned will
> make code much readable and maintainable.
>
> It will be nice to collect coding style guidelines
> somewhere... at least in a Wiki,... or in a text
> file to be included in the git repository.
>
>
>       Luis
>
>
> -------------------------------------------------------------------------------
> On Thu, Apr 8, 2010 at 6:31 PM, Julien Finet<julien.finet at kitware.com>  wrote:
>> Agree, I'm not a big fan of the 2 solutions you gave though.
>> When I remember to avoid variable shadowing, I have a couple of tricks to
>> rename problematic variables:
>> for example a variable named "row" could be renamed as:
>>   - indexRow / itemRow / cellRow / childRow / parentRow  ( who owns the
>> variable)
>>   - rowToInsert/rowToDelete (action on the variable)
>>   - newRow / oldRow / tmpRow ( lifetime of the variable)
>>   - ....
>> There is almost always one prefix or suffix that can be added and gives more
>> information about the variable.
>> Julien.
>> On Thu, Apr 8, 2010 at 6:15 PM, Luis Ibanez<luis.ibanez at kitware.com>  wrote:
>>>
>>> I see,
>>>
>>> It is too bad that Qt got satisfied with a lower quality bar.
>>>
>>>
>>> We can still avoid having local variables with the same
>>> names as member variables and member methods.
>>>
>>> For example,
>>> many of the offending warnings were fixed by replacing
>>>
>>>      ambiguousvariablename
>>>
>>> with
>>>
>>>      ambiguousvariablenameValue
>>>
>>> we could have as well used the more arcane
>>>
>>>      ambiguousvariablename_
>>>
>>>
>>> Making good choices on names of methods, and variables
>>> is a an effective way of documenting the code.
>>>
>>>
>>>     Luis
>>>
>>>
>>>
>>> -----------------------------------------------------------------------------------
>>> On Thu, Apr 8, 2010 at 6:08 PM, Julien Finet<julien.finet at kitware.com>
>>> wrote:
>>>> Hi Luis,
>>>> I understand your concern.
>>>> We can't change the name of the method index() as it is a virtual method
>>>> that has been declared in a Qt subclass.
>>>> And the problem we face here is that the Qt library doesn't care about
>>>> shadow variables. (and if I recall, they didn't plan on changing that).
>>>> We unofficially decided during the CTK Pre Hackfest that a CTK class
>>>> should
>>>> follow the naming convention of its derived class, Qt in this example.
>>>> Is that still OK for everybody ?
>>>> Setting up KWStyle seems like a good idea. Can we configure it in a way
>>>> to
>>>> check a style depending on a filename (ideally the style of the derived
>>>> class)
>>>> Julien.
>>>> On Thu, Apr 8, 2010 at 5:50 PM, Luis Ibanez<luis.ibanez at kitware.com>
>>>> wrote:
>>>>>
>>>>> I recall that at some point we discussed a
>>>>> naming convention for Qt-like classes, but
>>>>> I'm not sure if we arrived to a consensus.
>>>>>
>>>>>
>>>>> The file:
>>>>>
>>>>>   Libs/DICOM/Core/ctkDICOMModel.cxx
>>>>>
>>>>> had an abundant use of local variables with
>>>>> names matching some member variables and
>>>>> member methods.
>>>>>
>>>>> A notable example:
>>>>>
>>>>>                        "index"
>>>>>
>>>>> I just committed a fix for most of them,
>>>>> but for the long run it will be great if we
>>>>> agree on some naming convention.
>>>>>
>>>>> I would vote against having a method of a C++ class
>>>>> to be called "index()" for example.       :-)
>>>>>
>>>>> a more useful name will be "getIndex()", for example,
>>>>> or in this particular case, it looks like what the function
>>>>> is actually doing is generating an index, so good names
>>>>> could be
>>>>>
>>>>> ctkDICOMModel::generatgeIndex ( int row, int column, const QModelIndex
>>>>> &  parentValue ) const
>>>>>
>>>>> or
>>>>>
>>>>> ctkDICOMModel::computeIndex ( int row, int column, const QModelIndex&
>>>>> parentValue ) const
>>>>>
>>>>> or
>>>>>
>>>>> ctkDICOMModel::produceIndex ( int row, int column, const QModelIndex&
>>>>> parentValue ) const
>>>>>
>>>>>
>>>>> There is already another
>>>>>
>>>>>                 createIndex(int, int, Node *)
>>>>>
>>>>>
>>>>> so it will be fine if we use
>>>>>
>>>>> ctkDICOMModel::createIndex ( int row, int column, const QModelIndex&
>>>>> parentValue ) const
>>>>>
>>>>> since the signature will be different.
>>>>>
>>>>>
>>>>> ---
>>>>>
>>>>>
>>>>> Do we have a document on coding style ?
>>>>>
>>>>> if we do,
>>>>> we probably should setup KWStyle testing
>>>>> before things get out of control.
>>>>>
>>>>>
>>>>>
>>>>>       Luis
>>>>
>>>>
>>
>>
> _______________________________________________
> Ctk-developers mailing list
> Ctk-developers at commontk.org
> http://public.kitware.com/cgi-bin/mailman/listinfo/ctk-developers