Ruby class, arguments being passed around methods, instance variable?

Question

I'm having this problem which I think it might be a code smell, I have a class that receives an argument in its initialiser and contains one public and several private methods - everything normal. Example:

class Foo
  def initialize(a)
    @a = a
  end

  def apply?(examples)
    foo_1(examples)
  end

  private

  def foo_1(examples)
    foo_2(examples) ? 1 : 2
  end

  def foo_2(examples)
    examples.size > @a
  end
end

My problem here, is 'examples' that is received by the public method being carried around over and over the private methods, it doesn't look pretty and it seems like a code smell, what's the best approach here? Make it an instance variable inside the public method?

Thanks

Query objects violate “Tell Don't Ask” and the return type indicates you're not using “Duck Typing”. I suggest rethinking the implementation in terms of these for pure OO to help stay out of trouble in the future. — 6ft Dan
– 6ft Dan, Commented Dec 4, 2017 at 0:20
There's way too little information about the actual requirements and no real-world example use case for this. Normally lack of thereof might be fine for a constructive discussion, but since this is all about good design patterns more information is really required. Otherwise adding examples to initializers and instantiating the class for every query might be a reasonable solution. — mdrozdziel
– mdrozdziel, Commented Dec 4, 2017 at 8:03

Andrzej Krzywda · Accepted Answer · 2017-12-03 22:29:18Z

2

Yes, this may be considered a code smell if the number of private methods accepting examples is bigger than 1-2.

One thing to consider would be to extract a class to represent the rule here.

For example:

class Foo
  def initialize(a)
    @a = a
  end

  def apply?(examples)
    size_rule_applies?(examples) ? 1 : 2
  end

  private

  def size_rule_applies?(examples)
    SizeRule.new(@a, examples).apply?
  end

  class SizeRule
    def initialize(a, examples)
      @a        = a
      @examples = examples
    end

    def apply?
      @examples.size > @a
    end
  end
end

I wouldn't make the examples an instance variable of the Foo class as there's a risk it would persist in memory between the calls to that object. I've seen bugs like that.

answered Dec 3, 2017 at 22:29

Andrzej Krzywda

1818 bronze badges

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

2 Comments

mdrozdziel Over a year ago

Since one is creating a new instance of a SizeRule class for every query anyway, why not add examples to Foo's initializer?

Andrzej Krzywda Over a year ago

You can have one instance of Foo with the a always set, but then use this object several times to ask for apply? with different examples every time. At least, that's how I understand this setup here.

Roman Pushkin · Accepted Answer · 2017-12-04 03:44:33Z

0

If examples changes dynamically then making it instance variable is not an option. You'll either need to instantiate Foo for every examples or you'll end up having mutable Foo where examples itself is changing, which is also not good.

Your code looks fine. The only thing that concerns me is that one method depends on another. It's usually not a big deal, but this would look better to me:

  def apply?(examples)
    foo_1(examples) && foo_2(examples)
  end

answered Dec 4, 2017 at 3:44

Roman Pushkin

6,2015 gold badges44 silver badges62 bronze badges

Comments

Yanhao · Accepted Answer · 2017-12-04 06:36:10Z

0

Another option is to use block:

class Foo
  def initialize(a)
    @a = a
  end

  def apply?(examples)
    foo_1 { examples }
  end

  private

  def foo_1
    v = foo_2 { yield.size }
    v ? 1 : 2
  end

  def foo_2
    yield > @a
  end
end

answered Dec 4, 2017 at 6:36

Yanhao

5,3021 gold badge25 silver badges15 bronze badges

Comments

Olalekan Sogunle · Accepted Answer · 2018-02-10 16:53:23Z

0

Since the class Foo does nothing else presently than just evaluate .apply?(examples) I will advice examples be added to the initializer and made an instance variable. This is simpler, more efficient and more obvious.

class Foo
  def initialize(a, examples)
   @a = a
   @examples = examples
  end

  def apply?
    foo_1
  end

  private

  def foo_1
    foo_2 ? 1 : 2
  end

  def foo_2
    @examples.size > @a
  end
end

answered Feb 10, 2018 at 16:53

Olalekan Sogunle

2,3671 gold badge24 silver badges27 bronze badges

Collectives™ on Stack Overflow

Ruby class, arguments being passed around methods, instance variable?

4 Answers 4

2 Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related