Use accessors macros for lazy initialization

[Sija]

I've noticed an opportunity to reduce boilerplate code with accessors macros.
Not a big deal and you can call it pedantic but imo it improves code readability and cuts down on footprint and eye-strain ;)

[ysbaddaden]

I didn't know about initializers in property macros, and I don't like it, I prefer the explicitness of actual methods.

[Sija]

@ysbaddaden To me it's weird at least since it's a prime example of dogfooding, lazy block initialization is in stdlib since some time already and found its way into the codebase throughout the place, including compiler. Most recently I've seen it being used by @RX14.

Attitude like this IMO will lower confidence in stdlib features for no reason other than your personal preference. That's sad since you do great job on technical side, yet it seems to me that you treat shards as (only) your personal project and accept almost no external input, or at least discussion regarding the style or new features.

[ysbaddaden]

There are so many things to do to improve Shards. Let's focus on actual issues such as #1, #56, #58, #106, #124 and the v0.9.0 milestone rather than arguing endlessly on opinion based stylistic changes.

[Sija]

These things are not mutually exclusive. And I'm not arguing, I'm just trying to use the features that Crystal's stdlib provides, in order to cut down on boilerplate code...

I mean c'mon, why choose the unnecessarily longer way...

class Foo
  @foo : String?

  def foo
    @foo ||= compute_foo
  end

  def foo=(@foo)
  end
end

# vs

class Foo
  property(foo : String) { compute_foo }
end

[asterite]

I agree with @Sija . But I also agree with @ysbaddaden : this is mostly his project (even though it's in the crystal-lang organization) and mostly managed by him, so it's fine if he prefers to use the old well known constructs for this.

[RX14]

@ysbaddaden is the maintainer of shards, and has been doing a fantastic job of it. I'm more than happy for him to set the style of the whole project. Arguing that it will "lower confidence in stdlib features" is a big stretch.

[Sija]

@ysbaddaden is the maintainer of shards, and has been doing a fantastic job of it. I'm more than happy for him to set the style of the whole project.

From the technical perspective I'd fully agree, yet OSS is more than one person's viewpoint. What bothers me is lack of openness, discussion and collaborative approach to anything that @ysbaddaden doesn't agree with (i.e. closing PRs with nothing more than "I don't like it." response). I admit that's his right, no one said you ought to act nice towards contributors, it's just that such attitude is disenchanting. Summing up this already long comment, it's not that much about why, but how.

Arguing that it will "lower confidence in stdlib features" is a big stretch.

Perhaps, yet seeing willingly divergent approaches under an umbrella of one organization begs asking question why is it so? If another maintainer from the same org happens to purposefully avoid certain features from stdlib, one might conclude that there's sth wrong with the features themselves... Lack of cohesiveness from within the core team might throw a shadow on the organization/language itself.

[asterite]

Well, if you don't know anything about getter(...) { } it's a bit hard to figure out that it does memoization and so on. It's not very intuitive. You need to learn something new to understand that code. The current method is maybe clearer. So that might be one reason.

[j8r]

There is Hash#fetch(key, &block) that ressembles in the behavior – executing a block when the variable/key is nil/undefined.

[ysbaddaden]

I apologize for closing so fast after barely looking at the patch.

I'm not saying this is bad, it may eventually be the standard way to lazy initialize class/instance variables safely in a MT Crystal (assuming it's updated to deal with sync/races). I'll be happy to make the change then. Maybe I'll even adopt it in new projects or when we'll rework Shards internals —which are messy and not suited for a library.

But today? This is just the same. No benefit to the Shards application. It's a mere stylistic (thus opinionated) change that will pollute the Git history... again.

Having one stylistic change ain't much, but having more and more makes digging through the history (like searching for why was @update_cache implemented and when did it becomes obsolete) becomes time consuming.

I'm closing stylistic refactors fast and barely look at the patch. Let's make these changes as we rework the code instead, that is along with an actual technical enhancement (app feature, API rework).