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

Strict mode #143

Open
DevNico opened this issue Oct 29, 2023 · 6 comments
Open

Strict mode #143

DevNico opened this issue Oct 29, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@DevNico
Copy link
Contributor

DevNico commented Oct 29, 2023

I want to propose another build option I'd call strict mode which disables automatically setting fields to null if there is no mapper function available. The warning can be ignored to easily this option would fail the build if a mapper cannot be found.

Sidenote: The implementation should probably find as many errors as possible before failing in the end.

@DevNico DevNico added the enhancement New feature or request label Oct 29, 2023
@tenhobi
Copy link
Member

tenhobi commented Oct 30, 2023

Is there some benefit to doing that? When such a parameter has a nullable type, and even when it has a default value, it still works the same as if not passing anything, or not? 🤔

@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

Syntactically it works yes. But I already see this becoming a source of problems because the warning about setting the value to null can easily be ignored. Even tho a parameter can be nullable if the input wasn't the output probably shouldn't be. This nullability makes such an issue not easy to find later because the app wouldn't error anywhere since the UI is probably built to handle null. An experienced developer might know about this, read the warning and fix the mistake but not everybody does/should know to look out for those warnings.

@tenhobi
Copy link
Member

tenhobi commented Oct 30, 2023

And are you suggesting not to put nulls (when mapping is not found) only to named/optional parameters, where it's okay, or also for positional parameters, where that would be an error?

@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

I think two levels of strictness might make sense.

  1. Never set null unless explicitly set in the MapType
  2. Only set null if the source doesn't exist e.g. the target can only be mapped as null. This would fail if the source has a matching field but a MapType/TypeConverter is missing.

@tenhobi
Copy link
Member

tenhobi commented Oct 30, 2023

I think 2 is how it works now -- when fields match, but no maptype/typeconverter is configured, it fails.

1 can be done probably. Would you want to spec that, then confirm the spec with us, and code it?

@DevNico
Copy link
Contributor Author

DevNico commented Oct 30, 2023

I think 2 is how it works now -- when fields match, but no maptype/typeconverter is configured, it fails.

I'll have to look for an example but I think it just prints a warning and sets it to null. Since this is not a blocker like the TypeConverter feature I'm not sure when I'll get to it (I kinda also abandoned TypeConverters because of other priorities) but I'm open to discussion and we'll see who implements it when. I just wanted to get the suggestion out for discussion :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants