Store a collection of Ruby objects

Question

I am trying to store a collection of objects within another object. I completed a coding challenge that looks like the following.

First the singular object:

class Account
  attr_reader :user_name, :credit, :debit

  def initialize(user_name)
    @user_name = user_name
    @credit = 0
    @debit = 0
  end
end

Next the collection:

class AccountsCollection
  attr_reader :accounts

  def initialize
    @accounts = []
  end

  def add_new_account(user)
    accounts << Account.new(user)
  end

  ...
end

This is how I use it:

accounts = AccountsCollection.new
# => #<AccountsCollection:0x00007fc76ba70b18 @accounts=[]>
accounts.add_new_account('A')
accounts.add_new_account('B')
accounts.add_new_account('C')
accounts.accounts
# =>[
#     #<Account:0x00007fc76b933890 @user_name="A">,
#     #<Account:0x00007fc76bc76d68 @user_name="B">,
#     #<Account:0x00007fc76c88c2d8 @user_name="C">
#   ]

I wanted to use it like this:

class Display
  attr_reader :accounts

  def initialize(accounts)
    @accounts = accounts
  end

  def display_inline
    accounts.each do |account|
      #do something
  end
  ...
end

Display.new(accounts.accounts).display_inline

But I have to call accounts.accounts to obtain the list of account objects. Is this weird? Can anyone show me how I can do this better?

"had to call accounts.accounts to obtain the list" – well, the list should be encapsulated within AccountsCollection. Why do you need / want to have direct access to it? — Stefan
– Stefan, Commented Jun 6, 2018 at 15:07
The question here is why you needed a new class in the first place? With the current implementation I would just use a plain array. If there is some more significant functionality there, it will guide you to a better name. Anyway, if for whatever reason you stick with AccountsCollection, just rename the accounts method to to_a. — ndnenkov
– ndnenkov, Commented Jun 6, 2018 at 15:08
@ndn owh that's because i left out a few attributes.there are other attrs such as credit, debit and balance. Sorry I should have included it in there. I left out another method for accountscollection, which is create or initialize get_user(name) || add_new_account(name) — Keith Chong
– Keith Chong, Commented Jun 6, 2018 at 15:23

iCodeSometime · Accepted Answer · 2018-06-06 15:11:03Z

2

Really it looks good to me other than the naming looking awkward. If it was me, I'd have the names like this so it looks good using it.

class AccountsCollection
  def initialize
    @accounts = []
  end

  def add(user)
    accounts << Account.new(user)
  end

  def to_a
    @accounts
  end
end

Then your code would look like

accounts = AccountsCollection.new
 => #<AccountsCollection:0x00007fc76ba70b18 @accounts=[]>

accounts.add('A')
accounts.add('B')
accounts.add('C')

accounts.to_a
 =>[
   #<Account:0x00007fc76b933890 @user_name="A">,
   #<Account:0x00007fc76bc76d68 @user_name="B">,
   #<Account:0x00007fc76c88c2d8 @user_name="C">
 ]

edited Jun 6, 2018 at 15:11

answered Jun 6, 2018 at 15:05

iCodeSometime

1,6791 gold badge18 silver badges35 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

SirDarius Over a year ago

It can be argued that show might not be the best name here. Assuming that you want to extract an array from this collection, the idiomatic ruby way would be to use the to_a method.

iCodeSometime Over a year ago

Right, I agree to_a would be better. Updated, thanks

Stefan Over a year ago

You might want to dup the array to prevent direct modification.

Simple Lime · Accepted Answer · 2018-06-06 16:40:35Z

The code smell, IMO, with your code is simply that it's a collection, but I can't treat it like a collection. For instance, if I wanted all the accounts that had more than 10 credits, I would have to pull the array of accounts out of the collection to do it:

accounts.accounts.select { |account| account.credit > 10 }

whereas what you'd really want is to be able to just query the collection for it:

accounts.select { |account| account.credit > 10 }

To make your collection feel more like a collection, you need to include Enumerable which requires that you implement the each method:

class AccountsCollection
  include Enumerable

  def initialize
    @accounts = []
  end

  def add(user_name)
    @accounts << Account.new(user_name)

    self # return self because that's more in-line with other collections
  end

  def each
    return to_enum(__method__) { @accounts.size } unless block_given?

    @accounts.each { |account| yield account }
  end
end

and with just that you can now treat your collection like the other collections in ruby:

accounts = AccountsCollection.new
accounts.add('A').add('B').add('C')

accounts.each.with_index do |account, index|
  # I changed the attr_reader in Account to attr_accessor just to illustrate
  account.credit = index * 10
end

accounts.select { |account| account.credit > 10 }
# => [#<Account:0x00007fe50d894208 @user_name="C", @credit=20, @debit=0>]

and if you want it as an array, you automatically got a to_a method as well:

accounts.to_a
# => [#<Account:0x00007f879a094538 @user_name="A", @credit=0, @debit=0>,
#     #<Account:0x00007f879a094308 @user_name="B", @credit=10, @debit=0>,
#     #<Account:0x00007f879a0968b0 @user_name="C", @credit=20, @debit=0>]

this (by default) will create a new array by going through the each method so the array won't be directly modified (though the contents still can be)

accounts.to_a[1].debit = 15
accounts.to_a << 'not an account'

accounts.to_a
# => [#<Account:0x00007fb51c08a9c0 @user_name="A", @credit=0, @debit=0>,
#     #<Account:0x00007fb51c08a6a0 @user_name="B", @credit=10, @debit=15>,
#     #<Account:0x00007fb51c08a588 @user_name="C", @credit=20, @debit=0>]

You can always replace any method of Enumerable with a better, custom implementation if you want:

def to_a
  @accounts.dup
end

Now things are starting to feel and smell a whole lot better, with minimal work, and others trying to use your AccountsCollection will be thoroughly pleased that they have access to all the search and traversal methods they're used to.

Collectives™ on Stack Overflow

Store a collection of Ruby objects

2 Answers 2

3 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related