0

I am having problems refactoring out some duplicated code from two methods sharing a for loop. The two methods with the duplicated code are gcdOfFiveUpToFive and remainderStepsUpToFive. The two loops share in common setting instance variable @m to 5 and the both use a for x in 1..5 loop and then set @n to x as well as both of them need to call euclidGCD although one calls euclidGCD for its return value and the other to add +=1 to the @count variable. I do want want to return 2 values from one method. I guess I could make a 4th instance variable called @countArray and get an array of the remainder step count. 

require 'minitest/autorun'


class GCDTest < Minitest::Test 
  def test_euclid_gcd
    gcdTestObject=GCD.new(20,5)
    assert gcdTestObject.euclidGcd==5
    assert gcdTestObject.gcdRemainderSteps==1

  end

  def test_euclid_two
    gcdTestObject=GCD.new(13,8)
    assert gcdTestObject.euclidGcd==1 
    assert gcdTestObject.gcdRemainderSteps==5
  end

  def test_euclid_loop
    gcdTestObject=GCD.new(0,0)
    assert gcdTestObject.gcdOfFiveUpToFive==[1,1,1,1,5]
  end 

  def test_count_of_loop
    gcdTestObject=GCD.new(0,0)
    assert gcdTestObject.remainderStepsUpToFive==[1,2,3,2,1]
  end


end



class GCD
  attr_accessor :m,:n
  attr_reader :count 
  def initialize(m,n)
    @m=m
    @n=n
    @count=0
  end

  def euclidGcd
    @count=1
    m=@m 
    n=@n
    r= m % n
    until r==0 
      m=n
      n=r
      r= m % n
      @count+=1
    end
    return n  
  end

  def gcdRemainderSteps
    return @count
  end

  def gcdOfFiveUpToFive
    @m=5
    gcdArrayUpToFive=[]
    for x in 1..5
      @n=x
      gcdArrayUpToFive << euclidGcd
    end
    return gcdArrayUpToFive
  end

  def remainderStepsUpToFive
    @m=5
    gcdStepArrayUpToFive=[]
    for x in 1..5
      @n=x
      euclidGcd
      gcdStepArrayUpToFive << gcdRemainderSteps
    end
    return gcdStepArrayUpToFive
  end

  def fiveLoopExtraction

  end
jack parsons
  • 165
  • 1
  • 13

1 Answers1

0

Code that repeats itself is this:

array=[]
for x in 1..5
  # result = do something with x
  array << result
end
return array

That is exactly what map function does.
What does the "map" method do in Ruby?

Ruby methods names should be snake_case. Lets refactor this to use proper naming convention and map function.

 def gcd_of_five_up_to_five
    @m=5
    (1..5).map do |x|
      @n = x
      # in ruby you don't have to write return
      # value of last expression is returned automatically
      euclid_gcd
    end
  end

  def remainder_steps_up_to_five
    @m=5
    (1..5).map do |x|
      @n = x
      euclid_gcd
      gcd_remainder_steps
    end
  end

I'd call it with params instead of using @m and @n. That would simplify the code. If you change euclid_gcd to this: def euclid_gcd(m:, n:) you'd get this:

  def gcd_of_5_up_to_5
    (1..5).map { |x| euclid_gcd(m: 5, n: x) }
  end

  def remainder_steps_up_to_5
    (1..5).map do |x| 
      euclid_gcd(m: 5, n: x)
      gcd_remainder_steps
    end
  end

Seems like this needs little or no further refactoring.

Community
  • 1
  • 1
Marko Avlijaš
  • 1,579
  • 13
  • 27
  • [link](https://github.com/jackparsons93/gcdAverage/blob/master/euclidGcdAverage.rb) This is a github link to my new code with ruby style guide and your help, all 6 unit test are green. Also do you think I need to refactor euclid_gcd it is 11 lines or 9 if you don't count def and end. Also I could not find on ruby style guide [link] (https://github.com/bbatsov/ruby-style-guide#classes--modules) If it was okay to use all caps as an abbreviation my class name is GCD and it is all caps but it is abbreviated. I wonder if all caps is okay for eloquent ruby code. I also omitted all the returns. – jack parsons May 04 '17 at 02:14
  • You can google ruby style guide - variables are also snake_case. I think both GCD and Gcd is fine, not sure about that. You do not need lines 44 and 46, try what I wrote, just one line I have for def gcd_of_5_up_to_5 .... end is enough. I suggest you go through a few tutorials/examples that explain `map`, it's important and often used function, also found in javascript & other languages. – Marko Avlijaš May 04 '17 at 06:05