patternrubyMinor
Outputting years with no repeated digits
Viewed 0 times
repeatedoutputtingdigitswithyears
Problem
I'm working through a course now, and was tasked with the following:
Write a function,
You should probably write a helper function,
I did not know about each_char yet, so I wrote the following:
I'm wondering if:
This is the model solution for a beginner:
I'm also wondering about the advantages and disadvantages of my code compared to the model solution. I kind of get that shorter code is better, and readability matters. I don't know how to test code for time yet.
Write a function,
no_repeats(year_start, year_end), which takes a range of years and outputs those years which do not have any repeated digits.You should probably write a helper function,
no_repeat?(year) which returns true/false if a single year doesn't have a repeat.I did not know about each_char yet, so I wrote the following:
def no_repeats(year_start, year_end)
(year_start..year_end).to_a.keep_if {|year| no_repeat?(year)}
end
def no_repeat?(year)
year_array = year.to_s.split(//)
year_array == year_array.uniq
endI'm wondering if:
- there is an easier way to make an array from a string with no spaces
- there is a way to convert each character in a number to an item in a single array without converting the number to a string first
- there is a better way to do this problem
This is the model solution for a beginner:
def no_repeats(year_start, year_end)
no_repeats = []
(year_start..year_end).each do |yr|
no_repeats << yr if no_repeat?(yr)
end
no_repeats
end
def no_repeat?(year)
chars_seen = []
year.to_s.each_char do |char|
return false if chars_seen.include?(char)
chars_seen << char
end
return true
endI'm also wondering about the advantages and disadvantages of my code compared to the model solution. I kind of get that shorter code is better, and readability matters. I don't know how to test code for time yet.
Solution
The model solution they propose is very unidiomatic, your code is far better. I'd just made some minor changes:
I'd write:
- Use
String#charsinstead ofString#split(//).
- Use
Enumerable#select?instead ofArray#keep_ifto avoid modifying data in-place. It's better to work with immutable streams of data (with methods fromEnumerable).
I'd write:
def no_repeats(year_start, year_end)
(year_start..year_end).select { |year| no_repeat?(year) }
end
def no_repeat?(year)
year_digits = year.to_s.chars
year_digits == year_digits.uniq
endCode Snippets
def no_repeats(year_start, year_end)
(year_start..year_end).select { |year| no_repeat?(year) }
end
def no_repeat?(year)
year_digits = year.to_s.chars
year_digits == year_digits.uniq
endContext
StackExchange Code Review Q#144629, answer score: 2
Revisions (0)
No revisions yet.