6
\$\begingroup\$

This is a solution for the Day 8 hackerrank.com challenge. The basic idea of the challenge is to create a mapping of names to phone numbers based on the the given input, and later look up the phone numbers for certain names. If the entry exists, print it and its number. If it doesn't, print out "Not found"

Here's my solution:

n = gets.strip.to_i
phonebook = Hash.new

n.times do   
    name = gets.strip
    number = gets.strip 
    phonebook[name] = number           
end  

n.times do 
    check_name = gets.strip   
    if phonebook.has_key?(check_name)
        print "#{check_name}=#{phonebook[check_name]}"
        puts ""
    else
        puts "Not found"
    end
end    

I was wondering about more efficient and elegant Ruby solution. I'm not sure how big sample input was for some of those test cases, but Testcase #1,2,3 take up to 0.4s which seems a little bit too high for me.

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

Other than the fact that you process N queries rather than continuing until EOF is reached, your solution is fine. The main point is to understand how a Hash works in Ruby, and you've accomplished that goal well.

I think that your main inefficiency is calling both print and puts

print "#{check_name}=#{phonebook[check_name]}"
puts ""

… instead of just puts "#{check_name}=#{phonebook[check_name]}". I/O calls can be surprisingly time-consuming.


While your code is fine, I'd like to point out that Ruby can be a bit more slick and compact.

There's no need to strip before calling to_i:

n = gets.to_i

You can define the phonebook with a one-liner, using the Hash[ [key, value], … ] ] constructor.

phonebook = Hash[(0...n).collect {[gets.strip, gets.strip]}]

One way to process STDIN line by line is to use IO::each_line with a block.

STDIN.each_line do |query|
  query.strip!
  puts phonebook.has_key?(query) ? "#{query}=#{phonebook[query]}" : 'Not found'
end
\$\endgroup\$
4
\$\begingroup\$

You got it (mostly) right, I'd say. I wouldn't worry about runtime on hackerrank. They're not going to throw massive resources at every piece of code someone uploads. You're basically sharing their server with everyone else, so they're throttling things. Plus, there's just basic network lag.

Still, there's a bug:

N is the number of phonebook entries to read, but after that you should read to the end of input. But your code just reads N times in both cases.

Other things:

  • The Ruby convention is 2 spaces of indentation. Not 4 spaces, not tabs.

  • Don't use print and then puts with an empty line; just use puts.

  • Don't use strip unless you want exactly what it does. Most often, you'll see gets.chomp for input handling. #chomp only strips the trailing linebreak, but leaves other whitespace intact.

And you can shorten the if..else to a ternary:

N = gets.chomp.to_i # make this a constant, because why not
phonebook = {}      # shorter way to write Hash.new

N.times do
  name = gets.chomp
  number = gets.chomp
  phonebook[name] = number
end

until STDIN.eof? do # read to end-of-file (i.e. end of input)
  name = gets.chomp
  number = phonebook[name]
  puts number ? "#{name}=#{number}" : "Not found"
end

However, you could also do something else:

N = gets.chomp.to_i
phonebook = Hash.new { "Not found" } # set a default value for undefined keys

N.times do
  name = gets.chomp
  number = gets.chomp
  phonebook[name] = "#{name}=#{number}" # set the entire output string
end

# Will print the name=number string or "Not found"
puts phonebook[gets.chomp] until STDIN.eof?
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot. I wasn't aware it actually took servers response time in to consideration. \$\endgroup\$ – Lotix Jan 9 '16 at 12:27
  • \$\begingroup\$ @Lotix Well, I don't know exactly how it calculates runtime. But that's sort of the point; it's just a rough indicator of performance. You can use it to compare two solutions, for instance, and see which is faster. But it's not a an absolute measure. \$\endgroup\$ – Flambino Jan 9 '16 at 15:04
  • \$\begingroup\$ I'm not a big fan of the conditional operator. It is required in languages like C, where the conditional statement is, well, a statement, and the conditional operator is needed because operators are expressions. In Ruby, however, there are not statements anyway, everything is an expression, including conditionals. I almost always find puts if number then "#{name}=#{number}" else 'Not found' end easier to read. Plus, the conditional expression has the precedence you would expect, whereas there are tons of questions on SO which boil down to "ternary precedence is weird, use copious amounts … \$\endgroup\$ – Jörg W Mittag Jan 21 '16 at 11:42
  • \$\begingroup\$ … of parentheses", which can be completely avoided by using the conditional expression instead of the conditional operator. \$\endgroup\$ – Jörg W Mittag Jan 21 '16 at 11:42

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.