std.getopt suggestion

[Andrei Alexandrescu]

On 9/29/11 3:17 AM, Jonathan M Davis wrote:
> On Thursday, September 29, 2011 00:40:44 Andrei Alexandrescu wrote:
>> Why?
>
> 1. Mutable globals are generally considered to be bad practice. As you
> yourself have stated before, Phobos should be an example of good software
> practices in D. Having mutable globals goes directly contrary to that goal.

Globals are bad when functions communicate through them. std.getopt is 
one function, which is overwhelmingly called once. The variables that 
condition its workings are unrestricted. It is poorer style to create an 
object just for the sake of calling a method for it. GetOpt doesn't even 
make sense as a noun. It would be something like OptGetter - ah, the 
stigma of "er"-ending objects.

> 2. Conceptually, what happens with getopt is that you configure it and then run
> it. It is much better encapsulation for the configuration to be tied to the
> function call. The normal way to do this (at least in an OO language) is to
> make it a member function of the configuration. As it stands, those
> configuration variables are just sitting in the same module. As getopt is
> really the only function of consequence in the module, it's not as big a deal
> as it would be in most modules, but it's still better encapsulation to
> actually tie the configuration to the function that it's configuring.

Encapsulation makes sense when there's matter to encapsulate. I don't 
see a point in encapsulating one function that's called once and two 
unrestricted variables.

> 3. By putting the configuration options in a struct, they are better organized.
> It makes it easier to see what the whole list is without having to search the
> module. It also gives a very good place to document them all together.

Not buying this at all. It's a module with one blessed function!

> 4. If you need to run getopt multiple times - particularly if you need to run
> it with different configurations each time - it's definitely cleaner to do that
> when you can just use a different GetOpt instance in each case. You don't have
> to worry about one run of getopt affecting another. Now, this matters far less
> for getopt than it would your average function, since it would be highly
> abnormal to need to run getopt multiple times like that, but the general
> design principle holds.

It would be quite abnormal to run getopt multiple times in the same app 
with different configurations. So in this case using globals is 
_better_, not worse.

> 5. Assuming that we were creating std.getopt from scratch, there would be
> _zero_ benefit in having any of its configuration options be at the module
> level.

If I were creating std.getopt today from scratch, I'd define it the same 
way it is. It is simple, to the point, and gets its job done. It 
compares favorably with all getopt frameworks I've ever seen. I am very 
happy with its design and implementation.

> There is a definite argument for leaving them there given that moving
> them could break code (though honestly, it wouldn't surprise me if no one in
> the history of D has ever written a program that changed any of those
> variables from their defaults given how standard they are and how little gain
> there normally is in changing them), but from the perspective of design, I
> don't see any reason why it would ever be better to have the variables be at
> module scope. On the contrary, it goes against what is generally considered
> good design.

Not buying this at all. I've seen "good" and "better" a lot of times in 
this email, but not properly substantiated for the case at hand. The 
design of a simple artifact should be simple. Introducing an object to 
obey design principles not applicable to the case at hand strikes me as 
futile.

> 6. Putting the configuration in a struct would probably allow getopt to be pure
> (depending on its implementation). I don't know if there would ever be a
> program where that would really matter given how getopt is normally used, but
> it would be a benefit to having the configuration encapsulated in a struct
> rather than at module scope. And as it stands, getopt definitely can't be pure,
> so if it ever did matter, it would be a problem.

This is a good argument.

> Honestly, I think that it primarily comes down to it being better design to
> encapsulate the configuration options together, tying them to the function that
> they're for, rather than having mutable variables at module scope. And Phobos
> should not only be well-designed on general principle, but it's supposed to be
> an example good D code and practices, and as it stands, std.getopt doesn't do
> that with regards to module-level variables. The only reason that I see to
> leave it as it is is because it's already that way.

I see another reason: it works very well and it keeps simple things simple.

> And if we mess with what's currently there in order to change any of the
> defaults for the config enum (as opposed to the variables at module scope which
> we've been discussing) and/or to change getopt to getOpt to follow Phobos'
> naming conventions, it just makes sense to change anything about the design
> which is suboptimal which can be reasonably changed.

The proposed change adds net negative value. It forces people to create 
an object in order to call a simple function, for the vague benefit of 
tenuous corner cases.

I kindly suggest we stop the isometric workout and look into adding good 
value to Phobos.


Thanks,

Andrei