Remove redundant occurences of this->

Remove redundant occurences of this->

[Keith, 2015-07-21 04:07:14]

Why bother ?
It is probably not a good idea to remove the this-> where it was used in
comments, particularly to distinguish the function() in virtual-function table
for the object from function() in a parent class.

[dak, 2015-07-21 05:02:23]

On 2015/07/21 04:07:14, Keith wrote:
> Why bother ?

Because it's distracting and makes some things too long to fit a line.

> It is probably not a good idea to remove the this-> where it was used in
> comments, particularly to distinguish the function() in virtual-function table
> for the object from function() in a parent class.

Huh?  As opposed to Type::function (), this->function () is perfectly capable of
meaning either.  It's also unsuitable for distinguishing static and non-static
member functions.  What it can do is distinguish global and member functions. 
It doesn't actually make a difference here, either: it's just some sort of
implied comment that this cannot be anything other than a member function.

Which is comparatively weak sauce.

At any rate: point out any particular instance where you (and not some
hypothetical strawman) would prefer to keep seeing this-> in the patch.

[Dan Eble, 2015-07-21 18:20:13]

this->f() is sometimes necessary in templates (I recall), but in those cases the
compiler warns about ambiguity when it is omitted.

I prefer not to see this-> where it is unnecessary, however Keith does have a
good point about extra clarity in comments.

[dak, 2015-07-21 19:37:21]

On 2015/07/21 18:20:13, Dan Eble wrote:
> this->f() is sometimes necessary in templates (I recall), but in those cases
the
> compiler warns about ambiguity when it is omitted.
> 
> I prefer not to see this-> where it is unnecessary, however Keith does have a
> good point about extra clarity in comments.

Shrug.  This patch changes 24 lines.  It does not make any sense to dabble in
general statements.  If there is a particular comment where anyone would not
want to see the change, it is easy enough to point it out and it can be changed
if others agree.  If there isn't, what are we even talking about?

We have likely already spent more lines of text on the discussion than are
affected by the patch.  I've had 9000-line automated changes go through with
less of a discussion than this.  And for things like 9000-line automated
changes, it is quite usual to do preparatory/cleanup work in order to pass
muster.  So if this patch needs a 1-line change to be ok, no problem with doing
that.

Or is there?

[Dan Eble, 2015-07-22 02:07:53]

On 2015/07/21 19:37:21, dak wrote:
> So if this patch needs a 1-line change to be ok, no problem with doing
> that.
> 
> Or is there?

LGTM.  No need to lift a finger.  I just wanted to register that Keith's
comments weren't entirely unappreciated.

[Keith, 2015-07-23 05:50:38]

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc#newcode232
lily/grob.cc:232: /* This version of get_system is more reliable than get_system
()
What do you mean by "get_system is more reliable than get_system ()" ?

[dak, 2015-07-23 06:33:07]

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc#newcode232
lily/grob.cc:232: /* This version of get_system is more reliable than get_system
()
On 2015/07/23 05:50:38, Keith wrote:
> What do you mean by "get_system is more reliable than get_system ()" ?

The one-argument form of get_system is "more reliable" than the zero-argument
form of get_system.  You can perfectly well call the one-element form with
this->get_system (me) as well, given a suitable this-pointer.  Or with
me->get_system (me).

So the comment does not make less sense with regard to the actual language
definition than it did before.  It may be more apparent.

At any rate, providing two identically named member functions, one static and
one non-static (the latter of course makes use of the virtual function table but
that's not really obvious from the code and comments here) seems like really bad
interface design.

At any rate, this would need to be

  The static member function get_system (Grob *) is more reliable than the
non-static member function get_system ()

You can call either function using some this->get_system (...) call.

[dak, 2015-07-23 06:38:00]

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc#newcode232
lily/grob.cc:232: /* This version of get_system is more reliable than get_system
()
On 2015/07/23 05:50:38, Keith wrote:
> What do you mean by "get_system is more reliable than get_system ()" ?

Actually, what did the original author mean by "more reliable than
this->get_system ()"?

I can perfectly well write "me->get_system ()" to call the non-static member
function get_system ().  I don't need a this-pointer to call it, even though
whatever pointer I call it through then becomes the this-pointer _inside_ of the
function.

To preserve meaning, there must be some to start with.

[dak, 2015-07-23 06:44:41]

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc#newcode232
lily/grob.cc:232: /* This version of get_system is more reliable than get_system
()
On 2015/07/23 05:50:38, Keith wrote:
> What do you mean by "get_system is more reliable than get_system ()" ?

by the way: if you need to employ creative quoting in order to make your point
(in this case changing "This version of get_system" into "get_system"), it would
appear that you don't consider it convincing yourself.

[dak, 2015-07-23 13:47:42]

Change obtuse comment back to Keith's preferred state

[dak, 2015-07-23 13:48:21]

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/258870043/diff/1/lily/grob.cc#newcode232
lily/grob.cc:232: /* This version of get_system is more reliable than get_system
()
On 2015/07/23 05:50:38, Keith wrote:
> What do you mean by "get_system is more reliable than get_system ()" ?

Done.