Naming things

      There are only two hard things
      in Computer Science: cache
      invalidation and naming things.
      -- Phil Karlton

Hello,

There has been a lot of recent debate regarding the names of some new functions recently added to Phobos.

Mostly this concerns the good work of Walter Bright, and new functions which operate on ranges rather than strings.

A few times, many people felt that the names could have been chosen better - in some cases, much better. This is not a singular occurrence, but a recurrent one:

http://forum.dlang.org/post/ybbwpgmgsqvmbvoqhmxm@forum.dlang.org
https://github.com/D-Programming-Language/phobos/pull/2149#issuecomment-42867964

So far, post-merge name changes have been rejected:

https://github.com/D-Programming-Language/phobos/pull/3243
https://github.com/D-Programming-Language/phobos/pull/3426

Two examples of controversial name pairs: setExt/setExtension, and toLower/toLowerCase. These functions have the same functionality, but one of them is eager, and the other is lazy. Can you guess which is which?

I would like to argue that these rejections were poorly argumented and should have been accepted. Although the names of these particular functions are the primary point of the post, I would like to also discuss the general policy of minor changes.

I have discussed the issue at hand with Andrei Alexandrescu on IRC. Here are some points gathered:

1.

The renames do not apply to code that appeared in a DMD release, thus they are non-breaking changes.

As the actual code has been merged, the renames are not blocking anything either.

2.

There seems to be some confusion regarding what counts as consensus.

Walter Bright argues that there is no consensus regarding the new names. I would like to split this argument into two questions:

a) Is there consensus that the current names are very bad, and should be changed?

b) Is there consensus on which name to use?

These two questions must not be confused.

I think there is sufficient evidence that shows that everyone who has an opinion on the names, agrees that the current names are pretty bad.

What the new names should be is of secondary importance, as long as the names are changed to any of the suggested names.

3.

The main argument against allowing post-merge renames is that allowing one invites an infinite number of other minor changes. I think this is not a good argument, because:

- In this particular case, there is unilateral consensus that the current names are objectively bad, and should be changed. There are no arguments that show that e.g. "setExt" is a better name than e.g. "withExtension". I see no problem with not acting on renaming suggestions in situations when there is no consensus.

- Naming things well matters. We need to treat renames in the same way as minor breaking changes. In the same way that we do not reject minor breaking fixes and improvements to the implementations of functions that have not yet been released, we should improve the naming of identifiers if there is consensus that the change is an improvement.

4.

I have often heard the argument that bikeshedding distracts from getting actual work done, or variations of such. I think this argument is flawed.

Discussions about minor things will continue regardless of whether small changes, such as renames, are rejected as a matter or policy or not. (Yes, this post is an example of this.) Yes, allowing some minor changes but not others will generate debate on why some changes were accepted and others not. Rejecting all minor changes does not prevent such debate from occurring, especially since there will always be exceptions (see e.g. std.meta).

I would thus like to argue that the policy of "no minor changes, even non-breaking" should be reviewed.

5.

Again, naming things well matters. An API with confusing or overlapping identifier names is a bad API. I've said this above but I want to say this again: we need to stop looking at renames as evil or as a waste of time, and look at them in the same way as (breaking) changes to the API / functionality. Just like API or functionality changes can be subjective in their usefulness, so can renames be controversial or overwhelmingly positive.

I do not disagree that how well identifiers are named is a secondary concern to the functionality that they provide. But this does not mean that we should ignore the quality of the names, and furthermore, reject any attempts to improve them.

6.

Concerning the naming itself.

My involvement comes from when my PR to rename setExt to withExtension was closed.

I would like to present a very similar case in another language, JavaScript.

The String method has two functions with a similar name and functionality: "substr" and "substring". If you were to search the web, you can find a multitude of confusion over these functions:

http://stackoverflow.com/questions/3745515/what-is-the-difference-between-substr-and-substring
https://nathanhoad.net/javascript-difference-between-substr-and-substring
http://javarevisited.blogspot.com/2013/08/difference-between-substr-vs-substring-in-JavaScript-tutorial-example.html
https://rapd.wordpress.com/2007/07/12/javascript-substr-vs-substring/
https://www.youtube.com/watch?v=OAameXW5r10

I think it's safe to say that it's one of JavaScript's many small warts.

The closest analogy with the case at hand is the toLower/toLowerCase functions. It would be unfortunate if we were to have such warts in D, so I think we should at least not outright reject PRs which fix them.

On Saturday, 20 June 2015 at 09:27:16 UTC, Vladimir Panteleev wrote:
> Naming things
>
>       There are only two hard things
>       in Computer Science: cache
>       invalidation and naming things.
>       -- Phil Karlton
>
> Hello,
>
> There has been a lot of recent debate regarding the names of some new functions recently added to Phobos.
> ...

I'd like to note my support for consistent/standardized naming (especially with regards to `withExtension`/`setExt`). As an end-user of D, it's very important to me that a precedent be set for naming prior to D releases - it means less time spent upfront perusing documentation, and less time spent trying to understand how a particular function works. When I'm viewing code "in the wild," so to speak, time spent trying to understand byzantine names is time wasted.

In this particular case, `withExtension` is objectively better than `setExt.` The `with` prefix indicates lazy operation - this is a much better cue as to the function's operation than the truncation of an already-existing name. Truncating the name will, without a doubt, lead to user confusion: these two functions have the same goal, but operate in fundamentally different ways, and the name should reflect this. The alphanumerical sorting argument has little validity, especially seeing as the "See Also" section serves the same purpose.

I understand that the community's been beset with naming discussions for the longest of times - and yes, they can often be non-productive - but there are some cases in which it is very much worth the time choosing a better name. `setExt` is objectively confusing and uncommunicative of its actual functionality - and it can be fixed now, before it becomes a permanent wart.

As a final note, naming conventions are very important for the end-user of a programming language. If one goes with a 'pick the first name that works' approach, the result is a very unproductive, contradictory language; an extreme example of this can be seen in PHP, where programmers often have to consult the documentation for *every* function to find the correct name for every function. We have the ability to prevent that from happening here.

On Saturday, 20 June 2015 at 09:27:16 UTC, Vladimir Panteleev wrote:
> I would like to present a very similar case in another language, JavaScript.
>
> The String method has two functions with a similar name and functionality: "substr" and "substring". If you were to search the web, you can find a multitude of confusion over these functions:
but see how you will hit some of that links by searching for javascript substr or something like it! what that gives is more popularity: everyone wants to write an article for such a hot, yet easy topic. thus, having the same naming in D inevitably leads to more articles about D, and to increased popularity!

On 6/20/15 5:27 AM, Vladimir Panteleev wrote:
> Naming things
>
>        There are only two hard things
>        in Computer Science: cache
>        invalidation and naming things.
>        -- Phil Karlton
>

I think the issue we are struggling with here is that:

abbreviate -> abbreviated

This makes some sense. However, the past participle of "set" is "set".

So "set the extension" -> "a set extension" doesn't work, because "set" doesn't change. Our enemy here is the English language :)

If the original was named something like "modifyExt", then "modifiedExt" would be fine.

And my understanding of the pushback from Walter about renaming really has to do with avoiding breaking code for the sake of renaming. At this point (before setExtension has ever been released), it's "what is the best name". No code should be broken, the renaming objection shouldn't apply. I'm 100% in favor of not having both setExt and setExtension to mean different but similar things.

withExt seems better, and reasonably informative.

path.withExt("abc") -> "use this path, but with extension '.abc'"

And 'with' doesn't work with every possible updated version, we have to work around the quirks of English here.

But really, the egregious error is the slightly different yet identical names. It's like having setExt and set_ext mean different things. This also reminds me of std.regex vs. std.regexp. I never knew which one was the "new" version.

If there is some other idea besides setExtension (or some prefix of that), I think we should go with that. We could use some analog to 'set', like 'modifiedExt' or 'changedExt' if that sounds better. It just shouldn't be the same exact name, with differing levels of abbreviation.

-Steve

On Saturday, 20 June 2015 at 18:46:45 UTC, Steven Schveighoffer wrote:
> I think the issue we are struggling with here is that:
>
> abbreviate -> abbreviated
>
> This makes some sense. However, the past participle of "set" is "set".
>
> So "set the extension" -> "a set extension" doesn't work, because "set" doesn't change. Our enemy here is the English language :)
>
> If the original was named something like "modifyExt", then "modifiedExt" would be fine.

I would understand that if it was part of a consistent pattern for new names. However, judging by toLower/toLowerCase, there is none - and, if I understand Walter Bright's argument correctly, he argues that since we were not consistent in naming when creating std.algorithm, there is no reason to be consistent about it now.

> And my understanding of the pushback from Walter about renaming really has to do with avoiding breaking code for the sake of renaming. At this point (before setExtension has ever been released), it's "what is the best name". No code should be broken, the renaming objection shouldn't apply. I'm 100% in favor of not having both setExt and setExtension to mean different but similar things.

Just to clarify, it's still not too late to rename setExt.

> And 'with' doesn't work with every possible updated version, we have to work around the quirks of English here.

I think just using different verbs/prepositions would work. For example, "asLowerCase".

> But really, the egregious error is the slightly different yet identical names. It's like having setExt and set_ext mean different things. This also reminds me of std.regex vs. std.regexp. I never knew which one was the "new" version.

At least that was temporary. This is going to be set in stone once 2.068 rolls out.

On 06/20/2015 05:27 AM, Vladimir Panteleev wrote:
> [...]

+1kazillion

On Saturday, 20 June 2015 at 09:27:16 UTC, Vladimir Panteleev wrote:

> Two examples of controversial name pairs: setExt/setExtension, and toLower/toLowerCase. These functions have the same functionality, but one of them is eager, and the other is lazy. Can you guess which is which?

Yikes!  That should have never passed scrutiny in the pull request.  I'm sorry I didn't see it, as I would have voiced opposition to it.  I only just started monitoring Phobos this week.  My work doesn't really require me to use Phobos much.

> It would be unfortunate if we were to have such warts in D, so I think we should at least not outright reject PRs which fix them.

I totally agree.  I was really excited about D a year and a half ago, and what really lit my fire was Andrei's talk about "Operational Professionalism" at DConf 2013.  At that time, I thought, "Wow, this community really cares about getting things right".  How naive of me :)  But, I'm still here...perhaps foolishly.

It makes me disappointed to see contributions that dot the 'i's and cross the 't's get turned away and belittled.  But, I don't think there's anything I can do about it, and although you've made an excellent argument, I've gathered enough wisdom in my time here to make a reasonable prediction of the outcome.  I also believe the relatively little response you've received in this thread is likely not an indication that few care, or that few support your argument, but rather that they've seen it before, and they know how it ends.

Mike

On 22/06/2015 7:17 p.m., Mike wrote:
> On Saturday, 20 June 2015 at 09:27:16 UTC, Vladimir Panteleev wrote:
>
>> Two examples of controversial name pairs: setExt/setExtension, and
>> toLower/toLowerCase. These functions have the same functionality, but
>> one of them is eager, and the other is lazy. Can you guess which is
>> which?
>
> Yikes!  That should have never passed scrutiny in the pull request.  I'm
> sorry I didn't see it, as I would have voiced opposition to it.  I only
> just started monitoring Phobos this week.  My work doesn't really
> require me to use Phobos much.
>
>> It would be unfortunate if we were to have such warts in D, so I think
>> we should at least not outright reject PRs which fix them.
>
> I totally agree.  I was really excited about D a year and a half ago,
> and what really lit my fire was Andrei's talk about "Operational
> Professionalism" at DConf 2013.  At that time, I thought, "Wow, this
> community really cares about getting things right".  How naive of me :)
> But, I'm still here...perhaps foolishly.
>
> It makes me disappointed to see contributions that dot the 'i's and
> cross the 't's get turned away and belittled.  But, I don't think
> there's anything I can do about it, and although you've made an
> excellent argument, I've gathered enough wisdom in my time here to make
> a reasonable prediction of the outcome.  I also believe the relatively
> little response you've received in this thread is likely not an
> indication that few care, or that few support your argument, but rather
> that they've seen it before, and they know how it ends.
>
> Mike

I haven't commented yet but you have half hit a nerve for me.
Okay so, I'm in agreement about the point being made 100%.
But there isn't really anything I can _do_ about it.
I don't really review Phobos PR's.

Just a thought, add a checklist to CONTRIBUTING.md on Github and have this as one of them for function/method names.
It probably just slipped peoples minds. Problem solved.

On Monday, 22 June 2015 at 07:25:19 UTC, Rikki Cattermole wrote:

> But there isn't really anything I can _do_ about it.

See the sentence before that.

> I don't really review Phobos PR's.

That's not at all what I said.

On Saturday, 20 June 2015 at 09:27:16 UTC, Vladimir Panteleev wrote:
>
> Two examples of controversial name pairs: setExt/setExtension, and toLower/toLowerCase. These functions have the same functionality, but one of them is eager, and the other is lazy. Can you guess which is which?

I've taken a look at the offending PR (https://github.com/D-Programming-Language/phobos/pull/3370).  Unfortunately, it was only up for review for about a day before it was merged, and only one person commented on it before it was merged.

Given that some PRs are currently rotting for months, I hate to say that PRs should remain open for a minimum amount of time, but that's what I'm saying.  We're all very busy, so give PRs a few days for reviewers to get to them.  IMO, that PR should have never passed scrutiny, and probably wouldn't have if it was given a little more time.


Mike