0
\$\begingroup\$

Task description: Implement a generate_unique_phone_numbers function. The function shall parse the given text-file.

Content-example:

Trinidad Kemp,(838) 352-2212
Noel Bush,(647) 477-7093
Claudette Davila,(686) 868-8153
Shaun Mcbride,(433) 380-7556
Katy Blanchard,(792) 460-2468
...

Then return a Set with unique phone-numbers. Means: Some phone-numbers might be included multiple times.

My solution:

require "set"

def generate_unique_phone_numbers(path)
  numbs = Set.new
  if File.exist?(path)
    lines = File.readlines(path)
    lines.each do |line|
      numbs.add line.chomp.split(",").last
    end
  else
    puts "File does not exist."
  end
  numbs
end

Are there any improvements concerning my solution, which should be implemented?

Would you have done it completely differently? Then how and why?

\$\endgroup\$
1
  • \$\begingroup\$ I don't understand "Some phone-numbers might be included multiple times". This directly contradicts "return a Set with unique phone-numbers" \$\endgroup\$
    – radarbob
    Sep 8 at 1:19

1 Answer 1

1
\$\begingroup\$

This is how I'd do it:

require "set"
require "csv"

def csv_to_iterator(path)
  CSV.open(path).collect { |(_name, number)| number } 
rescue Errno::ENOENT
  []
end

def unique(iterator)
  iterator.to_set
end

def generate_unique_phone_numbers(path)
  unique(csv_to_iterator(path))
end

Ideally I would wrap it in a class, but for simplicity methods will do for this example. I'm not sure how I feel about handling exception in the first method, but I decided to include it here since you have some handling in case a file doesn't exist in your example as well.

Rationale:

  1. My methods are targeting an interface instead of a concrete implementation. So for instance, it will be easier to replace csv_to_iterator to something else, let's say a DB call in case our data source changes, as long as they quack like an Enumerable.

  2. The methods are congruent. In the example you might return a Set or nil (due to puts), in my case I'm always returning a set, doesn't matter what. This create a coherent interface for the execution.

  3. Well defined responsibilities. In the example, a single method is managing opening a file, verifying its stats, parsing, and removing duplicates. I broke down this into multiple steps and in smaller functions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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