4
\$\begingroup\$

I have finally finished a Ruby calculator project, which is not based on eval. Instead it parses input char by char. The project is hosted on GitHub.

However, I find a specific part of the program very annoying since whenever I have to add something I have to rewrite a portion of it again.

def trig
  case @look
  when 'c'
    match_all('cos')
    case @look
    when 'h'
      match_all('h(')
      value = cosh(calculate)
    when '('
      match('(')
      value = cos(calculate)
    else
      expected('cos() or cosh()')
    end
  when 's'
    match('s')
    if @look == 'q'
      match_all('qrt(')
      value = sqrt(calculate)
    else
      match_all('in')
      case @look
      when 'h'
        match_all('h(')
        value = sinh(calculate)
      when '('
        match('(')
        value = sin(calculate)
      else
        expected('sin() or sinh()')
      end
    end
  when 'r'
    match_all('root')
    base = get_number
    match('(')
    value = calculate ** (1.0/base)
  when 't'
    match_all('tan')
    case @look
    when 'h'
      match_all('h(')
      value = tanh(calculate)
    when '('
      match('(')
      value = tan(calculate)
    else
      expected('tan() or tanh()')
    end
  when 'l'
    match('l')
    case @look
    when 'n'
      match_all('n(')
      value = log(calculate)
    when 'o'
      match_all('og')
      if digit? @look
        base = get_number
      elsif @look == "("
        base = 10
      else
        expected("integer or ( ")
      end
      match('(')
      value = log(calculate, base)
    else
      expected('ln() or log()')
    end
  when 'e'
    match_all('exp(')
    value = exp(calculate)
  when 'a'
    match_all('arc')
    case @look
    when 'c'
      match_all('cos')
      case @look
      when 'h'
        match_all('h(')
        value = acosh(calculate)
      when '('
        match('(')
        value = acos(calculate)
      else
        expected('arccos() or arccosh()')
      end
    when 's'
      match_all('sin')
      case @look
      when 'h'
        match_all('h(')
        value = asinh(calculate)
      when '('
        match('(')
        value = asin(calculate)
      else
        expected('arcsin() or arcsinh()')
      end
    when 't'
      match_all('tan')
      case @look
      when 'h'
        match_all('h(')
        value = atanh(calculate)
      when '('
        match('(')
        value = atan(calculate)
      else
        expected('arctan() or arctanh()')
      end
    end
  else
    raise InvalidInput, "unexpected input: \"#{@look}\""
  end
  match(')')
  value
end

The @look variable will contain the next character. The match_all function will simply match the input character by character. For example, it would match c and if it works it continues to match o and so on, but if it doesn't it will output an error.

\$\endgroup\$
2
\$\begingroup\$

Here's a toy program to illustrate the separation of user input collection from the string matching that determines which calc to use, as we discussed in the comments. Note that the class NamedCalculations has no knowledge of IO with the user. All the IO is encapsulated in CmdLineCalculator.

When I find a match, I just evaluate it with the argument pi. In your real application, you'd need to do further process to gather the user's input for the argument.

Also note how if we decide we want to change which calculations we support (one of the most likely changes for the application), it becomes a 1 line change, and the user input processing code doesn't need to be touched.

class NamedCalculations

  # a hash of named calcs
  # eg, {cos: Math.method(:cos), sin: Math.method(:sin) }
  def initialize(calc_definitions)
    @calc_definitions = calc_definitions
    @calc_names = @calc_definitions.keys.map(&:to_s)
  end

  def method_for(calc_name)
    calc_name = calc_name.to_sym
    raise "Invalid Calculation Name" unless @calc_definitions.key? calc_name
    @calc_definitions[calc_name]
  end

  def exact_match?(input)
    @calc_names.any? {|name| input == name}
  end

  def partial_match?(input)
    @calc_names.any? {|name| name.include? input}
  end

end


class CmdLineCalculator

  def initialize(named_calcs)
    @named_calcs = named_calcs
    @input = ''
  end

  def start
    begin
      system("stty raw -echo")
      collect_user_input while not calc_complete?
      show_answer
    ensure
      system("stty -raw echo")
    end
  end

  private

  def collect_user_input
    @input += next_input_char
    validate_input
    echo_last_char
  end

  def validate_input
    raise "Invalid Calculation" unless @named_calcs.partial_match? @input
  end

  def next_input_char
    STDIN.getc
  end

  def echo_last_char
    STDOUT.putc @input[-1]
  end

  def calc_complete?
    @named_calcs.exact_match? @input
  end

  def show_answer
    selected_method = @named_calcs.method_for(@input)
    value_at_pi = selected_method.call(Math::PI)
    STDOUT.puts "\nThe #{@input}(pi) = #{value_at_pi}" 
  end

end

my_calcs = {cos: Math.method(:cos), sin: Math.method(:sin) }
client = CmdLineCalculator.new(NamedCalculations.new(my_calcs))
client.start
\$\endgroup\$
1
\$\begingroup\$

Should sqrt, exp, etc. be in your Trig function? This seems to break the S in SOLID. I would abstract out your trig function and write something like:

def trig(input, val)
  case input
  when "sinh" return sinh(val)
  when "sin" return sin(val)
  when "cosh" return cosh(val)
  ... // cos and tans
  end
end

You were right to be concerned about repeating similar pieces of code. That's something that should be avoided at all times. Coding is about being concise and minimal.

\$\endgroup\$
  • \$\begingroup\$ The problem with this approach is that I don't know the next character . For example, if I find an s I don't know if it is sin or sqrt \$\endgroup\$ – Mhmd Sep 20 '15 at 8:19
  • \$\begingroup\$ @Mhmd, How do you not know the next character? Does match_all collect user input 1 character at a time? Can you explain why it works this way? \$\endgroup\$ – Jonah Sep 21 '15 at 3:02
  • \$\begingroup\$ @jonah yes it does that, I did this based on the "let's build a compiler" book and then added some functionality. \$\endgroup\$ – Mhmd Sep 21 '15 at 6:32
  • \$\begingroup\$ @Mhmd, In that case the first thing you want to do is to separate the logic for matching a string to a computuation (sin, cos, etc getting mapped to their matching methods) from the user input collection. You should have one method that is collecting user input, and after each charcter is typed, it should check if there is a match yet. Does that make sense. You have the matching method responsible for collecting input. But the two things are totally separate. The matching method should know nothing about input collection. \$\endgroup\$ – Jonah Sep 21 '15 at 18:17
  • \$\begingroup\$ @Jonah thanks could you write this as an answer. \$\endgroup\$ – Mhmd Sep 21 '15 at 18:19

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.