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

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

Към профила на Илиян Танев

Резултати

  • 5 точки от тестове
  • 0 бонус точки
  • 5 точки общо
  • 12 успешни тест(а)
  • 2 неуспешни тест(а)

Код

class Integer
def zero?
self == 0
end
def remainder(number)
self % number
end
def negative?
self < 0
end
def prime?
divisor = 2
return false if negative?
2.upto(Math.sqrt(self)) do |divisor|
return false if remainder(divisor).zero? or negative?
end
true
end
def generate_factors(number, factors=[])
return factors if number == 1
new_factor = (2..number).find {|factor| number.remainder(factor).zero? }
generate_factors(number / new_factor, factors << new_factor)
end
def prime_factors
generate_factors(self.abs)
end
def harmonic
sum = 0r
1.upto(self) do |number|
sum += 1r / number
end
sum
end
def digits
digits = []
number = abs
while number >= 1
digits.unshift(number % 10)
number /= 10
end
digits
end
end
class Array
def frequencies
hash = Hash.new(0)
each{ |item| hash[item] += 1 }
hash
end
def average
sum = 0.to_f
each { |item| sum += item }
sum / size
end
def drop_every(number)
remaining_elements = []
0.upto(size) do |i|
return remaining_elements if not self[i]
remaining_elements << self[i] unless (i + 1).remainder(number).zero?
end
remaining_elements
end
def combine_with(other)
combined = []
size = self.size >= other.size ? self.size : other.size
0.upto(size) do |i|
combined << self[i] unless i >= self.size
combined << other[i] unless i >= other.size
end
combined
end
end

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

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

Failures:

  1) Integer#prime? checks if a number is prime
     Failure/Error: 0.prime?.should   eq false
       
       expected: false
            got: true
       
       (compared using ==)
     # /tmp/d20131023-4395-1e1ne4g/spec.rb:4: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)>'

  2) Integer#digits constructs an array containing the digits of a number
     Failure/Error: 0.digits.should      eq [0]
       
       expected: [0]
            got: []
       
       (compared using ==)
     # /tmp/d20131023-4395-1e1ne4g/spec.rb:44: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.01786 seconds
14 examples, 2 failures

Failed examples:

rspec /tmp/d20131023-4395-1e1ne4g/spec.rb:2 # Integer#prime? checks if a number is prime
rspec /tmp/d20131023-4395-1e1ne4g/spec.rb:43 # Integer#digits constructs an array containing the digits of a number

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

Илиян обнови решението на 12.10.2013 15:34 (преди над 10 години)

+# Integer class extentions
+
+class Integer
+ def number_under_top_border?(divider)
+ result = (self < 0) ? false : (divider <= Math.sqrt(self))
+ return result
+ end
+
+ def prime?
+ divider = 2
+ number_not_prime = ((self % divider) == 0) or (self < 0)
+ while number_under_top_border?(divider) do
+ break if (number_not_prime = ((self % divider) == 0) or (self < 0))
+ divider += 1
+ end
+
+ return (not number_not_prime)
+ end
+
+ # -------------------
+ def push_factors(number, divider, factors)
+ while number % divider == 0 do
+ factors.push(divider)
+ number /= divider
+ end
+
+ return number
+ end
+
+ def push_curr_number(factors, number)
+ factors.push(number) if number > 1
+ end
+
+ def fill_factors(number, divider, factors)
+ while number > 1 do
+ number = push_factors(number, divider, factors)
+
+ if divider * divider > number
+ push_curr_number(factors, number)
+ break
+ end
+
+ divider += 1
+ end
+ end
+
+ def prime_factors
+ divider = 2
+ factors = []
+ number = self.abs
+
+ fill_factors(number, divider, factors)
+ return factors
+ end
+
+ #-------------------------
+ def harmonic
+ result = 0r
+ number = 1r
+ while number <= self do
+ result += 1 / number
+ number += 1
+ end
+
+ return result
+ end
+
+ #-------------------------
+ def digits
+ digits_array = []
+ number = self.abs
+ while number >= 1 do
+ digits_array.unshift(number % 10)
+ number /= 10
+ end
+
+ return digits_array
+ end
+end
+
+
+# Array class extentions
+class Array
+ def put_in_hash(item, hash)
+ if hash[item]
+ hash[item] += 1
+ else
+ hash[item] = 1
+ end
+ end
+
+ def frequencies
+ hash = {}
+ self.each{ |item| put_in_hash(item, hash) }
+ return hash
+ end
+
+ #--------------------
+ def average
+ sum = 0.to_f
+ self.each{|item| sum += item}
+
+ return sum / self.size
+ end
+
+ #---------------------
+ def drop_every(number)
+ i = 0
+ new_array = []
+ while i < self.size do
+ new_array.push(self[i]) unless ((i + 1) % number == 0)
+ i += 1
+ end
+
+ return new_array
+ end
+
+ #---------------------
+ def combine(combined, other)
+ size = self.size >= other.size ? self.size : other.size
+ i = 0
+ while i < size
+ combined.push(self[i]) unless i >= self.size
+ combined.push(other[i]) unless i >= other.size
+ i += 1
+ end
+ end
+
+ def combine_with(other)
+ combined = []
+ combine(combined, other)
+ return combined
+ end
+end

Ето малко бележки:

  • Коментарите, които си сложил, не помагат с нищо, най-добре ги махни
  • В Ruby няма нужда да пишеш return, ако искаш да върнеш стойността на последно изпълнения израз в метод; това важи за ред 6, ред 17 (където скобите са ненужни), ред 27 и т.н.
  • Скобите са ненужни и около условието на ред 5; този метод е малко странно написан :) Може да се запише така (тялото на целия метод, на един ред): self >= 0 and divider <= Math.sqrt(self) – мисля, че е доста по-добре :)
  • divider означава "разделител"; може би имаш предвид divisor, за "делител"?
  • Скобите на ред 11 са излишни; употребата на or тук е проблем, има проблем с приоритетите; ако искаш да прекратиш работата на метода в тези два случая, се ползва постфиксен вариант на условен оператор (if) и return, т.е. return false if self % divider == 0 or self < 0
  • Допълнително, предпочитаме вариантите с думи на проверката за делимост, т.е. remainder(number) и методите zero? и nonzero?
  • Ред 12, ред 22 и други – do не се слага след while и е ненужно; махни го
  • Махни скобите от условието в if-а на ред 13
  • Пак там, избягвай да правиш присвоявания в условни конструкции; лош стил е и ще взимаме точки за това
  • Предпочитай Array#<<, вместо Array#push (ред 23)
  • От push_curr_number не виждам никакъв смисъл - ползва се на едно място и е абсолютно излишна абстракция
  • Използвам случая да споделя, че curr_number е кофти име; съкращавайки няколко символа, не печелиш нищо, а губиш много; спазвай clarity over brevity
  • Като цяло, не мисля, че си спечелил много от изваждането на логика в тези "помощни" методи, около prime_factors; също така, имената им са неподходящи, лоши и подвеждащи; опитай да пренапишеш целият този сегмент; ползвай рекурсия, ако трябва; решението е на няколко реда; имаш твърде много while цикли; подсказка: виж какво правят методи като upto/downto
  • self. се "подразбира", когато предшества извикване на метод (ще го кажем по-натам) и е излишен; в почти всички такива случаи се пропуска (напр. ред 50, 71, 94, 101, 110 и т.н.)
  • Опитай да си опростиш решението на harmonic; виж upto/downto и виж какво прави методът reduce и дали има как да го ползваш там
  • Типа на променливата е излишно да го има в името ѝ; имам предвид digits_array; напълно окей е да го кръстиш и само digits
  • Ред 94 -- трябва да има интервал тук: each {
  • За този метод, виж какво прави Hash.new(0) и виж дали има как да го ползваш, вместо помощния метод, който си си създал
  • Имаш проблем с идентацията на ред 99
  • Ред 101, трябва да има интервали около { и }
  • Очаквам да ползваш пак upto/downto в drop_every, вместо този while и това i
  • Пак там, new_array е лошо име; remaining_elements е по-добро
  • Ред 111, отново foo << bar, вместо foo.push(bar) и махни скобите около условието; може да го запишеш и така: ... unless remainder(i + 1).zero?
  • Не виждам причина да изваждаш метод combine в combine_with; смятам, че с натрупаните до момента от този коментар знания, ще можеш да го напишеш по-добре в рамките на един метод :)

Илиян обнови решението на 15.10.2013 00:59 (преди над 10 години)

-# Integer class extentions
-
class Integer
- def number_under_top_border?(divider)
- result = (self < 0) ? false : (divider <= Math.sqrt(self))
- return result
+ def zero?
+ self == 0
end
- def prime?
- divider = 2
- number_not_prime = ((self % divider) == 0) or (self < 0)
- while number_under_top_border?(divider) do
- break if (number_not_prime = ((self % divider) == 0) or (self < 0))
- divider += 1
- end
+ def remainder(number)
+ self % number
+ end
- return (not number_not_prime)
+ def negative?
+ self < 0
end
- # -------------------
- def push_factors(number, divider, factors)
- while number % divider == 0 do
- factors.push(divider)
- number /= divider
+ def prime?
+ divisor = 2
+ return false if negative?
+
+ 2.upto(Math.sqrt(self)) do |divisor|
+ return false if remainder(divisor).zero? or negative?
end
- return number
+ true
end
- def push_curr_number(factors, number)
- factors.push(number) if number > 1
+ def generate_factors(number, factors=[])
+ return factors if number == 1
+ new_factor = (2..number).find {|factor| number.remainder(factor).zero? }
+ generate_factors(number / new_factor, factors << new_factor)
end
- def fill_factors(number, divider, factors)
- while number > 1 do
- number = push_factors(number, divider, factors)
-
- if divider * divider > number
- push_curr_number(factors, number)
- break
- end
-
- divider += 1
- end
- end
-
def prime_factors
- divider = 2
- factors = []
- number = self.abs
-
- fill_factors(number, divider, factors)
- return factors
+ generate_factors(self.abs)
end
- #-------------------------
def harmonic
- result = 0r
- number = 1r
- while number <= self do
- result += 1 / number
- number += 1
+ sum = 0r
+ 1.upto(self) do |number|
+ sum += 1r / number
end
- return result
+ sum
end
- #-------------------------
def digits
- digits_array = []
- number = self.abs
- while number >= 1 do
- digits_array.unshift(number % 10)
+ digits = []
+ number = abs
+ while number >= 1
+ digits.unshift(number % 10)
number /= 10
end
- return digits_array
+ digits
end
end
-
-# Array class extentions
class Array
- def put_in_hash(item, hash)
- if hash[item]
- hash[item] += 1
- else
- hash[item] = 1
- end
- end
-
def frequencies
- hash = {}
- self.each{ |item| put_in_hash(item, hash) }
- return hash
+ hash = Hash.new(0)
+ each{ |item| hash[item] += 1 }
+
+ hash
end
- #--------------------
- def average
+ def average
sum = 0.to_f
- self.each{|item| sum += item}
+ each { |item| sum += item }
- return sum / self.size
+ sum / size
end
- #---------------------
def drop_every(number)
- i = 0
- new_array = []
- while i < self.size do
- new_array.push(self[i]) unless ((i + 1) % number == 0)
- i += 1
+ remaining_elements = []
+ 0.upto(size) do |i|
+ return remaining_elements if not self[i]
+ remaining_elements << self[i] unless (i + 1).remainder(number).zero?
end
- return new_array
+ remaining_elements
end
- #---------------------
- def combine(combined, other)
+ def combine_with(other)
+ combined = []
size = self.size >= other.size ? self.size : other.size
- i = 0
- while i < size
- combined.push(self[i]) unless i >= self.size
- combined.push(other[i]) unless i >= other.size
- i += 1
+ 0.upto(size) do |i|
+ combined << self[i] unless i >= self.size
+ combined << other[i] unless i >= other.size
end
- end
- def combine_with(other)
- combined = []
- combine(combined, other)
- return combined
+ combined
end
end