Решение на Първа задача от Стефан Василев

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

Към профила на Стефан Василев

Резултати

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

Код

class Integer
def first_divisor
(2..self).each { |i| return i if remainder(i).zero? }
end
def prime?
((2...self).none? { |i| remainder(i).zero? } and self > 0)
end
def prime_factors
result_array, self_value = [], abs
until self_value == 1
result_array << self_value.first_divisor
self_value /= self_value.first_divisor
end
result_array
end
def harmonic
result = Rational(0, 1)
(1..self).each { |i| result += Rational(1, i) }
result
end
def digits
result_array, self_value = [], abs
until self_value == 0
result_array << self_value % 10
self_value /= 10
end
result_array.reverse
end
end
class Array
def frequencies
result_hash = {}
self.each { |i| result_hash[i] = count(i) }
result_hash
end
def average
return if empty?
sum = 0.0
self.each { |i| sum += i }
sum / length
end
def drop_every(n)
result = []
self.each_index { |i| result << self[i] unless (i + 1).remainder(n).zero? }
result
end
def combine_with(other)
small = length <= other.length ? self : other
large = length <= other.length ? other : self
result = []
(0...small.length).each { |i| result << self[i] << other[i] }
(small.length..large.length - 1).each { |i| result << large[i] }
result
end
end

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

F...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-owuspp/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)>'

  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-owuspp/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.01839 seconds
14 examples, 2 failures

Failed examples:

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

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

Стефан обнови решението на 13.10.2013 01:37 (преди над 10 години)

+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)

Стефан обнови решението на 15.10.2013 00:09 (преди над 10 години)

class Integer
- #Help function
- def firstDivision
- (2..self).each do |i|
- return i if self % i == 0
- end
+ def first_divisor
+ (2..self).each { |i| return i if remainder(i).zero? }
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
+ ((2...self).none? { |i| remainder(i).zero? } and self > 0)
end
def prime_factors
- arr, selfValue = [], self.abs
- until selfValue == 1
- arr = arr << selfValue.firstDivision
- selfValue = selfValue / selfValue.firstDivision
+ result_array, self_value = [], abs
+ until self_value == 1
+ result_array << self_value.first_divisor
+ self_value /= self_value.first_divisor
end
- p arr
+ result_array
end
def harmonic
result = Rational(0, 1)
- (1..self).each do |i|
- intToRat = Rational(1, i)
- result = result + intToRat
- end
- p result
+ (1..self).each { |i| result += Rational(1, i) }
+ result
end
def digits
- arr, selfValue = [], self.abs
- until selfValue == 0
- arr = arr << selfValue % 10
- selfValue = selfValue / 10
+ result_array, self_value = [], abs
+ until self_value == 0
+ result_array << self_value % 10
+ self_value /= 10
end
- p arr.reverse
+ result_array.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
+ result_hash = {}
+ self.each { |i| result_hash[i] = count(i) }
+ result_hash
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
+ return if empty?
+ sum = 0.0
+ self.each { |i| sum += i }
+ sum / 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
+ result = []
+ self.each_index { |i| result << self[i] unless (i + 1).remainder(n).zero? }
+ result
end
def combine_with(other)
- small = self.length <= other.length ? self : other
- large = self.length <= other.length ? other : self
+ small = length <= other.length ? self : other
+ large = 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
+
+ (0...small.length).each { |i| result << self[i] << other[i] }
+ (small.length..large.length - 1).each { |i| result << large[i] }
+ result
end
end