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

Consider changing "diff" to "diff" and "diffOrNull" #339

Closed
koperagen opened this issue Apr 4, 2023 · 3 comments
Closed

Consider changing "diff" to "diff" and "diffOrNull" #339

koperagen opened this issue Apr 4, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@koperagen
Copy link
Collaborator

Right now diff can return null, but i'd argue that in some situations it's not desirable and people will write something like
diff { } ?: defaultValue
But it's not that obvious that diff returns null at all and personally i find out about it only after my code explodes
So, i think diff should have value parameter default and return non-null, and then we introduce a special version diffOrNull
It can even be nicely deprecated, as we add a new parameter, so we don't silently break anything

@koperagen koperagen changed the title Consider change "diff" to "diff" and "diffOrNull" Consider changing "diff" to "diff" and "diffOrNull" Apr 4, 2023
@Jolanrensen Jolanrensen added this to the 0.11.0 milestone Apr 6, 2023
@zaleslaw zaleslaw added the enhancement New feature or request label Apr 25, 2023
@koperagen
Copy link
Collaborator Author

I want to move this to backlog, because deprecation works really strange for this one and it needs to be fixed on the IDE side . I think this improvement without proper deprecation will bring more harm than use
image
image

@koperagen koperagen modified the milestones: 0.11.0, Backlog Jun 19, 2023
@Jolanrensen
Copy link
Collaborator

I don't think that's much of an issue. I face the same with some of my functions in the Selection DSL. This auto-generated replacement code can be shortened back to a lambda using IDE hints anyways, and for people using other IDEs, where ReplaceWith doesn't even work, they have to fix the deprecations manually anyways.

@koperagen
Copy link
Collaborator Author

IDE quick fix for conversion to lambda is a good point, i didn't think about it. Well, in that case, i guess i'll fix it in this release

@koperagen koperagen modified the milestones: Backlog, 0.11.0 Jun 19, 2023
koperagen added a commit that referenced this issue Jun 20, 2023
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

3 participants