Single responsibility constructors

[dcousens]

At this time, many of the constructors (or constructing functions) for ECKey, Address, and probably most of the modules, all accept a variety of parameter types, with branching to handle each possibility.

This goes against the separation of concern principles for these functions, and ensures that each function has the responsibility of correctly handling each possible set of inputs; which may be out of order.

A possible solution would be that rather than having catch-all constructor/builder functions, we could better separate the functions into their own responsibilities.

That is, rather than supply the developer with the behemoth: ECKey(hex | base64 | WIF | byteArray | base58) etc, we provide: ECKey.fromWIF, ECKey.fromBuffer and so on.

This allows for simpler verification and testing of these functions also.

[abrkn]

My thoughts exactly. Was just typing to Wei:

[10:10:14] Andreas Brekken (Justcoin): but seriously, fuck absolutely everything about that ECKey constructor
[10:10:38] Andreas Brekken (Justcoin): it should only have one version, which takes in bytes (eventually buffer?)
[10:10:46] Andreas Brekken (Justcoin): everything else needs to be static methods or helpers

[kyledrake]

I did want to say briefly here, that I agree with the single constructor approach, but I think the detection component of this is pretty neat. It might be fun to keep the constructor code as a sort of auto-detect function people could use if they weren't quite sure. Maybe it could be refactored to do a fromUnknown or autoDetect function or something like that.

[dcousens]

@kyledrake I can't agree with that.
There is only two formats that are in use by the community at large: HEX and WIF.
One has a length of 64 and the other is base58check encoded.
If you aren't sure which is which, you can trivially write up your own detection function (Hint: if base58check.decode fails, try hex).

If a service wants to support Base64or Base6, then they should be easily capable of writing a convenience function to choose the correct constructor.

The only type I am on the fence about is MINI.
This format is different in that it is import only and not export, which seems so special cased anyway.

[kyledrake]

If the auto-detect is indeed not as useful as you believe then I agree that it's not a big deal.