Add reader: :private option ?

[jmgarnier]

Following poodr book by Sandy Metz, I tend to make internal dependencies private by default. To make my life easier as a developer, I add attr_reader for these instance variables bellow a private keyword.

DSL could look like:

class Thingy
  extend Dry::Initializer::Mixin

  option :my_internal_dependency, reader: :private
end

Plain ruby equivalent:

class Thingy
  def initialize(my_internal_dependency:)
    @my_internal_dependency = my_internal_dependency
  end

  private

  attr_reader  :my_internal_dependency
end

Would you like a PR to add this feature?

[nepalez]

@jmgarnier Hi!

From my point of view, all the initializer should do is... well, initialize dependencies. Both for unit testing and debugging, I'd prefer to have a public access to that features.

Well, suppose you really need hiding those dependencies from public eye. In that case you could use reader: false, and no method reader will be created at all. You still can made coercion via type: SomeCoercibleType and then access a resulting value via variable (@my_internal_dependency) instead of private reader.

Look at it from the other side. In "dry" gems mutability of an instance is not welcome. We are willing to tolerate memoizers to some extent, but after a variable is set, it should not be changed. The only reason to provide a reader for "attribute" is to make it publicly accessible. If you don't want it public, why create a reader method?

May be it could be useful to add a variable protected, but I think this rare case don't worth a special option

Could you provide a case where such an approach is not acceptable for you?

[AMHOL]

@nepalez I like the idea and think that it should actually default to private, with DI an objects dependencies are unlikely (probably never) going to be accessed publicly on an instance, they're just used internally to encapsulate some behaviour and respect single responsibility/loose coupling. I prefer a narrow public interface that exposes only what is necessary myself.

As for unit testing, you shouldn't need to mock/stub/monkey patch anything with DI as you can just pass fakes into the constructor.

// EDIT: Also, re the instance variable, I hardly ever access instance variables directly as I prefer to encapsulate access with an accessor method

[nepalez]

@AMHOL Hah! I started asking about a variable, but you've added an //EDIT

Still I wonder why do you prefer private readers over instance variables? I'd understood if you wanted to stub public readers, but there is no need in stubbing private ones. I still find a generated private reader a sort of overkill.

PS. That's interesting, how we mention each other via variables ;)

[AMHOL]

I prefer it because it adds an extra layer of encapsulation, if I want to change the behaviour of the variable at a later stage I can do it in a reader rather than updating all instances where the ivar is referenced, of course you could just handle the case when setting the ivar but it doesn't work as well when the setter is generated by an external library or for more complex cases.

lol yeah funny you said that, I set up a Github account for my GF and recently she's been getting tonnes of emails from people using her Github username as an instance variable name not adding markdown tags around their code in comments

[nepalez]

Well, we could support 4 options here. Namely:

• reader: false
• reader: :public (I'd still prefer it as a default option)
• reader: :private
• reader: :protected

Ultimately, this will touch only load time, so this addition won't affect performance of the instantiation.

@jmgarnier if you want to make a PR (do not miss https://hacktoberfest.digitalocean.com ;) ), you should start here https://github.com/dry-rb/dry-initializer/blob/master/lib/dry/initializer/builder.rb#L78-L83

[jmgarnier]

Thank you @AMHOL variable 😄 for explaining why I would like this feature, I agree with you.
@nepalez I will try to work on it. :public as default makes sense to me.

Hacktoberfest sounds nice 👕 . I guess you need to add the Hacktoberfest label to your project and issue then

[nepalez]

@solnic Could you please add Hacktoberfest label? It's weird I cannot do this on my own ;)
UPD: "Yes, I can!"

[jmgarnier]

Cheers @nepalez 🙇 I guess one more step is needed, label your issues with Hacktoberfest

https://github.com/search?l=&q=state%3Aopen+label%3Ahacktoberfest&ref=advsearch&type=Issues&utf8=%E2%9C%93

[fuadsaud]

Adding 2 cents: one advantage of private methods over @instance_var direct access is that if you make a typo using the former, ruby will blow with NoMethodError, whereas with the latter it'll silently return nil.