-
Notifications
You must be signed in to change notification settings - Fork 39
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
List all suported units #98
Comments
I haven't seen the listUnits while submiting my previous post. However, we need to manually register all the unit, is there a way to instiante the registry with all the supported units preregistred ? |
I have writed a class to solved my issue. Maybe it will be a good point of adding it in the library. `class ConversionService {
private function getAllUnitsArray() {
}` |
@proualexandre Would love to have this feature implemented! Perhaps not as a new class, though.
Not yet, there isn't. This can also be added as a feature. 👍 |
@proualexandre After some more thinking about your suggestions, I think that a really simple implementiion of the builder pattern would be the best solution. Since you've already got a start on your concept, would you like me to leave this up to you for a pull request? I know that you originally closed the issue, but I will give it the 4 days I took to reply, for you to reply, before carrying on. 🙂 Here is some scratchpad pseudo-y code as an example of how it might/could look, to help explain what I mean above. # A new public static method could be added to the UnitConverter class. It will
# return a new instance of a ConverterBuilder (this class would have to be
# implemented)
$builder = UnitConverter::createBuilder();
$converter = $builder
# Calculator Production Step Methods
->addSimpleCalculator() # constructs with a simple calculator
->addBinaryCalculator() # constructs with a binary calculator
# Unit Registry Production Step Methods
->addDefaultRegistry() # constructs with the entire set of built-in units
->addRegistryWith([]) # constructs with a set of supplied units
# Final call to the build method that will return the actual UnitConverter
# instance
->build(); |
I'm really happy of your interrest about my suggestion. In my own implementation, I have writed a specific class because it contains also the specific logic of my application but , I am agree with you, for an official integration in the library a specific class is not required. |
@proualexandre Just so I can label this issue properly – are you going to work on a branch & submit a PR, or is this a feature request? |
Yes, I will submit a pull request. |
@jordanbrauer The implementation of the ConverterBuilder and of the fully loaded registry is ready on my local but I have somme rights issue to push on the branch fully-loaded-converter. How can I proceed to submit a pull request ? |
@proualexandre Ah! You will have to fork the project and create your own branch, there. Here is link to the steps to create a PR to a project on GitHub |
I submit the pull request right now. I will be happy if you let me know your though about my implementation. |
Does a method exist to list all the units currently supported by the librarie ? If not, i think implement it will be a good improuvment. I know the listMesurements method of the UnitRegistry class but it seems to list only the kind of unit, like length or speed, but not the unit itself.
The text was updated successfully, but these errors were encountered: