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

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

Към профила на Георги Ангелов

Резултати

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

Код

class Integer
def prime?
return false if self < 1
2.upto(Math.sqrt(self)).none? { |number| remainder(number).zero? }
end
def prime_factors
remaining_number = self
2.upto(abs).each_with_object([]) do |number, prime_factors|
while remaining_number.remainder(number).zero?
prime_factors << number
remaining_number /= number
end
end
end
def harmonic
1.upto(self).map { |number| Rational(1, number) }.reduce(:+)
end
def digits
abs.to_s.split('').map(&:to_i)
end
end
class Array
def frequencies
uniq.each_with_object({}) do |element, frequencies|
frequencies[element] = count element
end
end
def average
reduce(:+).to_f / length unless length.zero?
end
def drop_every(n)
each_slice(n).flat_map { |slice| slice.take(n - 1) }
end
def combine_with(other_array)
min_length = [length, other_array.length].min
first_part = take(min_length).zip(other_array.take(min_length)).flatten
remainder = drop(min_length) + other_array.drop(min_length)
first_part + remainder
end
end

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

F.............

Failures:

  1) Integer#prime? checks if a number is prime
     Failure/Error: 1.prime?.should   eq false
       
       expected: false
            got: true
       
       (compared using ==)
     # /tmp/d20131023-4395-fmx2jc/spec.rb:5:in `block (2 levels) in <top (required)>'
     # ./lib/language/ruby/run_with_timeout.rb:5:in `block (3 levels) in <top (required)>'
     # ./lib/language/ruby/run_with_timeout.rb:5:in `block (2 levels) in <top (required)>'

Finished in 0.02048 seconds
14 examples, 1 failure

Failed examples:

rspec /tmp/d20131023-4395-fmx2jc/spec.rb:2 # Integer#prime? checks if a number is prime

История (5 версии и 6 коментара)

Георги обнови решението на 10.10.2013 18:24 (преди над 10 години)

+class Integer
+ def prime?
+ return false if self < 1
+
+ 2.upto(Math.sqrt(self)).all? { |num| self % num != 0 }
+ end
+
+ def prime_factors
+ remaining_num = self
+
+ 2.upto(abs).each_with_object([]) do |num, result|
+ while remaining_num % num == 0
+ result << num
+ remaining_num /= num
+ end
+ end
+ end
+
+ def harmonic
+ 1.upto(self).map { |num| Rational(1, num) }.inject(:+)
+ end
+
+ def digits
+ abs.to_s.split('').map(&:to_i)
+ end
+end
+
+class Array
+ def frequencies
+ uniq.each_with_object({}) do |element, hash|
+ hash[element] = count element
+ end
+ end
+
+ def average
+ inject(:+).to_f / length
+ end
+
+ def drop_every(n)
+ each_slice(n).flat_map { |slice| slice.take(n - 1) }
+ end
+
+ def combine_with(other)
+ min_length = [length, other.length].min
+ first_part = take(min_length).zip(other.take(min_length)).flatten
+ remainder = drop(min_length) + other.drop(min_length)
+
+ first_part + remainder
+ end
+end

Георги обнови решението на 10.10.2013 18:29 (преди над 10 години)

class Integer
def prime?
return false if self < 1
2.upto(Math.sqrt(self)).all? { |num| self % num != 0 }
end
def prime_factors
remaining_num = self
2.upto(abs).each_with_object([]) do |num, result|
while remaining_num % num == 0
result << num
remaining_num /= num
end
end
end
def harmonic
1.upto(self).map { |num| Rational(1, num) }.inject(:+)
end
def digits
abs.to_s.split('').map(&:to_i)
end
end
class Array
def frequencies
uniq.each_with_object({}) do |element, hash|
hash[element] = count element
end
end
def average
+ return nil if length == 0
+
inject(:+).to_f / length
end
def drop_every(n)
each_slice(n).flat_map { |slice| slice.take(n - 1) }
end
def combine_with(other)
min_length = [length, other.length].min
first_part = take(min_length).zip(other.take(min_length)).flatten
remainder = drop(min_length) + other.drop(min_length)
first_part + remainder
end
end
  • Избягвай да съкращаваш, когато именуваш неща; number е по-добре от num и не печелиш нищо, като пропуснеш два символа; същото важи и за remaining_num
  • Предпочитаме варианта на %, който е с думи (remainder(number)); виж и метода zero?
  • result също е лошо име; prime_factors е по-добро
  • Нека предпочитаме reduce пред inject (синоними са)
  • hash също не е добро име на променлива; element_counts ми звучи по-добре
  • Виж дали можеш да направиш #average на един ред някак (hint: възползвай се от върнатата стойност на условните конструкции)
  • drop_every е супер; дори на мен не ми дойде на ум да ползвам flat_map :)
  • На ред 48 бих сложил още един интервал преди символа =, за да е подредено всичко

Решението ти е много добро, браво!

Имам няколко въпроса:

  • Къде видя това с map(&:to_i)?
  • Имаш ли предварителен опит с Ruby?
  • Ако не, откъде си почерпил информация за методите, които си ползвал?

Много благодаря за коментарите, ще преработя кода.

Нямам предварителен опит с Ruby. Видях някъде inject(:+) и ми се стори логично да има начин просто да се подаде име на метод, с който да се map-не. Пробвах с map(:to_i), обаче то явно очакваше да му дам аргумент и така в един въпрос в StackOverflow намерих номера с & .

Иначе прерових документацията на Enumerable (от там flat_map ми се стори полезно) и прегледах style гайда.

reduce и на мен ми звучи по-добре. Каква е причината да ги има тези синоними? Видях още поне 2 метода от Enumerable, които имат синоними, и ми се стори много странно.

Притесняваше ме този while на 12 ред и се опитах да го направя по друг начин, но като се замисля май си е точно на мястото в случая.

Справил си се чудесно.

"Номерът" с & е нещо много интересно и ще говорим за него след някоя друга лекция :)

Относно синонимите – има ги и на други места. Понякога са наследство (езикът е над 20 годишен), а понякога са там, за да може да се ползва по-добре звучащият в определена ситуация (с оглед по-добра четимост, примерно).

Този while е стандартното итеративно решение на проблема. Може да се направи и с рекурсия. Ако оставиш while-а, няма да е проблем.

Георги обнови решението на 10.10.2013 21:44 (преди над 10 години)

class Integer
def prime?
return false if self < 1
- 2.upto(Math.sqrt(self)).all? { |num| self % num != 0 }
+ 2.upto(Math.sqrt(self)).all? { |number| not remainder(number).zero? }
end
def prime_factors
- remaining_num = self
+ remaining_number = self
- 2.upto(abs).each_with_object([]) do |num, result|
- while remaining_num % num == 0
- result << num
- remaining_num /= num
+ 2.upto(abs).each_with_object([]) do |number, prime_factors|
+ while remaining_number.remainder(number).zero?
+ prime_factors << number
+ remaining_number /= number
end
end
end
def harmonic
- 1.upto(self).map { |num| Rational(1, num) }.inject(:+)
+ 1.upto(self).map { |number| Rational(1, number) }.reduce(:+)
end
def digits
abs.to_s.split('').map(&:to_i)
end
end
class Array
def frequencies
- uniq.each_with_object({}) do |element, hash|
- hash[element] = count element
+ uniq.each_with_object({}) do |element, frequencies|
+ frequencies[element] = count element
end
end
def average
- return nil if length == 0
-
- inject(:+).to_f / length
+ length.zero? ? nil : reduce(:+).to_f / length
end
def drop_every(n)
each_slice(n).flat_map { |slice| slice.take(n - 1) }
end
def combine_with(other)
min_length = [length, other.length].min
first_part = take(min_length).zip(other.take(min_length)).flatten
- remainder = drop(min_length) + other.drop(min_length)
+ remainder = drop(min_length) + other.drop(min_length)
first_part + remainder
end
end

Направих промените :) Обаче видях в style guide-a, че пише да предпочитаме ?: вместо едноредов if, което ми се струва странно предвид колко държите на четимостта и думите, а if then else наистина звучи по-добре. Това едно от тези неща дето ще се променят в гайда ли е или предпочитате ?: ?

EDIT: Направих го с unless...

Георги обнови решението на 10.10.2013 21:51 (преди над 10 години)

class Integer
def prime?
return false if self < 1
2.upto(Math.sqrt(self)).all? { |number| not remainder(number).zero? }
end
def prime_factors
remaining_number = self
2.upto(abs).each_with_object([]) do |number, prime_factors|
while remaining_number.remainder(number).zero?
prime_factors << number
remaining_number /= number
end
end
end
def harmonic
1.upto(self).map { |number| Rational(1, number) }.reduce(:+)
end
def digits
abs.to_s.split('').map(&:to_i)
end
end
class Array
def frequencies
uniq.each_with_object({}) do |element, frequencies|
frequencies[element] = count element
end
end
def average
- length.zero? ? nil : reduce(:+).to_f / length
+ reduce(:+).to_f / length unless length.zero?
end
def drop_every(n)
each_slice(n).flat_map { |slice| slice.take(n - 1) }
end
def combine_with(other)
min_length = [length, other.length].min
first_part = take(min_length).zip(other.take(min_length)).flatten
remainder = drop(min_length) + other.drop(min_length)
first_part + remainder
end
end

Това решение ще показваме на следващи лекции, става все по-добро :)

  • Относно not (...).zero?, виж това
  • Като цяло, избягваме ternary оператора, освен за много прости изрази. Когато все пак го ползваме, наистина е във варианта ... ? ... : ...

Някои имена на променливи все още не са ми на нивото на останалите части от решението, но не мога да измисля по-добри в момента. Добра работа!

Георги обнови решението на 11.10.2013 18:36 (преди над 10 години)

class Integer
def prime?
return false if self < 1
- 2.upto(Math.sqrt(self)).all? { |number| not remainder(number).zero? }
+ 2.upto(Math.sqrt(self)).none? { |number| remainder(number).zero? }
end
def prime_factors
remaining_number = self
2.upto(abs).each_with_object([]) do |number, prime_factors|
while remaining_number.remainder(number).zero?
prime_factors << number
remaining_number /= number
end
end
end
def harmonic
1.upto(self).map { |number| Rational(1, number) }.reduce(:+)
end
def digits
abs.to_s.split('').map(&:to_i)
end
end
class Array
def frequencies
uniq.each_with_object({}) do |element, frequencies|
frequencies[element] = count element
end
end
def average
reduce(:+).to_f / length unless length.zero?
end
def drop_every(n)
each_slice(n).flat_map { |slice| slice.take(n - 1) }
end
- def combine_with(other)
- min_length = [length, other.length].min
- first_part = take(min_length).zip(other.take(min_length)).flatten
- remainder = drop(min_length) + other.drop(min_length)
+ def combine_with(other_array)
+ min_length = [length, other_array.length].min
+ first_part = take(min_length).zip(other_array.take(min_length)).flatten
+ remainder = drop(min_length) + other_array.drop(min_length)
first_part + remainder
end
end