Refactor MakeMaker.pm

[mohawk2]

Mainly breaking MM->new up from being nearly 400 lines, to 40ish. Done as part of effort to implement extensions but turns out best way forward on that is orthogonal.

[haarg]

Aside from a couple small nits, this looks pretty good to me.

[Leont]

To quote @ribasushi

As far as the larger ticket - step back explain what you are trying to achieve with this sort of cleanup. So far it only seems to be done for the sake of... cleaning up. Which baffles me.

This whole breaking up the new method is superficial at best. Whether we use one big method or a dozen smaller ones, we're still setting up a god object with unpredictable action at a distance. This is the kind of refactor that makes us feel good because wise men have told us that methods should be small, but doesn't address any of the fundamental design mistakes. All it does is distributing the crap for the sake of dogma.

I'm not convinced that this makes the codebase any better.

[mohawk2]

I went through the 400-line "new" method (which isn't an accurate description of what it does), and couldn't understand what it did. I now understand what the various parts of it do, and claim it is far easier for others to do so as well. We may be stuck with the poor design of it doing too much, because of back-compat (I found modules that rely on current behaviour), but I didn't think we were stuck with incomprehensibe code?

[Leont]

and claim it is far easier for others to do so as well.

The init_* methods are similarly cut up in 19 pieces and I never found that to be particularly helpful. new is currently a long list of code that is run serially but interdepends in non-obvious ways, and after the breakup it is still a long list of code that is run serially but interdepends in non-obvious ways.

I don't oppose refactoring this (in fact I like some of it), but I don't think it should be approached this mechanically.

We may be stuck with the poor design of it doing too much, because of back-compat (I found modules that rely on current behaviour), but I didn't think we were stuck with incomprehensibe code?

You're missing my point entirely.

First of all, its length may be ugly but that's not the same as incomprehensible. I don't believe it is made more comprehensible by moving it around, even if it may be prettier. It's far from being the most difficult part of MakeMaker anyway.

Secondly, after your refactoring it is still poorly designed. IME, having short methods is not a goal by itself, but a typical result of good architecture. This is even more the case for constructors.

But my real point is this: is there anything that is made possible or even easier after this? Is there any concrete advantage to this? This has more costs than your effort in writing it, I'd like to know the benefits.

[sjn]

FWIW, refactoring new() is something I approve, even if it doesn't immediately offer any direct benefits other than increasing the understandability/obviousness of the code.

Refactoring code just to make it easier to understand does IMO not have to be defended, so please accept this pull request! We need more PRs like this.