Стефан обнови решението на 13.10.2013 01:37 (преди около 11 години)
+class Integer
+ #Help function
+ def firstDivision
+ (2..self).each do |i|
+ return i if self % i == 0
+ end
+ end
+
+ #Functions for the homework
+ def prime?
+ flag = self < 0 ? "false" : "true"
+ (2..self - 1).each do |i|
+ flag = "false" if self % i == 0
+ end
+ puts flag
+ end
+
+ def prime_factors
+ arr, selfValue = [], self.abs
+ until selfValue == 1
+ arr = arr << selfValue.firstDivision
+ selfValue = selfValue / selfValue.firstDivision
+ end
+ p arr
+ end
+
+ def harmonic
+ result = Rational(0, 1)
+ (1..self).each do |i|
+ intToRat = Rational(1, i)
+ result = result + intToRat
+ end
+ p result
+ end
+
+ def digits
+ arr, selfValue = [], self.abs
+ until selfValue == 0
+ arr = arr << selfValue % 10
+ selfValue = selfValue / 10
+ end
+ p arr.reverse
+ end
+end
+
+class Array
+ #Help functions
+ def countMatches(item)
+ matches = 0
+ self.each { |i| matches = matches + 1 if item == i }
+ return matches
+ end
+
+ #Functions for homework
+ def frequencies
+ resultHash = Hash.new
+ self.each do |i|
+ resultHash[i] = self.countMatches(i)
+ end
+ p resultHash
+ end
+
+ def average
+ return p nil if self.empty?
+ selfValue, result = self, 0.0
+ selfValue.each do |i|
+ result = result + i
+ end
+ p result / selfValue.length
+ end
+
+ def drop_every(n)
+ resultArr, position = [], 1
+ self.each do |i|
+ resultArr = resultArr << i unless position % n == 0
+ position = position + 1
+ end
+ p resultArr
+ end
+
+ def combine_with(other)
+ small = self.length <= other.length ? self : other
+ large = self.length <= other.length ? other : self
+ result = []
+ (0..small.length - 1).each { |i| result = result << self[i] << other[i] }
+ (small.length..large.length - 1).each { |i| result = result << large[i] }
+ p result
+ end
+end
Ето малко бележки:
- Коментарите, които си сложил, не помагат с нищо, най-добре ги махни
- Имената на методи и променливи в Руби са snake_case, а не camelCase; поправи го (ред 3, ред 19 и други)
- "division" означава "разделяне"; може би искаш да кажеш "first_divisor"
- Надявам се това да е някаква дебъг версия; не си си пускал примерния тест със сигурност; в
prime?
не връщаш резултат, а вадиш нещо на екрана; такъв проблем имаш и в други методи - Тази употреба на низовете "false" и "true" трябва да изчезне от решението ти до сряда, иначе ще ти вземем точки като проверяваме
- За този метод, най-малкото може да ползваш истинските
true
иfalse
обекти, а не низове с тези стойности - Допълнително,
self < 0
резултира вtrue
илиfalse
и няма нужда отself < 0 ? true : false
-
flag
е ужасно име на променлива;is_prime
е малко по-добро, но пак – опитай да го напишеш така, че да избягаш от нуждата да имашflag
- Виж какво правят методите
all?
,any?
,none?
; ще ти помогнат да си напишешprime?
- Предпочитаме вариантите с думи на проверката за делимост, т.е.
remainder(number)
и методитеzero?
иnonzero?
-
arr
е още по-лошо име; съкращавайки няколко символа, не печелиш нищо, а губиш много; спазвай clarity over brevity; тук вместоarr
може да ползваш спокойно иprime_factors
, няма да има проблем - Методът
Array#<<
променя обекта; т.е. няма нужда от присвояването на ред 21, ред 85 и 86 - Като гледам твоя
prime_factors
, мисля, че не ти е нужден методаfirstDivision
изобщо; спокойно можеш да го замениш с локална променлива вprime_factors
- Няма никакъв смислъл от променливата
intToRat
(да не говорим, че е именована не според конвенцията и самото име е лошо); спокойно можеш да сложиш изразаRational(1, i)
в събирането на долния ред; може да ползваш и+=
- За този метод, виж какво прави
reduce
и дали можеш да го ползваш по някакъв начин -
selfValue
също е лошо име - Ред 48 - преди да дефинираш метод за нещо, потърси дали вече го няма в съответния клас (в случая, разгледай методите на
Array
и наEnumerable
); в Руби има много готови методи - В Ruby няма нужда да пишеш
return
, ако искаш да върнеш стойността на последно изпълнения израз в метод; това важи за ред 51; пишеш самоmatches
- Плюс това, смятам, че
occurrances
е по-добро име отmatches
- Никога не създавай хеш с
Hash.new
, ако няма да подаваш аргументи на конструктора; има си литерален синтаксис, който е{}
(ред 56) -
self.
се "подразбира", когато предшества извикване на метод (ще го кажем по-натам) и е излишен; в почти всички такива случаи се пропуска (напр. ред 37, 50, 57, 58, 74...) - Ред 64 не е нужен, няма да викаме
average
на празен списък; освен това, тамp
не трябва да го има, а иnil
е излишно; достатъчно щеше да е самоreturn if empty?
-
selfValue
е абсолютно излишно в този метод, може да го разкараш изцяло -
result
е лошо име;sum
е по-добро в случая - За
drop_every
, разгледай отново методите наEnumerable
(всеки списък ги има); там има неща катоeach_with_index
, например, които обезсмислятposition
- Добра практика за по-четим код е когато имаме няколко реда с равенства един под друг, да ги подравняваш по символа
=
; например, при теб това може да стане на ред 82-84 - Празните редове не се броят от
skeptic
; на някои места помагат за четимостта на кода (добре е да има такива вcombine_with
) - Вместо
a..b - 1
може да ползвашa...b
(non-inclusive range)