2
\$\begingroup\$

I am taking a class on ruby and our assignment is to take user input and print out a polynomial. Wondering if I can get any feedback to how to improve my code.

class PolynomialElements
  attr_accessor :element, :size

  def printElement
    if size.to_i == 0
      num = "#{@element}"
    elsif size.to_i == 1
      if element.to_i.abs == 1
        num = "x"
      else
        num = "#{@element}x"
      end
    else
      if element.to_i.abs > 1
        num = "#{@element}x^#{@size}"
      elsif element.to_i.abs == 1
        num = "x^#{@size}"
      end
    end
  end
end


askAgain = true
polyArray = Array.new

while askAgain
  puts "How many numbers do you want to enter? "
  numString = gets
  num = numString.to_i
  while num > 0
    puts "Enter a value for the Polynomial "
    value = gets
    polyArray.push(value.chomp)
    num -= 1
  end
  outputArray = Array.new
  sizeOfArray = polyArray.length
  polyArray.each do |x|
    var = PolynomialElements.new
    sizeOfArray -= 1
    var.size = sizeOfArray
    if x.to_i != 0
      var.element = x
      if var.element.to_i > 0
        outputArray.push("+")
        outputArray.push(var.printElement)
      elsif var.element.to_i < 0
        outputArray.push(var.printElement)
      end
    end
  end

  if outputArray[0] == "+"
    outputArray.delete_at(0);
  end

  outputArray.each do |x|
    print x
  end
  puts

  puts "Enter y to enter new number or anything else to quit"
  cont = gets
  if cont.chomp != "y"
    askAgain = false
  else
    polyArray.clear
  end
end
\$\endgroup\$
2
\$\begingroup\$

Ruby agreement is to use snake_case to name methods and variables, and not javaCase. While you could say it's a matter of taste, it's so common in Ruby world you would do better to just stick with it. Ruby programmers also tend to push for initializing arrays like this:

polyArray = []

Your PolynomialElements class is not how you use OOP. It has accessor, but what you pass to them aren't parts of its state - those are arguments for printElement. More proper abstraction would be to make it hold an array of coeficients, and iterate over it to form a string.

class Polynomial
  attr_accessors :coeficients # I think this is a proper name for elements (?)

  def initialize coeficients_array = [0] # default value, just in case
    coeficients = coeficients_array
  end

  def to_s
    coefficients.each_with_object("").with_index do |(coef, str), idx|
      # ...
    end
  end
end

Than, you could use it like:

polynomial = Polynomial.new poly_array
puts polynomial.to_s

Notice that you don't need to operate on array of characters - Ruby string allows you to work in the same way, just use append instead of push, and then you just print/puts it instead of printing each character separately.


Generally speaking, you barely ever need to keep track of index of iteration manually with Ruby. This:

num = numString.to_i
while num > 0
  puts "Enter a value for the Polynomial "
  value = gets
  polyArray.push(value.chomp)
  num -= 1
end

could be much better written as

num_string.to_i.times do
  # ...
end

Ruby way to do (potentially) infinite loop would be:

loop do
  polyArray = []
  # ...
  cont = gets
  break if cont.chomp != "y"
end

This seems more readable, as we directly talk to our loop itself by breaking it, instead of using variable as a "middleman". Also notice how initializing array inside a loop eliminates need for clearing it.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ In to_s, rather than create a local variable and mutate it inside the each loop, you can just use reduce or, better yet, each_with_object \$\endgroup\$ – Jonah Oct 20 '15 at 14:38
  • \$\begingroup\$ @Jonah I never was a huge fan of each_with_object, but you're probably right. \$\endgroup\$ – Borsunho Oct 22 '15 at 7:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.