Ruby Acronym Creator from string

1.5k Views Asked by At

I'm creating a function that takes a string and creates an acronym but am running into errors. When I input "Complementary metal-oxide semiconductor" I get "CS" in return when expecting "CMOS". Any suggestions why this might happen? I pass it plenty of other strings and it works, just doesn't work in this case.

class Acronym

    def self.abbreviate(phrase)
        letters = phrase.split("")
        acronym = []
        letters.each do |letter|
            previous = letters.index(letter) - 1
            if previous == -1
                acronym.push(letter)
            elsif letters[previous] == " " || letters[previous] == "-"
                acronym.push(letter)
            end
        end
        acronym.join("").upcase
    end

end
4

There are 4 best solutions below

2
On BEST ANSWER

The issue with your code is that index() returns the first occurrence of the given letter. So, two problems:

  1. The 'm' in 'metal' is not the first occurrence of 'm' in the string. It appears in the word 'complementary'. Thus, whenever it sees an 'm' in the string, previous will always be 'o' and thus not trigger a push().
  2. Anytime the first letter in your string recurs (regardless of position), it will trigger your first condition. You can see the effect if you change the initial 'C' to 'c' in your test string. The result will be CSCC because there are two 'c's in 'semiconductor'.

As an alternative, here is an option that uses regex:

def self.abbreviate(phrase)
  phrase.gsub('-', ' ')
        .scan(/(\A\w|(?<=\s)\w)/)
        .flatten
        .join.upcase
end

Step by step:

  1. Borrowing the .gsub from @DollarChills to turn the '-' into a space.
  2. scan() returns an array of all matches. The regex matches the first word in the string and any word that is preceded by a space.
  3. The result of the scan is actually an array of arrays, so flatten unnests them.
  4. Combine into a string and upcase
0
On

You have a bug in previous = letters.index(letter) - 1

see if you can spot it:

arr = [:a, :b, :c, :a]
previous_indexes = arr.map { |n| arr.index(n) - 1 }
you_are_expecting = [-1, 0, 1, 2]

previous_indexes == you_are_expecting
# => false

arr.index(:a) # => 0
arr.index(:b) # => 1
arr.index(:c) # => 2
arr.index(:a) # => 0

To get indexes with iterating, use with_index:

arr = %i[a b c a]
arr.map.with_index { |x, i| [x, i] }
# => [[:a, 0], [:b, 1], [:c, 2], [:a, 3]]

If you make that fix, your code does what you intended.

A suggestion though: you can often avoid dealing with the details of array indexes. Take a look at how @Mori's answer works by operating at a higher level.

3
On

Simplifies to

def acronym(str)
  str.split(/ |-/).map(&:first).join.upcase
end

The above depends on the Rails activesupport library. Here's a Ruby-only variation:

str.split(/ |-/).map { |s| s[0] }.join.upcase 
1
On

You could try using gsub to ignore the hyphen.

<%= ('Complementary metal-oxide semiconductor').gsub('-', ' ') %>

Returns: Complementary metaloxide semiconductor