Решение на Първа задача от Александър Попов

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

Към профила на Александър Попов

Резултати

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

Код

class Integer
def prime?
return true if self == 2
return false if self < 2
if (2..Math.sqrt(self).ceil).any? { |number| (self % number).zero? }
return false
end
true
end
def prime_factors
return [] if self.zero? or self == 1
factor = (2..abs).find { |number| (abs % number).zero? }
[factor] + (abs / factor).prime_factors
end
def harmonic
raise ArgumentError, "Argument is not of type Integer" unless integer?
raise ArgumentError, "Number can't be negative or null" if self <= 0
(1..self).reduce { |sum, number| sum + Rational(1, number) }
end
def digits
abs.to_s.chars.map { |digit| digit.to_i }
end
end
class Array
def frequencies
frequencies = {}
uniq.each { |element| frequencies[element] = count(element) }
frequencies
end
def average
raise ArgumentError, "Method undefined for empty array." if empty?
reduce { |sum, element| sum + element }.to_f / size
end
def drop_every(n)
reject.with_index { |_, index| (index + 1) % n == 0 }
end
def combine_with(other)
combined = []
(0...[size, other.size].max).each do |index|
combined << self[index] unless self[index].nil?
combined << other[index] unless other[index].nil?
end
combined
end
end

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

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

Failures:

  1) Array#combine_with combines two arrays by alternatingly taking elements
     Failure/Error: [:a, :b, :c].combine_with([1, nil, 3]).should       eq [:a, 1, :b, nil, :c, 3]
       
       expected: [:a, 1, :b, nil, :c, 3]
            got: [:a, 1, :b, :c, 3]
       
       (compared using ==)
     # /tmp/d20131023-4395-430aop/spec.rb:110: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.01762 seconds
14 examples, 1 failure

Failed examples:

rspec /tmp/d20131023-4395-430aop/spec.rb:103 # Array#combine_with combines two arrays by alternatingly taking elements

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

Александър обнови решението на 13.10.2013 21:34 (преди около 11 години)

+class Integer
+ def prime?
+ return false if self < 2
+ return true if self == 2
+ (2..Math.sqrt(self).ceil).each { |x| return false if self % x == 0 }
+ true
+ end
+
+ def prime_factors
+ num = self.abs
+ return [] << num if num.prime?
+ (2...num).select { |n| n.prime? and (num % n).zero? }
+ end
+
+ def harmonic
+ raise ArgumentError, "Number can't be negative or null" if self <= 0
+ (1..self).inject(0) { |h, n| h + Rational(1, n) }
+ end
+
+ def digits
+ self.abs.to_s.split("").map { |c| c.to_i }
+ end
+end
+
+class Array
+ def frequencies
+ frequencies = {}
+ self.uniq.each { |x| frequencies[x] = self.count(x) }
+ frequencies
+ end
+
+ def average
+ raise ArgumentError, "Method undefined for empty array." if self.empty?
+ self.inject { |sum, element| sum + element }.to_f / self.size
+ end
+
+ def drop_every(n)
+ reject.with_index { |element, index| (index + 1) % n == 0 }
+ end
+
+ def combine_with(other)
+ result, size1, size2 = [], self.size, other.size
+ while result.size < 2 * [size1, size2].min
+ result << self.shift
+ result << other.shift
+ end
+ result += self if other.empty?
+ result += other if self.empty?
+ end
+end

Малко бележки:

  • За prime? виж дали не можеш да използваш някак методите all?, any? или none?
  • number е по-добро име от x на ред 5
  • Предпочитаме вариантите с думи на проверката за делимост, т.е. remainder(number), zero? и nonzero?
  • Имена като num, h, x, c, size1, size2 са лоши; съкращавайки няколко символа, не печелиш нищо, а губиш много; стреми се да спазваш clarity over brevity
  • На ред 11 може би си искал да напишеш return [number] if number.prime? :) Тук е окей да си ползваш и abs, няма нужда да го слагаш в отделна променлива
  • Грешката, която хвърляш на ред 16, има подвеждащ текст, защото 1) никъде не проверяваш за nil, 2) "nil", а не "null"; иначе е добре, че се хвърля грешка, в духа на "fail early, fail fast"
  • Предпочитаме reduce пред inject
  • На ред 17, по-добре да ползваш sum, вместо h и number вместо n
  • self. се "подразбира", когато предшества извикване на метод (ще го кажем по-натам) и е излишен; в почти всички такива случаи се пропуска (напр. ред 10, 21, 28, 34, ...)
  • За ред 21 виж String#chars и опитай да измислиш по-добро име за c
  • Да дам пример за result в combine_with - по-добре ще е да е combined, например (поне)
  • Пак там, вместо size1, size2, направи си едно smaller_size = [size, other.size].min и ползвай него
  • И внимавай - мутираш списъците в този метод! Пренапиши го, за да не триеш елементи
  • И пак там -- внимавай какво връщаш; в момента може да е и nil

Много благодаря за полезните коментари. Това не е финална версия, тъй като някои от тестовете ми fail-ват, не съм имплемвнтирал препоръките от stуle guide-а и разни други неща. Утре ще рефакторирам, а и ще внимавам за обещаните хинтове по време на лекция. Отново благодаря за бързия feedback :)

P.S.. За грешката на ред 16 - писал съм null, защото имам предвид null като нула, а не nil като нищо.

Александър обнови решението на 14.10.2013 15:27 (преди около 11 години)

class Integer
def prime?
- return false if self < 2
return true if self == 2
- (2..Math.sqrt(self).ceil).each { |x| return false if self % x == 0 }
+ return false if self < 2
+ if (2..Math.sqrt(self).ceil).any? { |number| (self % number).zero? }
+ return false
+ end
true
end
def prime_factors
- num = self.abs
- return [] << num if num.prime?
- (2...num).select { |n| n.prime? and (num % n).zero? }
+ return [] if self.zero? or self == 1
+ factor = (2..abs).find { |number| (abs % number).zero? }
+ [factor] + (abs / factor).prime_factors
end
def harmonic
+ raise ArgumentError, "Argument is not of type Integer" unless integer?
raise ArgumentError, "Number can't be negative or null" if self <= 0
- (1..self).inject(0) { |h, n| h + Rational(1, n) }
+ (1..self).reduce { |sum, number| sum + Rational(1, number) }
end
def digits
- self.abs.to_s.split("").map { |c| c.to_i }
+ abs.to_s.chars.map { |digit| digit.to_i }
end
end
class Array
def frequencies
frequencies = {}
- self.uniq.each { |x| frequencies[x] = self.count(x) }
+ uniq.each { |element| frequencies[element] = count(element) }
frequencies
end
def average
- raise ArgumentError, "Method undefined for empty array." if self.empty?
- self.inject { |sum, element| sum + element }.to_f / self.size
+ raise ArgumentError, "Method undefined for empty array." if empty?
+ reduce { |sum, element| sum + element }.to_f / size
end
def drop_every(n)
- reject.with_index { |element, index| (index + 1) % n == 0 }
+ reject.with_index { |_, index| (index + 1) % n == 0 }
end
def combine_with(other)
- result, size1, size2 = [], self.size, other.size
- while result.size < 2 * [size1, size2].min
- result << self.shift
- result << other.shift
+ combined = []
+ (0...[size, other.size].max).each do |index|
+ combined << self[index] unless self[index].nil?
+ combined << other[index] unless other[index].nil?
end
- result += self if other.empty?
- result += other if self.empty?
+ combined
end
end

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

def prime?
  return true if self == 2
  return false if self < 2 or (2..Math.sqrt(self).ceil).any? { |number| (self % number).zero? }
  true
end

В този случай обаче трети ред щеше да е много над 80 знака. Реших да го променя на (като тогава обаче трябваше да заменя number с n, което противоречи на твоя втори коментар):

def prime?
  return true if self == 2
  return false if self < 2 
  return false (2..Math.sqrt(self).ceil).any? { |n| (self % n).zero? }
  true
end

Накрая, за да може хем да взема под внимание твоята забележка, хем да не прекрачвам 80 знака използвах дългата форма на if:

def prime?
  return true if self == 2
  return false if self < 2
  if (2..Math.sqrt(self).ceil).any? { |number| (self % number).zero? }
    return false
  end
  true
end

В случая какъв е проблемът - не съм се сетил по-креативно решение или просто това е граничен слчучай и в практиката просто би нарушил някое от ограниченията?