Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

ruby refactoring class method

Tags:

ruby

I am new to ruby. I am trying to create a report_checker function that checks how often the word "green, red, amber" appears and returns it in the format: "Green: 2/nAmber: 1/nRed:1".

If the word is not one of the free mentioned, it is replaced with the word 'unaccounted' but the number of times it appears is still counted.

My code is returning repeats e.g if I give it the input report_checker("Green, Amber, Green"). It returns "Green: 2/nAmber: 1/nGreen: 2" as opposed to "Green: 2/nAmber: 1".

Also, it doesn't count the number of times an unaccounted word appears. Any guidance on where I am going wrong?

def report_checker(string)
  array = []
  grading = ["Green", "Amber", "Red"]

    input = string.tr(',', ' ').split(" ")
  
    input.each do |x|
       if grading.include?(x)
        array.push( "#{x}: #{input.count(x)}")
       else
         x = "Unaccounted"
        array.push( "#{x}: #{input.count(x)}")
       end
    end   
    
    array.join("/n")
end

 report_checker("Green, Amber, Green")

I tried pushing the words into separate words and returning the expected word with its count

like image 665
jennifercodes Avatar asked Jun 10 '26 16:06

jennifercodes


1 Answers

There's a lot of things you can do here to steer this into more idiomatic Ruby:

# Use a constant, as this never changes, and a Set, since you only care
# about inclusion, not order. Calling #include? on a Set is always
# quick, while on a longer array it can be very slow.
GRADING = Set.new(%w[ Green Amber Red ])

def report_checker(string)
  # Do this as a series of transformations:
  # 1. More lenient splitting on either comma or space, with optional leading
  #    and trailing spaces.
  # 2. Conversion of invalid inputs into 'Unaccounted'
  # 3. Grouping together of identical inputs via the #itself method
  # 4. Combining these remapped strings into a single string
  string.split(/\s*[,|\s]\s*/).map do |input|
    if (GRADING.include?(input))
      input
    else
      'Unaccounted'
    end
  end.group_by(&:itself).map do |input, samples|
    "#{input}: #{samples.length}"
  end.join("\n")
end

report_checker("Green, Amber, Green, Orange")

One thing you'll come to learn about Ruby is that simple mappings like this translate into very simple Ruby code. This might look a bit daunting now if you're not used to it, but keep in mind each component of that transformation isn't that complex, and further, that you can run up to that point to see what's going on, or even use .tap { |v| p v }. in the middle to expand on what's flowing through there.

Taking this further into the Ruby realm, you'd probably want to use symbols, as in :green and :amber, as these are very tidy as things like Hash keys: { green: 0, amber: 2 } etc.

While this is done as a single method, it might make sense to split this into two concerns: One focused on computing the report itself, as in to a form like { green: 2, amber: 1, unaccounted: 1 } and a second that can convert reports of that form into the desired output string.

like image 191
tadman Avatar answered Jun 13 '26 08:06

tadman



Donate For Us

If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!