5
\$\begingroup\$

I am learning Ruby programming from "Learn Ruby the Hard way" and I am doing the "Ruby koans".

I have heard a little bit about "Idiomatic" Ruby but I don't know much about it. How can I make it more idiomatic?

# Program to solve polynomials
def solve(coeff)
 coefficients = []
 coeff.each{ |a|
 coefficients.push a.to_f
 }
 diffrentiate = []
 n = 0
 coefficients.reverse_each { |a| 
 diffrentiate.unshift n*a
 n += 1
 }
 diffrentiate.pop
 return newton(coefficients,diffrentiate,-1)
end
def newton(coefficients,diffrentiate,initialvalue)
 x = initialvalue
 10.times { |n| 
 x -= evaluate_polynomial(coefficients,x)/evaluate_polynomial(diffrentiate,x)
 }
 return x
end
def evaluate_polynomial(coefficients,x)
 n = 0
 value = 0
 coefficients.reverse_each { |a| 
 value += a*x**n
 n+=1
 }
 return value
end
puts solve ([1,0,0,0,-1,1])
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 28, 2015 at 11:07
\$\endgroup\$
1
  • \$\begingroup\$ There's nothing wrong with your code, an if you're just learning then just make it run and ask for tips on how to make it better. Idiomatic programming can be tricky and chances are with your coding up there, you'll probably get about twelve different answers that are the same written differently, for an example see this: stackoverflow.com/questions/1364113/… Ruby is all about being easy to read and write. Just have fun with it, that's what counts and never let anybody tell you that you're wrong. \$\endgroup\$ Commented Nov 28, 2015 at 13:59

1 Answer 1

5
\$\begingroup\$
  1. Use do...end instead of {...} for multi-line blocks.

  2. return is implicit at the end of a method.

  3. differentiate is a verb. derivatives is the plural noun you're looking for when naming the array.

  4. Use Ruby's array methods (map, reduce, and friends) to your advantage.

  5. Don't modify external variables from inside a block if you can avoid it. I.e. don't do things like n += 1, or x -= .... A combination of with_index and reduce should do the same trick more neatly.

  6. There's some repetition in your handling of the order of coefficients (i.e. the n += 1 in the blocks). It'd be nice to get rid of that.

  7. It'd be nice to specify the number of iterations the approximation should use.

Here's a pretty direct translation of your solution:

# Add a method to Array which zips the array with the index
# of each element in reverse order
class Array
 def zip_reverse_index
 zip((0...count).to_a.reverse)
 end
end
# Use a *-splat to make the method variadic
def solve(*coefficients)
 coefficients.map!(&:to_f)
 derivatives = coefficients.zip_reverse_index.map { |n, i| n * i }
 newton(coefficients, derivatives[0...-1])
end
# Added some (optional) arguments
def newton(coefficients, derivatives, iterations = 10, initial = -1)
 iterations.times.reduce(initial) do |x, i|
 x -= evaluate_polynomial(coefficients, x) / evaluate_polynomial(derivatives, x)
 end
end
# A simpler implementation using reduce
def evaluate_polynomial(coefficients, x)
 coefficients.zip_reverse_index.reduce(0) do |sum, (c, n)|
 sum += c * x ** n
 end
end

Here's a different version that keeps everything in 1 method:

def newton_raphson(coefficients, intial = -1, iterations = 10)
 evaluate = -> (array, x) do
 array.reduce(0) { |sum, (c, ord)| sum += c * x ** ord }
 end
 coefficients = coefficients
 .map(&:to_f) 
 .zip((0...coefficients.count).to_a.reverse)
 derivatives = coefficients.map { |c, ord| [c * ord, ord - 1] }[0...-1]
 iterations.times.reduce(intial) do |approx, _|
 approx -= evaluate[coefficients, approx] / evaluate[derivatives, approx]
 end
end

It's pretty long for a Ruby method, but some of the methods in the other implementation seemed too specific to keep around in the global scope (and the Array mixin also seemed a little heavy-handed).


Edit: as @jQweirdy mentioned in the comments, this could also be solved with a class, but I was hesitant to add a class just for one method. But there are a few things here, that might be generically useful for Polynomial class. So here's an example:

class Polynomial
 attr_reader :coefficients
 def initialize(*coefficients)
 @coefficients = coefficients
 end
 def evaluate(x)
 coefficients.zip(ordinals).reduce(0) do |sum, (n, i)|
 sum += n * x ** i
 end
 end
 def derivative
 @derivative ||= begin
 derivatives = coefficients.zip(ordinals)[0...-1].map { |n, i| n * i }
 self.class.new(*derivatives)
 end
 end
 def order
 coefficients.count - 1
 end
 def ordinals
 (0..order).to_a.reverse
 end
 def newton_raphson_approximation(iterations = 1, initial = -1)
 iterations.times.reduce(initial) do |x, _|
 x -= evaluate(x).fdiv(derivative.evaluate(x))
 end
 end
end
answered Nov 28, 2015 at 15:50
\$\endgroup\$
6
  • 1
    \$\begingroup\$ isn't ruby 'idiomatically' object oriented? Couldn't you change your code to be more OO (like the first example) without the 'public methods floating about? Maybe a Polynomial class? \$\endgroup\$ Commented Dec 8, 2015 at 16:53
  • \$\begingroup\$ @jQwierdy I saw your answer before it was removed. And yes, it could be put into a class. However, here the goal is only to implement the Newton-Raphson approximation, so you're looking at a function rather than a class with methods. It's pure input → output with no state being kept. But yes, it could be a class. I'd just like to add few more polynomial-manipulation methods to "justify" the class. In this case, derivatives could be such a method - the usefulness of something like that is not exclusive to the Newton approximation. \$\endgroup\$ Commented Dec 8, 2015 at 18:44
  • \$\begingroup\$ I get what you're saying, and I love functional programming but you're going against the "send messages" OO part of ruby - aren't you? Even stateless classes, or a non-instatiatable class would be better than floating methods - wouldn't it? \$\endgroup\$ Commented Dec 8, 2015 at 18:50
  • 1
    \$\begingroup\$ @jQwierdy Sure, certainly better than methods floating around in global scope. But that's also why I added the alternative that just keeps everything in 1 method, just to contain it. There are many ways to structure all of this. This was a learning project according to OP, not something part of a larger code base, so even keeping things in the global scope would be ok for now. I just looked at micro level (how to produce the desired result with different code) and less at the macro level (how to structure the thing). So you're right - it just seemed out-of-scope for this review. \$\endgroup\$ Commented Dec 8, 2015 at 18:58
  • \$\begingroup\$ Fair enough! Thanks for the detailed response. :-) \$\endgroup\$ Commented Dec 8, 2015 at 21:30

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.