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

[Julien Finet]

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
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/ctk-developers/attachments/20100408/7e769487/attachment.htm>