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

Add lazy versions of Object#getter?/property? macros #7322

Merged
merged 1 commit into from
Feb 16, 2019

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Jan 18, 2019

This PR adds lazy initialized versions of Object#getter?/property?. They do exist for getter/property macros, so that would bring feature parity to both variants.

Possibly closes #7321

@ysbaddaden
Copy link
Contributor

A foo? getter with a lazy initializer will always initialize a value and never return nil, whatever if I did set it to nil. Or am I missing something?

At least, it should memorize whether the value has been initialized (which can be nil).

@Sija
Copy link
Contributor Author

Sija commented Jan 19, 2019

A foo? getter with a lazy initializer will always initialize a value and never return nil, whatever if I did set it to nil. Or am I missing something?

I'm not sure if I follow... Using nil-able types for any of the property/setter macros would result in re-initialization of the value inside getter on every call. This PR doesn't change that. IMHO that's a very rare edgecase, there's no need to support, at least not atm.

class Foo
  getter?(installed : Bool) { true }
end

foo = Foo.new
p foo.@installed # => nil
p foo.installed? # => true
p foo.@installed # => true

foo.installed = nil
p foo.@installed # => nil
p foo.installed? # => true
p foo.@installed # => true

@ysbaddaden
Copy link
Contributor

ysbaddaden commented Jan 19, 2019

That's exactly my issue: what's the difference between getter?(foo) { } and getter(foo) { } in this patch? The foo? method is supposed to return a nilable, but it will always (re)initialize the variable, thus never return a nilable value, which is the behavior of the foo method. That's confusing.

Unless there are actual use cases that I'm obviously missing? The whole patch looks like a very edge case to me. I'm not sure that getter?(foo) { } should compile at all.

@j8r
Copy link
Contributor

j8r commented Jan 19, 2019

Yes I was suggesting what @ysbaddaden says. Else, this ? variants won't add much compared to non-? macros.

src/object.cr Outdated Show resolved Hide resolved
src/object.cr Outdated Show resolved Hide resolved
@j8r
Copy link
Contributor

j8r commented Jan 19, 2019

To know if the nillable variables was lazily initialized with the block, a second boolean variable has to be used.

My use case here is to detect the Init system of the machine. I don't want to repeat the operation, and it can return nil if the init system is unsupported. It shouldn't raise because the program can still work without.

@Sija
Copy link
Contributor Author

Sija commented Jan 19, 2019

The foo? method is supposed to return a nilable, but it will always (re)initialize the variable, thus never return a nilable value, which is the behavior of the foo method. That's confusing.

@ysbaddaden I think there's some misunderstanding about the use cases for getter? methods - as I see it + is stated in api docs - getter? macro is used to generate query methods - usually returning Bool, not as nilable version of regular getter.

@ysbaddaden
Copy link
Contributor

A lazy initialized boolean getter method just makes no sense to me at all. I can't see any use case 😕

@RX14
Copy link
Contributor

RX14 commented Jan 19, 2019

@ysbaddaden a boolean's just another return type, it could be computed from the network, or some other expensive operation which is best done on-demand.

@Sija
Copy link
Contributor Author

Sija commented Jan 19, 2019

@Sija Sija force-pushed the allow-lazy-querying-getter branch from 40d67d3 to f5bdf69 Compare January 19, 2019 19:49
@j8r
Copy link
Contributor

j8r commented Jan 19, 2019

Since #7313 is merged, what brings this implementation vs. regular getter and property, despite the ? at the end of the method name?

@Sija
Copy link
Contributor Author

Sija commented Jan 19, 2019

@j8r As I said before: it brings feature parity (lazy block initialization). ? at the end of the method name is the difference between getter/property and getter?/property?, has been since before, and this PR doesn't change that.

Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

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

Thank you @Sija 👍

@sdogruyol sdogruyol requested a review from asterite February 9, 2019 13:51
@Sija
Copy link
Contributor Author

Sija commented Feb 16, 2019

@asterite Would you care for a review (& merge if all's good)?

@asterite
Copy link
Member

Sure, I'll do it in max 3 days.

@asterite asterite added this to the 0.28.0 milestone Feb 16, 2019
@asterite asterite merged commit fc36b85 into crystal-lang:master Feb 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a purpose for block variants of (class_)getter? macros
7 participants