Илиян обнови решението на 12.10.2013 15:34 (преди около 11 години)
+# 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
; смятам, че с натрупаните до момента от този коментар знания, ще можеш да го напишеш по-добре в рамките на един метод :)