Решение на Първа задача от Орлин Христов

Обратно към всички решения

Към профила на Орлин Христов

Резултати

  • 6 точки от тестове
  • 0 бонус точки
  • 6 точки общо
  • 8 успешни тест(а)
  • 0 неуспешни тест(а)

Код

class Integer
def prime_divisors
n = abs
2.upto(n).each_with_object [] do |divisor, prime_divisors|
return prime_divisors if n == 1
next if n % divisor != 0
prime_divisors << divisor
while n % divisor == 0 do n /= divisor end
end
end
end
class Range
def fizzbuzz
to_a.map do |x|
case x % 15
when 3, 6, 9, 12
:fizz
when 5, 10
:buzz
when 0
:fizzbuzz
else
x
end
end
end
end
class Hash
def group_values
each_with_object({}) do |(k, v), groups|
groups[v] = groups[v] ? groups[v] << k : [k]
end
end
end
class Array
def densities
density = Hash.new(0)
each { |x| density[x] += 1 }
map { |x| density[x] }
end
end

Лог от изпълнението

........

Finished in 0.00729 seconds
8 examples, 0 failures

История (4 версии и 2 коментара)

Орлин обнови решението на 10.10.2012 02:54 (преди около 12 години)

+class Integer
+ def prime_divisors
+ n = self
+ result = []
+ for divisor in 2.upto(self)
+ break if divisor > n
+ next if n % divisor != 0
+
+ result << divisor
+ while n % divisor == 0 do n /= divisor end
+ end
+ return result
+ end
+end
+
+class Range
+ def fizzbuzz
+ self.to_a.map {|x| case x % 15
+ when 3, 6, 9, 12
+ :fizz
+ when 5, 10
+ :buzz
+ when 0
+ :fizzbuzz
+ else
+ x
+ end }
+ end
+end
+
+class Hash
+ def group_values
+ value_hash = {}
+ self.each {|key, value| if value_hash.key? value
+ value_hash[value] << key
+ else
+ value_hash[value] = [key]
+ end }
+ return value_hash
+ end
+end
+
+class Array
+ def densities
+ density = Hash.new(0)
+ self.each {|x| density[x] += 1}
+ self.map {|x| density[x]}
+ end
+end

Орлин обнови решението на 10.10.2012 03:07 (преди около 12 години)

class Integer
def prime_divisors
n = self
result = []
- for divisor in 2.upto(self)
+ 2.upto(self).each do |divisor|
break if divisor > n
next if n % divisor != 0
result << divisor
while n % divisor == 0 do n /= divisor end
end
return result
end
end
class Range
def fizzbuzz
self.to_a.map {|x| case x % 15
when 3, 6, 9, 12
:fizz
when 5, 10
:buzz
when 0
:fizzbuzz
else
x
end }
end
end
class Hash
def group_values
value_hash = {}
self.each {|key, value| if value_hash.key? value
value_hash[value] << key
else
value_hash[value] = [key]
end }
return value_hash
end
end
class Array
def densities
density = Hash.new(0)
self.each {|x| density[x] += 1}
self.map {|x| density[x]}
end
end

Изобщо не спазваш конвенциите за стил. Хвърли един поглед на style guide-a на Божидар и обнови, моля.

Допълнително, на няколко места self. и return са напълно излишни, което ние смятаме за лош стил.

Фигурните скоби в Hash#group_values и Range#fizzbuzz ме плашат. Моля, оправи ги :)

Орлин обнови решението на 14.10.2012 11:04 (преди около 12 години)

class Integer
def prime_divisors
n = self
result = []
2.upto(self).each do |divisor|
break if divisor > n
next if n % divisor != 0
result << divisor
while n % divisor == 0 do n /= divisor end
end
- return result
+ result
end
end
class Range
def fizzbuzz
- self.to_a.map {|x| case x % 15
- when 3, 6, 9, 12
- :fizz
- when 5, 10
- :buzz
- when 0
- :fizzbuzz
- else
- x
- end }
+ self.to_a.map do |x|
+ case x % 15
+ when 3, 6, 9, 12
+ :fizz
+ when 5, 10
+ :buzz
+ when 0
+ :fizzbuzz
+ else
+ x
+ end
+ end
end
end
class Hash
def group_values
- value_hash = {}
- self.each {|key, value| if value_hash.key? value
- value_hash[value] << key
- else
- value_hash[value] = [key]
- end }
- return value_hash
+ groups = {}
+ each do |key, value|
+ if groups.has_key? value
+ groups[value] << key
+ else
+ groups[value] = [key]
+ end
+ end
+ groups
end
end
class Array
def densities
density = Hash.new(0)
self.each {|x| density[x] += 1}
self.map {|x| density[x]}
end
end

Орлин обнови решението на 15.10.2012 11:27 (преди около 12 години)

class Integer
def prime_divisors
- n = self
- result = []
- 2.upto(self).each do |divisor|
- break if divisor > n
+ n = abs
+ 2.upto(n).each_with_object [] do |divisor, prime_divisors|
+ return prime_divisors if n == 1
next if n % divisor != 0
- result << divisor
+ prime_divisors << divisor
while n % divisor == 0 do n /= divisor end
end
- result
end
end
class Range
def fizzbuzz
- self.to_a.map do |x|
+ to_a.map do |x|
case x % 15
when 3, 6, 9, 12
:fizz
when 5, 10
:buzz
when 0
:fizzbuzz
else
x
end
end
end
end
class Hash
def group_values
- groups = {}
- each do |key, value|
- if groups.has_key? value
- groups[value] << key
- else
- groups[value] = [key]
- end
+ each_with_object({}) do |(k, v), groups|
+ groups[v] = groups[v] ? groups[v] << k : [k]
end
- groups
end
end
class Array
def densities
density = Hash.new(0)
- self.each {|x| density[x] += 1}
- self.map {|x| density[x]}
+ each { |x| density[x] += 1 }
+ map { |x| density[x] }
end
end
  • Integer#prime_divisors трябва да ти бъде на два отделни метода - един който проверява дали число е просто и един, който акумулира всички прости числа. Щеше да е ОК да направиш Integer#prime?.
  • В този ред на мисли, temp променливи (n в Integer#prime_divisors) са донякъде лош стил, защото много изчисления в Ruby могат да се направят без тях.
  • n, k, v и най-вече x са лоши имена на променливи. Нищо няма да ти стане, ако напишеш по-дълго и дескриптивно име.
  • #each_with_object много ми хареса като идея. Трябва да го добавим в слайдовете.
  • С case-а в Range#fizzbuzz просто се лигавиш. По-добре да го беше направил с if x % 3 == 0 и т.н. По-лесно щеше да се чете. Също е по-близо до проблема — условието казва "числа, които се делят на три", докато твоето решение казва "числа, чиито остатък при делене на 15 е"
  • groups[v] = groups[v] ? groups[v] << k : [k] се чете малко трудно. Най-вече защото в тернарното условие има страничен ефект. По-добре да го беше направил на два реда.

    groups[v] ||= [] groups[v] << k

Отвъд това, много хубаво си се справил. Браво.