Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static create method #47

Open
djgadd opened this issue Aug 24, 2015 · 6 comments
Open

Static create method #47

djgadd opened this issue Aug 24, 2015 · 6 comments
Assignees

Comments

@djgadd
Copy link

djgadd commented Aug 24, 2015

Thanks for the solid library.

What do you think about a static create method in the AbstractPhysicalQuantity class? Allowing: -

Length::create(6.16, 'feet')->toUnit('m');

It's obviously nothing major, I've just got a few cases where I begrudge having an extra line just to declare the Length object when all I want is a simple conversion.

@ste93cry
Copy link

👍

@fyrye
Copy link

fyrye commented Jan 17, 2017

In php 5.4+ you really don't need an extra line, you can wrap instantiators in parenthesis to chain method calls.
See: http://php.net/manual/en/migration54.new-features.php

echo (new Length(6.16, 'ft'))->toUnit('m');

@ste93cry
Copy link

This is true, but a static method feels more appropriate and the legibility of the code is better imho

@triplepoint
Copy link
Member

Sorry for not addressing this ticket earlier.

It's a style judgement, and therefore somewhat arbitrary, but I'd prefer to lean toward existing language semantics rather than code around them. @fyrye's suggestion above is what I'd consider to be the recommended procedure for a one-liner unit conversion.

@ste93cry
Copy link

I would like to give an example of when the static method could be useful: in some cases you have to instantiate a lot of Length objects in a loop, maybe using array_map (think maybe to create instances out of a SQL result set). This is not possible without a static method or a lambda function that invokes the constructor, and it would be prettier to write something like array_map([Length::class, 'create'], ...)

@triplepoint triplepoint reopened this Jul 26, 2017
@triplepoint
Copy link
Member

Hmm, ok it looks like there are legit use cases here, and the added complexity is minimal. I'm ok with implementing this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants