Bug 18124 – std.regex.RegexMatch's front property is under-documented
Status
RESOLVED
Resolution
FIXED
Severity
trivial
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2017-12-25T03:16:25Z
Last change time
2018-01-03T07:32:28Z
Keywords
ddoc
Assigned to
No Owner
Creator
|
Comments
Comment #0 by dhasenan — 2017-12-25T03:16:25Z
auto return types are terrible for documentation. You need to include the full documentation of the return type in the function description, or you need to separately link to the return type. It's a lot nicer and more efficient to simply fill in a concrete return type (when not using voldemort types).
std.regex uses `auto` as a return type rather a lot. Sometimes it has okay documentation for what the return type is. Sometimes it doesn't. PR incoming.
Comment #1 by greensunny12 — 2017-12-25T17:02:21Z
Advantage of `auto`:
- we can change the API
- less visual clutter (that's subjective though)
See the discussion at https://github.com/dlang/phobos/pull/5963
We have to be careful here, this a bikesheeding topic and a lot time can be wasted with discussing the pros and cons.
A good improvement, however, could be to improve Ddoc or documentation, s.t. it helps the reader more what `auto` could be mean and whats capabilities one can expect from it. Please feel free to open a respective issue with concrete for dlang.org ;-)
Comment #2 by dhasenan — 2017-12-25T17:37:33Z
I wasn't aware that a decision to use the type system was bikeshedding.
Just a tip: emoticons can make an otherwise professional comment seem smarmy or condescending. If you do not intend to seem smarmy or condescending, you may wish to avoid them.
Comment #3 by greensunny12 — 2017-12-26T00:40:04Z
> I wasn't aware that a decision to use the type system was bikeshedding.
Okay let me go one step back here: I do see your point of explicitly stating the return type, s.t. it's one click away from the user. However, this comes up regularly and as you have seen in your recent NG thread [1] this is a topic with divided minds.
Regarding bikeshedding:
> spending the majority of its time on discussions about relatively minor but easy-to-grasp issues, such as what materials to use for the staff bike shed, while neglecting the proposed design of the plant itself, which is far more important and a far more difficult and complex task.
Maybe it's a better of interpretetion.
[1] http://forum.dlang.org/post/[email protected]
[2] https://en.wikipedia.org/wiki/Law_of_triviality
> Just a tip: emoticons can make an otherwise professional comment seem smarmy or condescending. If you do not intend to seem smarmy or condescending, you may wish to avoid them.
This wasn't my intention. Thanks for the advice!
(FYI: I'm doing most of open source work on my phone, so typos arise from time to time and emoticons are in close reach.)
Comment #4 by schveiguy — 2017-12-30T15:25:08Z
(In reply to Seb from comment #1)
> Advantage of `auto`:
> - we can change the API
No, you can't change the API. People depend on the API being the way it is. You can change the result's innards (read: private members) whether it's auto or not, but what auto does allow is changing the NAME of the result, and not break any code. In other words, it forces you to use auto when receiving the result. But this is only for private or voldemort types. In this case, we are defining the type publicly, so using auto here just avoids writing out the type itself.
Another advantage of auto is when you want to return different types based on static introspection. This allows a much DRYer mechanism than somehow generating the return type (and the result can look a lot worse than using auto).
> - less visual clutter (that's subjective though)
Well, we need to describe what the result actually can do. In this case, it seems the result is under-documented, so we should do *something* to fix that. Looking at the code, it seems it's returning a defined struct in the file itself, so we should just document it properly.
(In reply to Neia Neutuladh from comment #2)
> Just a tip: emoticons can make an otherwise professional comment seem smarmy
> or condescending. If you do not intend to seem smarmy or condescending, you
> may wish to avoid them.
Just another tip: you may want to avoid taking unnecessary offense at innocuous things for no reason. Most people in our organization are friendly and cheerful people, including Seb. Thanks! ;)
Comment #5 by dhasenan — 2017-12-30T18:08:06Z
(In reply to Seb from comment #1)
> A good improvement, however, could be to improve Ddoc or documentation, s.t.
> it helps the reader more what `auto` could be mean and whats capabilities
> one can expect from it.
std.regex.Captures has fourteen public members. You think it is better to document its public members in std.regex.match *and* std.regex.matchFirst than to just point to the types? That's an interesting perspective.
(In reply to Steven Schveighoffer from comment #4)
> what auto does allow is changing the NAME of the result
Aliases also allow changing the name of the type even if you use concrete types. It's just that you need a deprecation cycle.
So if you think there's a strong risk you *absolutely must* have another struct in std.regex named Captures in the next Phobos release, sure, keep the existing one private and use `auto` return types. If you merely think you might get a better name for it in the near future, you can change it then and add a deprecated alias for the existing name. Or, more likely, you'll be told that changing the name is bikeshedding and that you should keep it as is -- which is generally a good idea.
A slightly more salient change is that you might move a type so it's more widely accessible. Perhaps a year from now we'll want to use a Captures object for a std.string.find function. A public import if the name is kept the same, or an alias if it is not, functions just as well as using an auto return type.
The only change that it possibly lets you do is change between several existing types instead of committing to the same type for each.
In this case, the implementation for std.regex.matchFirst is a manually inlined version of
return matchMany(args).front;
That makes it unlikely that the types will become incompatible.
> Just another tip: you may want to avoid taking unnecessary offense at
> innocuous things for no reason. Most people in our organization are friendly
> and cheerful people, including Seb. Thanks! ;)
In point of fact, what I experienced was _anger_, not offense. Detached professionalism doesn't give anger a target.
By reiterating a behavior I just complained about in response to me complaining about it, it looks like you are trying to deliberately antagonize me. This is a concept that most of my cohort had grasped by second grade. I believe you've been in the D community as long as me, which means you should be an adult, or near enough.
Do you have some sort of social disability that makes it difficult for you to learn things like this? If so, I'll try to take that into account in our future interactions.
Comment #6 by schveiguy — 2017-12-30T18:32:07Z
(In reply to Neia Neutuladh from comment #5)
> If you merely think
> you might get a better name for it in the near future, you can change it
> then and add a deprecated alias for the existing name.
I think we can keep things as is, but just document that RegexMatch.front returns a Captures struct. After all, Captures is already documented.
> A slightly more salient change is that you might move a type so it's more
> widely accessible. Perhaps a year from now we'll want to use a Captures
> object for a std.string.find function. A public import if the name is kept
> the same, or an alias if it is not, functions just as well as using an auto
> return type.
I don't think we need to move types around yet. When I look at std.regex docs, Captures has a lot of documentation, we just don't have the link between RegexMatch.front and the struct it returns (a downside of auto here). We just need to make that link.
> > Just another tip: you may want to avoid taking unnecessary offense at
> > innocuous things for no reason. Most people in our organization are friendly
> > and cheerful people, including Seb. Thanks! ;)
>
> In point of fact, what I experienced was _anger_, not offense. Detached
> professionalism doesn't give anger a target.
>
> By reiterating a behavior I just complained about in response to me
> complaining about it, it looks like you are trying to deliberately
> antagonize me. This is a concept that most of my cohort had grasped by
> second grade. I believe you've been in the D community as long as me, which
> means you should be an adult, or near enough.
Wow! It's a smiley, it literally is just a way to put a happy, friendly face on a comment. You were angry because someone typed a smiley and you grossly misinterpreted it? Have you used the Internet before?
> Do you have some sort of social disability that makes it difficult for you
> to learn things like this? If so, I'll try to take that into account in our
> future interactions.
Yes, I learned to get over trivialities that are obviously not meant for offense before they made me angry. Pretty much around the same time (2nd grade you say?) you were learning... whatever it is you are calling tantruming here.
By all means, we can stop this OT conversation, but let's get some good work done and move on.
Comment #7 by dhasenan — 2017-12-30T18:41:57Z
(In reply to Steven Schveighoffer from comment #6)
> Yes, I learned to get over trivialities that are obviously not meant for
> offense before they made me angry.
In other words, the other boys were jerks at you, and you learned to ignore it, so you're a jerk in response.
> By all means, we can stop this OT conversation, but let's get some good work
> done and move on.
If you wanted to stop having the OT conversation, you could have just...not replied to that part.