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

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

Към профила на Милан Миланов

Резултати

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

Код

class Integer
def prime?
return false if self <= 1
2.upto(Math.sqrt(self).to_i) do |number|
return false if self % number == 0
end
true
end
def prime_divisors
2.upto(abs).select { |number| number.prime? && abs % number == 0 }
end
end
class Range
def fizzbuzz
fizzbuzzed = []
each do |i|
if i % 15 == 0 then fizzbuzzed << :fizzbuzz
elsif i % 3 == 0 then fizzbuzzed << :fizz
elsif i % 5 == 0 then fizzbuzzed << :buzz
else fizzbuzzed << i
end
end
fizzbuzzed
end
end
class Hash
def group_values
group_hash = Hash.new()
each do |key, value|
group_hash[value] ||= []
group_hash[value] << key
end
group_hash
end
end
class Array
def densities
collect { |x| count x }
end
end

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

........

Finished in 0.00933 seconds
8 examples, 0 failures

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

Милан обнови решението на 12.10.2012 00:39 (преди около 12 години)

+class Integer
+ def prime?
+ return false if self <= 1
+ 2.upto(Math.sqrt(self).to_i).each { |num| return false if (self % num).zero? }
+ true
+ end
+
+ def prime_divisors
+ 2.upto(self.abs).select { |num| (self.abs % num).zero? && num.prime? }
+ end
+end
+
+
+class Range
+ def fizzbuzz
+ fizzbuzzed = []
+ each do |i|
+ if (i % 15).zero? then fizzbuzzed << :fizzbuzz
+ elsif (i % 3).zero? then fizzbuzzed << :fizz
+ elsif (i % 5).zero? then fizzbuzzed << :buzz
+ else fizzbuzzed << i
+ end
+ end
+ fizzbuzzed
+ end
+end
+
+class Hash
+ def group_values
+ group_hash = Hash.new { |hash, key| hash[key] = [] }
+ each { |key, value| group_hash[value] << key }
+ group_hash
+ end
+end
+
+class Array
+ def densities
+ collect { |x| count x }
+ end
+end
  • Разбивай нетривиалните блокове на повече от един ред, сега се затруднява четенето им.
  • self. се изпуска обикновено, когато не е нужно. При теб не е нужно никъде.
  • (... % ...).zero? по-добре го запиши като ... % ... == 0, понеже скобите тук са неидиоматична употреба.
  • Обръни внимание, в че Hash#group_values връщаш хеш с особено свойство. Като му поиска несъществуващ ключ, човек ще получи празен списък, което може да е изненадващ страничен ефект. Може би искаш да го избегнеш по някакъв начин.

Иначе, като цяло решението ти е прилично. Само не се притеснявай да слагаш нови редове тук-таме и не се стреми да постигнеш ∞ плътност на кода :)

Милан обнови решението на 12.10.2012 17:17 (преди около 12 години)

class Integer
+
def prime?
return false if self <= 1
- 2.upto(Math.sqrt(self).to_i).each { |num| return false if (self % num).zero? }
+ 2.upto(Math.sqrt(self).to_i).each { |num| return false if self % num == 0 }
true
end
def prime_divisors
- 2.upto(self.abs).select { |num| (self.abs % num).zero? && num.prime? }
+ 2.upto(abs).select { |num| abs % num == 0 && num.prime? }
end
+
end
class Range
+
def fizzbuzz
fizzbuzzed = []
each do |i|
- if (i % 15).zero? then fizzbuzzed << :fizzbuzz
- elsif (i % 3).zero? then fizzbuzzed << :fizz
- elsif (i % 5).zero? then fizzbuzzed << :buzz
+ if i % 15 == 0 then fizzbuzzed << :fizzbuzz
+ elsif i % 3 == 0 then fizzbuzzed << :fizz
+ elsif i % 5 == 0 then fizzbuzzed << :buzz
else fizzbuzzed << i
end
end
fizzbuzzed
end
+
end
class Hash
+
def group_values
group_hash = Hash.new { |hash, key| hash[key] = [] }
each { |key, value| group_hash[value] << key }
+ group_hash.default = nil
group_hash
end
+
end
class Array
+
def densities
collect { |x| count x }
end
+
end
  • Без нови редове веднага след началото и веднага преди края на дефиницията на клас; мисля, че се вижда ясно в style guide-а
  • Явно трябва да го кажа изрично: моля те поне 5-ти ред да го разбиеш с do/end на няколко реда :)
  • num е по-лошо име от number
  • За group_values, ако трябва да set-ваш и после да махаш дифолтна стойност на хеш, по-добре не я слагай изобщо, ами давай по идиоматичния начин, с едно if-че. Идиом е some_variable ||= default, т.е. group_hash[value] ||= [].

Най-важното е да не пестиш редове за сметка на четимост!

Милан обнови решението на 14.10.2012 06:52 (преди около 12 години)

class Integer
-
def prime?
return false if self <= 1
- 2.upto(Math.sqrt(self).to_i).each { |num| return false if self % num == 0 }
+ 2.upto(Math.sqrt(self).to_i) do |number|
+ return false if self % number == 0
+ end
true
end
-
+
def prime_divisors
- 2.upto(abs).select { |num| abs % num == 0 && num.prime? }
+ 2.upto(abs).select { |number| number.prime? && abs % number == 0 }
end
-
end
-
class Range
-
def fizzbuzz
fizzbuzzed = []
+
each do |i|
if i % 15 == 0 then fizzbuzzed << :fizzbuzz
elsif i % 3 == 0 then fizzbuzzed << :fizz
elsif i % 5 == 0 then fizzbuzzed << :buzz
else fizzbuzzed << i
end
end
+
fizzbuzzed
end
-
end
class Hash
-
def group_values
- group_hash = Hash.new { |hash, key| hash[key] = [] }
- each { |key, value| group_hash[value] << key }
- group_hash.default = nil
+ group_hash = Hash.new()
+ each do |key, value|
+ group_hash[value] ||= []
+ group_hash[value] << key
+ end
+
group_hash
end
-
end
class Array
-
def densities
collect { |x| count x }
end
-
end