Skip to content

Conversation

@danielkroeni
Copy link

I removed CornerRadiiConverter and marked some native data declarations as "pure".
Unfortunately I am not a git professional and all changes come in a single pull request. Just tell me if (and how) I should organize this differently.

I think looking for enums would be a good heuristic to identify immutables. Am I right that their valueOf method should be be declare :: IllegalArgumentException | EnumType ?

Daniel Kroeni added 5 commits September 30, 2015 23:17
CornerRadiiConverter has moved to
com.sun.javafx.scene.layout.region.CornerRadiiConverter in Java 1.8.0_60
@Dierk
Copy link
Member

Dierk commented Sep 30, 2015

Hi Dani,

thanks for the PR.

I removed CornerRadiiConverter and marked some native data declarations as "pure".

I just did the same thing :-)

Unfortunately I am not a git professional and all changes come in a single pull request. Just tell me if (and how) I should organize this differently.

No problem, I can sort that out.

Identifying purity is a bit involved though, and will need more investigation on my side.
For example, all events are mutable in JavaFX :-( since they have a "consumed" state.

• valueOf throws IllegalArgumentException

well spotted! Thanks.

I'll most likely not apply the PR at once, but in pieces.
It will also take a bit of time since I am busy/unavailable until mid Oct.

thanks a lot
Dierk

@danielkroeni
Copy link
Author

Hi Dierk

Identifying purity is a bit involved though, and will need more
investigation on my side.
For example, all events are mutable in JavaFX :-( since they have a
"consumed" state.

Ouch! I see. This means the commit "KeyEvent and EventType are immutable"
2c1c1d1 must be thrown away.
I hope at least Color and KeyCode are safe.

BTW I just found that KeyCode has another Method which could be improved to
return Maybe:
pure native getKeyCode "javafx.scene.input.KeyCode.getKeyCode" :: String ->
Maybe KeyCode

It just realize how labor-intensive it is to frege-ify such an enormous
library.

I'll most likely not apply the PR at once, but in pieces.

Sure. I understand that it needs more careful thought. Feel free to just
reject / ignore this request.

It will also take a bit of time since I am busy/unavailable until mid Oct.

Okay, no hurry.

Thanks
Dani

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants