barsoom / attr_extras

Takes some boilerplate out of Ruby with methods like attr_initialize.
MIT License
560 stars 31 forks source link

attr_initialize + attr_reader #9

Closed monkbroc closed 9 years ago

monkbroc commented 9 years ago

I often want to provide public attr_reader without implying value object semantics.

I have to use attr_initialize followed by attr_reader with the same list of attributes.

Would you be open to another shortcut like rattr_initialize for public reader initialize?

henrik commented 9 years ago

Sounds reasonable to me.

What are some examples of what you use this for, out of curiosity? I haven't felt the need myself, or not noticed if I have.

monkbroc commented 9 years ago

The main place where I want to use this are in service objects. They exist mostly for side-effects, but it's good to be able to get to their attributes.

It doesn't make sense to compare two service objects with each other, so using vattr_initialize would a little misleading.

Here's an example.

  class SendMonthlyDigest
    attr_initialize :organization, [:digest_date] do
      @digest_date ||= Date.today
    end

    attr_reader :organization, :digest_date

    def call
      generate_monthly_report
      send_email_to_organization
    end

    private

    def generate_monthly_report
    end

    def send_email_to_organization
    end
  end

I'll implement the code and tests then send a PR.

soma commented 9 years ago

I like the idea, it seems reasonable. I have a little issue with the name you suggested though (which is kind of a problem with more parts of attr_extras), the "p" in pattr_initialize stands for "private", and you suggest rattr_initialize where the r would mean reader, right? I agree vattr_initialize also has this problem. It is confusing in any case!

monkbroc commented 9 years ago

Yes, I agree. pattr_initialize for "public" reader wouldn't work for obvious reasons :smile:

What about attr_reader_initialize? It's a little longer, but it conveys the meaning well.

Maybe adding an alias from pattr_initialize to attr_private_initialize and vattr_initialize to attr_value_initialize would fix the confusion you are feeling.

soma commented 9 years ago

Yes, :+1: on more explicit naming!

monkbroc commented 9 years ago

I'd also suggest reordering the readme because the combination methods like pattr_initialize are shown before the simple cases. That was confusing for a first read.

henrik commented 9 years ago

I'd also suggest reordering the readme because the combination methods like pattr_initialize are shown before the simple cases. That was confusing for a first read.

Ah, that was intentional because we almost always use the higher-level ones in practise. But I can see how it doesn't make for a good introduction. Will fix the order right now.

monkbroc commented 9 years ago

OK. I think the summary paragraph shows clearly the usefulness of the higher-level methods.

About the comment :+1: on more explicit naming, do you want to keep the main name of methods short like pattr_initialize and vattr_initialize and have longer aliases, or would you be open to making the longer names the ones shown in the readme and have aliases for the short ones?

This is what I mean is:

attr_value_initialize :foo, :bar defines initializer, public readers and value object identity: shortcut for

attr_initialize :foo, :bar
attr_value :foo, :bar

The short name vattr_initialize is an alias for attr_value_initialize.

henrik commented 9 years ago

Aliases aren't a bad idea – I doubt we'd actually use them ourselves, but just having the long forms around would make things a bit more self-documenting.

Maybe the docs could have both in the header/ToC link: vattr_initialize / attr_value_initialize and so on.

Maybe it should be vattr_initialize / value_attr_initialize, pattr_initialize / private_attr_initialize, rattr_initialize / reader_attr_initialize so the short form maps more closely to the long form?

monkbroc commented 9 years ago

Having both in the header/ToC sounds like a good way to make it more self-documenting.

All other methods start with attr, so I would find it more natural to write attr_private_initialize.

henrik commented 9 years ago

You're right about the naming. Merged!

monkbroc commented 9 years ago

Thanks. Great doing software with you guys!