Possible help with code refactoring

Sandy Metz talks about GORUCO's SOLID OOPS concepts that the presence of blocks if..else

in Ruby can be seen as a deviation from the Open-Close principle. What are all the methods you can use to avoid non-urgent conditions if..else

? I tried the following code:

class Fun
   def park(s=String.new)
      puts s
   end
   def park(i=Fixnum.new)
      i=i+2
   end
end

      

and found out that function overloading in Ruby doesn't work. What are the other methods by which the code can be executed to obey the OCP?

I could just go:

class Fun
  def park(i)
      i=i+2 if i.class==1.class 
      puts i if i.class=="asd".class
  end
end

      

but it violates the OCP.

+1


source to share


5 answers


You can do something like this:

class Parent
  attr_reader :s

  def initialize(s='')
    @s = s
  end

  def park
    puts s
  end
end

class Child1 < Parent
  attr_reader :x

  def initialize(s, x)
    super(s)
    @x = x
  end

  def park
    puts x 
  end
end

class Child2 < Parent
  attr_reader :y

  def initialize(s, y)
    super(s)
    @y = y
  end

  def park
    puts y
  end
end


objects = [
  Parent.new('hello'),
  Child1.new('goodbye', 1),
  Child2.new('adios', 2),
]

objects.each do |obj|
  obj.park
end

--output:--
hello
1
2

      

Or maybe I missed one of your twists:



class Parent
  attr_reader :x

  def initialize(s='')
    @x = s
  end

  def park
    puts x
  end
end

class Child1 < Parent
  def initialize(x)
    super
  end

  def park
    x + 2 
  end
end

class Child2 < Parent
  def initialize(x)
    super
  end

  def park
    x * 2
  end
end


objects = [
  Parent.new('hello'),
  Child1.new(2),
  Child2.new(100),
]

results = objects.map do |obj|
  obj.park
end

p results

--output:--
hello
[nil, 4, 200]

      

And another example using blocks that are similar to anonymous functions. You can pass the desired behavior to park () as a function:

class Function
  attr_reader :block

  def initialize(&park)
    @block = park 
  end

  def park
    raise "Not implemented"
  end
end


class StringFunction < Function
  def initialize(&park)
    super
  end

  def park
    block.call
  end
end

class AdditionFunction < Function
  def initialize(&park)
    super
  end

  def park
    block.call 1
  end
end

class DogFunction < Function
  class Dog
    def bark
      puts 'woof, woof'
    end
  end

  def initialize(&park)
    super
  end

  def park
    block.call Dog.new
  end
end


objects = [
  StringFunction.new {puts 'hello'},
  AdditionFunction.new {|i| i+2},
  DogFunction.new {|dog| dog.bark},
]

results = objects.map do |obj|
  obj.park
end

p results

--output:--
hello
woof, woof
[nil, 3, nil]

      

+1


source


For your current example and wanting to avoid type detection, I would use Ruby's function to reopen classes to add the functionality needed for Integer

and String

:

class Integer
  def park
    puts self + 2
  end
end

class String
  def park
    puts self
  end
end

      

This will work more cleanly if you change your own classes. But it might not be in line with your conceptual model (it depends on what it represents Fun

and why it might accept these two different classes in the same method).

Equivalent, but keeping your class Fun

can be:

class Fun
  def park_fixnum i
    puts i + 2
  end

  def park_string s
    puts s
  end

  def park param
    send("park_#{param.class.to_s.downcase}", param)
  end
end

      



As an opinion, I'm not sure if you will get much Ruby this way. The principles you learn may be good (I don't know), but forcing them against the grain of the language can create less readable code, whether or not it is well-intentioned.

So, what I would probably do in practice is:

class Fun
  def park param
    case param
    when Integer
      puts param + 2
    when String
      puts param
    end
  end
end

      

This doesn't fit your guidelines, but is idiomatic Ruby and is a little easier to read and maintain than a block if

(where the conditions can be much more complex, so it takes a lot more for a human to understand).

+1


source


You could just create handled classes for Fun like this

class Fun
   def park(obj)
    @parker ||= Object.const_get("#{obj.class}Park").new(obj)
    @parker.park 
    rescue NameError => e
        raise ArgumentError, "expected String or Fixnum but recieved #{obj.class.name}"
   end
end

class Park
    def initialize(p)
        @park = p
    end
    def park
        @park
    end
end

class FixnumPark < Park
    def park
        @park += 2
    end
end

class StringPark < Park
end

      

Then things like this will work

f = Fun.new
f.park("string")
#=> "string"
f.instance_variable_get("@parker")
#=> #<StringPark:0x1e04b48 @park="string">
f = Fun.new
f.park(2)
#=> 4
f.instance_variable_get("@parker")
#=> #<FixnumPark:0x1e04b48 @park=4>
f.park(22)
#=> 6 because the instance is already loaded and 4 + 2 = 6
Fun.new.park(12.3)
#=> ArgumentError: expected String or Fixnum but received Float

      

+1


source


Have a look at is_a? method

def park(i)
  i.is_a?(Fixnum) ? (i + 2) : i
end

      

But it's even better not to check the type, but use duck typing:

def park(i)
  i.respond_to?(:+) ? (i + 2) : i
end

      

UPD: after reading comments. Yes, both examples above do not solve the OCP problem. This is how I would do it:

class Fun
  # The method doesn't know how to pluck data. But it knows a guy
  # who knows the trick
  def pluck(i)
    return __pluck_string__(i) if i.is_a? String
    __pluck_fixnum__(i) if i.is_a? Fixnum
  end

  private

  # Every method is responsible for plucking data in some special way
  # Only one cause of possible changes for each of them

  def __pluck_string__(i)
    puts i
  end

  def __pluck_fixnum__(i)
    i + 2
  end
end

      

0


source


I understand or equal the operation in ruby, but can you explain what you did with:

Object.const_get("#{obj.class}Park").new(obj)

      

In ruby, what begins with a capital letter is a constant. Here's a simpler example of how it works const_get()

:

class Dog
  def bark
    puts 'woof'
  end
end

dog_class = Object.const_get("Dog")
dog_class.new.bark

--output:--
woof

      

Of course, you can also pass arguments dog_class.new

:

class Dog
  attr_reader :name

  def initialize(name)
    @name = name
  end

  def bark
    puts "#{name} says woof!"
  end
end

dog_class = Object.const_get("Dog")
dog_class.new('Ralph').bark

--output:--
Ralph says woof!

      

And the next line is just a variation above:

Object.const_get("#{obj.class}Park").new(obj)

      

If obj = 'hello'

, the first part:

Object.const_get("#{obj.class}Park")

      

equivalent to:

Object.const_get("#{String}Park")

      

And when an object of class String is interpolated to a string, it is simply converted to "String", giving you:

Object.const_get("StringPark")

      

And this line extracts the StringPark class, giving you:

Object.const_get("StringPark")
            |
            V
      StringPark

      

Then adding the second part of the original line gives you:

      StringPark.new(obj)

      

And since obj = 'hello'

, which is equivalent:

      StringPark.new('hello')

      

Capice?

-1


source







All Articles