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

Do not automatically cast scalar types in "strict" mode #101

Closed
template-provider opened this issue Aug 10, 2021 · 3 comments
Closed

Do not automatically cast scalar types in "strict" mode #101

template-provider opened this issue Aug 10, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers Hacktoberfest Issues that are limited in size and are fit for Hacktoberfest

Comments

@template-provider
Copy link

It should be possible to throw an exception with incorrect scalar type casting.

Currently the scalar type casting is automatically depending on typed properties|doc block

public int $counter;

If the json returns a string, and not a number an exception should/could be thrown

I like a strict type mapping and not an automatically casting of the values.
For Implementing a REST API and json mapping it is easier to see a possible response problem at once (false return type).
A Feature with a strict mode casting would be nice.

@DannyvdSluijs
Copy link
Member

Welcome @template-provider and thanks for taking the opportunity to report an issue for adding support for strict type casting.

Would you be creating a PR for this? Or can anyone pick this up as they see fit?

A consideration I'm having is to make this available using configuration or feature flag to avoid current implementations to break on trivial type casting (e.g. casting string "12" to int 12) which was my intended goal. However I can see the opposite side here and will working on your local development you might want some stricter type casting. We should also consider union types where only after all specified types have bene tried without result throw an exception.

@template-provider
Copy link
Author

hi,
my workload is currently huge, but if I got some time, I will dig into it.
But that will take some time.
If you can do this in the meantime, that would be great.

A configuration parameter is imho the best choice.
As far as I can see now, the ScalarType Class is created directly, so if we can do that over the factory we can use a StrictScalarType ... or something like that.

@DannyvdSluijs
Copy link
Member

Although your approach seems logical the ScalarType is an enum and used throughout the code as indicators for the type it is difficult to replace it.

But given that the cast method is only used in the PropertyMapper and the test of the ScalarType the method could be extracted into a Caster of sorts which can be a property of the PropertyMapper. In turn this is defined as interface which can have a concrete implementation StrictScalarCaster. That would throws the exceptions you're looking after and a regular ScalarCaster which implements the current behaviour.

That way the steps for this would be:

  • Introduce an IScalarCasterHelper
  • Add IScalarCasterHelper as an attribute to the PropertyMapper and refactor the current cast calls to use the interface.
  • Move ScalarType::cast functionality to the ScalarCaster
  • Introduce the StrictScalarCaster and its behaviour
  • Ensure all test will keep working e.g. the ScalarCaster is the default behaviour
  • Introduce new test that show the StrictScalarCaster to work accordingly

Hope this list of steps is helpful for anyone to get started on the issue.

@DannyvdSluijs DannyvdSluijs added enhancement New feature or request good first issue Good for newcomers Hacktoberfest Issues that are limited in size and are fit for Hacktoberfest labels Oct 27, 2021
@DannyvdSluijs DannyvdSluijs self-assigned this Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Hacktoberfest Issues that are limited in size and are fit for Hacktoberfest
Projects
None yet
Development

No branches or pull requests

2 participants