Решение на Първа задача от Тихомир Янев

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

Към профила на Тихомир Янев

Резултати

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

Код

#
# Author: Tihomir Yanev
# Web: www.ti6o-bg.com
# Email: mg_91(at)abv.bg
# Last modified: 14.10.2012 19:29
#
# task 1.1
class Integer
def prime_divisors
# Note: it is pointless to verify if self is integer, since we're in its class : )
n = abs
# return empty array if n is in [0,1] range
return [] if n == 0 || n == 1
# create an array which will contain all prime divisiors
result = []
(2..n).each do |i|
# append the number only if is prime divisior
result.push(i) if (n % i).zero? && i.prime?
end
result
end
def prime?
return true if self == 2
return false if even? || self < 2
# look for numbers that divide ours in range:
# 2 * i + 1, 2 * (i + 1) + 1, ... sqrt(self), where i > 0
# Important: we need step(2), since we will verify only odd numbers
(3..Math.sqrt(self).to_i).step(2).each do |i|
# return false if this is not a prime number
return false if (self % i).zero?
end
true
end
end
# end of task 1.1
# task 1.2
class Range
def fizzbuzz
to_a.map do |x|
key_label = ''
# concat if true
key_label << "fizz" if (x % 3).zero?
key_label << "buzz" if (x % 5).zero?
# return original key if 'key_label' is empty, otherwise convert it to symbol
# Note: Use of predicate's brackets to separate from ternary operator
key_label.empty?() ? x : key_label.to_sym
end
end
end
#end of task 1.2
# task 1.3
class Hash
def group_values
result = {}
each do |key, val|
if result.has_key?(val)
result[val].push(key)
else
result[val] = [key]
end
end
result
end
end
# end of task 1.3
# task 1.4
class Array
def densities
result = []
each do |value|
# counts the number of elements which equals to obj and stores it
result.push(count(value))
end
result
end
end
#end of task 1.4

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

........

Finished in 0.01066 seconds
8 examples, 0 failures

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

Тихомир обнови решението на 12.10.2012 02:46 (преди над 11 години)

+#
+# Author: Tihomir Yanev
+# Web: www.ti6o-bg.com
+# Email: mg_91(at)abv.bg
+# Last modified: 12.10.2012 02:29
+#
+
+# task 1.1
+class Integer
+
+ def prime_divisors
+
+ # Note: it is pointless to verify if self is integer, since we're in its class : )
+ n = self.abs
+
+ # return empty array if n is in [0,1] range
+ return [] if n <= 1 && n >= 0
+
+ # create an array which will contain all prime divisiors
+ result = []
+
+ (2..n).each do |i|
+
+ # append the number only if is prime divisior
+ result.push(i) if (n % i).zero? && i.prime?
+
+ end
+
+ # return this array
+ result
+
+ end
+
+ def prime?
+
+ return true if self == 2
+ return false if self.even? || self < 2
+
+ # look for numbers that divide ours in range:
+ # 2 * i + 1, 2 * (i + 1) + 1, ... sqrt(self), where i > 0
+ # Important: we need step(2), since we will verify only odd numbers
+ (3..Math.sqrt(self).to_i).step(2).each do |i|
+
+ # return false if this is not a prime number
+ return false if (self % i).zero?
+
+ end
+
+ # it's a prime number if reaches here
+ true
+ end
+
+end
+# end of task 1.1
+
+
+
+# task 1.2
+class Range
+
+ def fizzbuzz
+ self.to_a.map do |x|
+
+ key_label = ''
+
+ # concat if true
+ key_label << "fizz" if (x % 3).zero?
+ key_label << "buzz" if (x % 5).zero?
+
+ # return original key if 'key_label' is empty, otherwise convert it to symbol
+ # Note: I use predicate's brackets to separate from ternary operator
+ key_label.empty?() ? x : key_label.to_sym
+
+ end
+ end
+
+end
+#end of task 1.2
+
+
+
+# task 1.3
+class Hash
+
+ def group_values
+ result = {}
+
+ self.each do |key, val|
+
+ result.has_key?(val) ? result[val].push(key) : result[val] = [key]
+
+ end
+
+ result
+ end
+
+end
+# end of task 1.3
+
+
+# task 1.4
+class Array
+
+ def densities
+ result = []
+
+ self.each do |value|
+
+ # counts the number of elements which equals to obj and stores it
+ result.push(self.count(value))
+
+ end
+
+ result
+ end
+
+end
+#end of task 1.4
  • Коригирай си идентацията и стила, кодът е подреден доста зле. Консултирай се с ръководството за стил на курса
  • Всичките ти коментари в това решение могат да бъдат премахнати без да се загуби кой знае какво (особено този коментар в началото, който ми напомня за enterprise или странно споделян open source отпреди три века)
  • Пропускай self. — подразбира се и в твоя случай може да се изпусне навсякъде
  • На ред 90 по-добре да имаш if/else, отколкото ternary оператор
  • Ред 17 — n <= 1 && n >= 0 е много странен начин да напишеш n == 0 || n == 1. Старай се кодът ти да издава максимално идеята зад всяка операция.

След като оправиш форматирането и разкараш коментарите, ще погледна пак :)

  1. Коригирах стила

  2. Не смятам коментарите за излишни, това е навик, който трудно изградих и нямам намерение да се отучвам. Вместо това ще съм ти благодарен ако споделиш как би изглеждал един "съвременен" стил на коментиране.

3-5. Забележките ти са уместни, коригирах кода

Тихомир обнови решението на 14.10.2012 20:20 (преди над 11 години)

#
# Author: Tihomir Yanev
# Web: www.ti6o-bg.com
# Email: mg_91(at)abv.bg
-# Last modified: 12.10.2012 02:29
+# Last modified: 14.10.2012 19:29
#
# task 1.1
class Integer
def prime_divisors
# Note: it is pointless to verify if self is integer, since we're in its class : )
- n = self.abs
+ n = abs
- # return empty array if n is in [0,1] range
- return [] if n <= 1 && n >= 0
+ # return empty array if n is in [0,1] range
+ return [] if n == 0 || n == 1
+
+ # create an array which will contain all prime divisiors
+ result = []
+
+ (2..n).each do |i|
- # create an array which will contain all prime divisiors
- result = []
-
- (2..n).each do |i|
-
- # append the number only if is prime divisior
- result.push(i) if (n % i).zero? && i.prime?
-
- end
-
- # return this array
- result
-
+ # append the number only if is prime divisior
+ result.push(i) if (n % i).zero? && i.prime?
+
+ end
+
+ result
+
end
-
+
def prime?
return true if self == 2
- return false if self.even? || self < 2
+ return false if even? || self < 2
# look for numbers that divide ours in range:
- # 2 * i + 1, 2 * (i + 1) + 1, ... sqrt(self), where i > 0
+ # 2 * i + 1, 2 * (i + 1) + 1, ... sqrt(self), where i > 0
# Important: we need step(2), since we will verify only odd numbers
- (3..Math.sqrt(self).to_i).step(2).each do |i|
-
- # return false if this is not a prime number
- return false if (self % i).zero?
-
- end
+ (3..Math.sqrt(self).to_i).step(2).each do |i|
+
+ # return false if this is not a prime number
+ return false if (self % i).zero?
+
+ end
- # it's a prime number if reaches here
true
end
end
# end of task 1.1
# task 1.2
class Range
def fizzbuzz
- self.to_a.map do |x|
-
- key_label = ''
-
- # concat if true
- key_label << "fizz" if (x % 3).zero?
- key_label << "buzz" if (x % 5).zero?
-
- # return original key if 'key_label' is empty, otherwise convert it to symbol
- # Note: I use predicate's brackets to separate from ternary operator
- key_label.empty?() ? x : key_label.to_sym
-
- end
+ to_a.map do |x|
+
+ key_label = ''
+
+ # concat if true
+ key_label << "fizz" if (x % 3).zero?
+ key_label << "buzz" if (x % 5).zero?
+
+ # return original key if 'key_label' is empty, otherwise convert it to symbol
+ # Note: Use of predicate's brackets to separate from ternary operator
+ key_label.empty?() ? x : key_label.to_sym
+
+ end
end
-
end
#end of task 1.2
# task 1.3
class Hash
def group_values
result = {}
-
- self.each do |key, val|
- result.has_key?(val) ? result[val].push(key) : result[val] = [key]
+ each do |key, val|
+
+ if result.has_key?(val)
+ result[val].push(key)
+ else
+ result[val] = [key]
+ end
- end
-
+ end
+
result
end
-
+
end
# end of task 1.3
+
# task 1.4
class Array
def densities
result = []
-
- self.each do |value|
- # counts the number of elements which equals to obj and stores it
- result.push(self.count(value))
-
- end
-
- result
+ each do |value|
+
+ # counts the number of elements which equals to obj and stores it
+ result.push(count(value))
+
+ end
+
+ result
end
-
+
end
-#end of task 1.4
+#end of task 1.4

Коментарите са супер шумни. Например:

# return empty array if n is in [0,1] range
return [] if n == 0 || n == 1

Кодът казва същото, но е далеч по-каратък. Коментарите task и end of task също са излишни. От имената на класовете и методи виждам какво започва и свършва. Говорихме си достатъчно за това в RockIt. Можеш да погледнеш по-широка дискусия за коментари в c2. Иначе:

  • Оставяш твърде много празни редове на произволни места.
  • В Integer#prime_divisors можеш да ползваш select вместо да създаваш масив и да го пълниш.
  • Начина, по който конструираш fizzbuzz е много сложен. Далеч по-добре си с едно условие с четири клона.
  • Можеш да опростиш 87-91 ред по следния начин:

    result[val] ||= []
    result[var] << key
    
  • В Array#densities си изтървал възможност да ползваш map