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

Immutable value #15

Merged
merged 9 commits into from Jun 12, 2023

Conversation

hungthai1401
Copy link
Member

Dear @WendellAdriel , @chrisjumptwentyfour ,
This PR instroduces two new features:

  • Immutable variable
  • Cloneable variable

Sometime, I need immutable variable for the business logic so I think they are very helpful.
Greetings and thanks,
Thai

@hungthai1401 hungthai1401 changed the title Feature/immutable value Immutable value Jun 9, 2023
Copy link
Member

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hungthai1401 I like the idea, but I think the cloneable is not needed here.
What would be the diff between doing

$var->clone();
// or
clone $var;

I don't see the benefit of having the cloneable here TBH.

On the Immutable, I think that's an amazing addition but I would rather have that as a Trait where it would add a property:

protected bool $immutable;

And a method immutable that would set this property as true and then in the StrictusTyping trait you would add the behavior.

The issue with creating the new ImmutableStrictus is that every time something gets added we need to add it to both Strictus and ImmutableStrictus and I don't want that.

@hungthai1401
Copy link
Member Author

@WendellAdriel I think it's better if we can clone in fluent way instead of use clone keyword

@WendellAdriel
Copy link
Member

@WendellAdriel I think it's better if we can clone in fluent way instead of use clone keyword

I don't see the benefit here, it would be the same, it won't have any additional benefit so let's remove this

@hungthai1401
Copy link
Member Author

@WendellAdriel okay, about second, could you explain more?

@WendellAdriel
Copy link
Member

@WendellAdriel okay, about second, could you explain more?

@hungthai1401
Put the immutable behaviour already in the StrictusTyping trait that's already used by all the classes.

We don't want to have duplicated logic in Strictus and ImmutableStrictus. The final goal is that all types would have a method ->immutable() that could be called when instantiating a variable:

$myString = Strictus::string('Hello, world!')->immutable();

Calling this would set a property $immutable in the object as true and when that's true, if trying to re-assign the value would throw the exception you created.

@hungthai1401
Copy link
Member Author

hungthai1401 commented Jun 12, 2023

@WendellAdriel Thanks for your help. I have reverted Cloneable and add Immutable

Copy link
Member

@WendellAdriel WendellAdriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hungthai1401 amazing!
Thank you so much for this contribution!
I was thinking of adding immutable variables already and your help with that was great!!!
🔥 💪

@WendellAdriel WendellAdriel merged commit 9e4c23f into php-strictus:main Jun 12, 2023
4 checks passed
@hungthai1401
Copy link
Member Author

@WendellAdriel You are welcome, my bro. I am very happy if my contribution is helpful. Do you have any new ideas?

@WendellAdriel
Copy link
Member

@hungthai1401 it is helpful!!! 💪
We are open to ideas, we want to add support for union types, but we still need to discuss how we will approach that

@hungthai1401
Copy link
Member Author

@WendellAdriel it's an amazing idea. I will follow your discussion

@hungthai1401 hungthai1401 deleted the feature/immutable-value branch June 26, 2023 10:00
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.

None yet

2 participants