HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Curious fractions in functional Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
combinatoricscodereviewstackoverflowfunctional-programmingprogramming-challengerubymathematics
functionalcuriousfractionsruby

Problem

Problem 33 in Project Euler asks:


The fraction 49/98 is a curious fraction, as an inexperienced
mathematician in attempting to simplify it may incorrectly believe
that 49/98 = 4/8, which is correct, is obtained by canceling the 9s.


We shall consider fractions like, 30/50 = 3/5, to be trivial examples.


There are exactly four non-trivial examples of this type of fraction,
less than one in value, and containing two digits in the numerator and
denominator.


If the product of these four fractions is given in its lowest common
terms, find the value of the denominator.

This is the code I came up with, which solves the problem pretty handily. But, I'm concerned that this is messy and unreadable, even with the comments - and I simply hate using magic numbers/indices littered throughout the code. Also, I do not want a procedural ("C" style) solution - I'm learning how to write idiomatic (read "functional") programming with Ruby, and I'd like to learn how to write cleaner functional code.

```
a = 10.upto(99).to_a.map(&:to_s) # Create array of integers 10 - 99
puts a.product(a) # Form cartesion product [a,b] for all 2 digit numbers a,b
.reject{|x| x.any? {|y| y.chars.include? '0'}} # Numbers inlcuding '0' are the trivial matches
.reject{|x| x.first == x.last || # Reject pairs where a = b
x.first.to_f/x.last.to_f > 1 || # Reject fractions greater than 1
(x.first.chars & x.last.chars).size != 1} # Reject pairs in which no digits are common
.map{|x| [x.first.to_f / x.last.to_f, x.first.chars, x.last.chars, x.first.chars & x.last.chars]} # Ex: [0.5, ["4", "9"], ["9", "8"], ["9"]]
.map{|x| [x.first, x[1] - x.last, x[2] - x.last, x[1], x[2]]} # Ex: [0.5, ["4"], ["8"], ["4", "9"], ["9", "8"]]
.select{|x| x.length == 5 && x.first == x[1].join.to_f/x[2].join.to_f} # Select only the tuples which match the condition
.map{|x| Rational(x[

Solution

Array multiple assignment

You pass arrays around, and you find yourself calling for x[1] and x.last - this is very unreadable, and brittle.

Ruby allows you to implicitely assign elements of the array to parameters in your block according to its location, so you could write the same "maps" like this:

.map do|numerator, denominator| 
     [numerator.to_f / denominator.to_f, numerator.chars, denominator.chars, 
      numerator.chars & denominator.chars] 
  end.map do |result, numerator_digits, denominator_digits, mutual_digit| 
    [result, numerator_digits - mutual_digit, denominator_digits - mutual_digit, 
     numerator_digits, denominator_digits]  
  end.select do|result, new_numerator_digits, new_denominator_digits| 
    result == new_numerator_digits.join.to_f/new_denominator_digits.join.to_f 
  end.map do |_, _, _, numerator_digits, denominator_digits| 
    Rational(numerator_digits.join.to_i, denominator_digits.join.to_i) 
  end.reduce(:*)


Note that unneeded array elements at the end of the array are truncated, so you don't have to write them. For the unneeded elements at the beginning of the array I've used the _ idiom which tells the method that I expect these parameters, but I won't need them.

If it deserves a comment - it deserves a method

Instead of commenting your code, consider refactoring it into methods. Each method will do one thing, and, given the proper name, will be descriptive when used, and easily read in itself:

def trivial?(pair)
  pair.any? {|y| y.chars.include? '0'}
end

def identical?(numerator, denominator)
  numerator == denominator
end

def larger_than_one?(numerator, denominator)
  numerator.to_i > numerator.to_i
end

def has_common_digits?(numerator, denominator)
  (numerator.chars & numerator.chars).size > 0
end

a = 10.upto(99).to_a.map(&:to_s)  
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .reject(&method(:identical?)) 
      .reject(&method(:larger_than_one?)) 
      .select(&method(:has_common_digits?))


Make your steps logical

The last few steps you take look like they are held out with gum, and that you made some acrobatics to stay one-line and "idiomatic". It is better to make your methods do something you can explain, than simply adding and removing temp-variables:

def reduced_fraction(numerator, denominator)
  mutual_digit = numerator.chars & denominator.chars
  (numerator.chars - mutual_digit).join.to_f / (denominator.chars - mutual_digit).join.to_f
end

a = 10.upto(99).to_a.map(&:to_s)
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .reject(&method(:identical?)) 
      .reject(&method(:larger_than_one?)) 
      .select(&method(:has_common_digits?))
      .select do |numerator, denominator| 
        numerator.to_f / denominator.to_f == reduced_fraction(numerator, denominator)
      end.map { |fraction| Rational(*fraction) }
      .reduce(:*)


Update

Yes, it seems that using &method(:something) in map and multiple assignment are mutually exclusive... :(

I can suggest a few strategies where you don't have to fall back to using pair.first, pair.last:

-
Use multiple assignment in the first line of your method:

def identical?(pair)
  numerator, denominator = pair
  ...
end


-
Use a hash to hold your values, and use keyword arguments in your method

def identical?(numerator: nil, denominator: nil)
  numerator == denominator
end

a = 10.upto(99).to_a.map(&:to_s)
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .map { |numerator, denominator| {numerator: numerator, denominator: denominator } }
      .reject(&method(:identical?))

Code Snippets

.map do|numerator, denominator| 
     [numerator.to_f / denominator.to_f, numerator.chars, denominator.chars, 
      numerator.chars & denominator.chars] 
  end.map do |result, numerator_digits, denominator_digits, mutual_digit| 
    [result, numerator_digits - mutual_digit, denominator_digits - mutual_digit, 
     numerator_digits, denominator_digits]  
  end.select do|result, new_numerator_digits, new_denominator_digits| 
    result == new_numerator_digits.join.to_f/new_denominator_digits.join.to_f 
  end.map do |_, _, _, numerator_digits, denominator_digits| 
    Rational(numerator_digits.join.to_i, denominator_digits.join.to_i) 
  end.reduce(:*)
def trivial?(pair)
  pair.any? {|y| y.chars.include? '0'}
end

def identical?(numerator, denominator)
  numerator == denominator
end

def larger_than_one?(numerator, denominator)
  numerator.to_i > numerator.to_i
end

def has_common_digits?(numerator, denominator)
  (numerator.chars & numerator.chars).size > 0
end

a = 10.upto(99).to_a.map(&:to_s)  
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .reject(&method(:identical?)) 
      .reject(&method(:larger_than_one?)) 
      .select(&method(:has_common_digits?))
def reduced_fraction(numerator, denominator)
  mutual_digit = numerator.chars & denominator.chars
  (numerator.chars - mutual_digit).join.to_f / (denominator.chars - mutual_digit).join.to_f
end


a = 10.upto(99).to_a.map(&:to_s)
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .reject(&method(:identical?)) 
      .reject(&method(:larger_than_one?)) 
      .select(&method(:has_common_digits?))
      .select do |numerator, denominator| 
        numerator.to_f / denominator.to_f == reduced_fraction(numerator, denominator)
      end.map { |fraction| Rational(*fraction) }
      .reduce(:*)
def identical?(pair)
  numerator, denominator = pair
  ...
end
def identical?(numerator: nil, denominator: nil)
  numerator == denominator
end

a = 10.upto(99).to_a.map(&:to_s)
puts a.product(a) 
      .reject(&method(:trivial?)) 
      .map { |numerator, denominator| {numerator: numerator, denominator: denominator } }
      .reject(&method(:identical?))

Context

StackExchange Code Review Q#48256, answer score: 3

Revisions (0)

No revisions yet.