Coding practise

Yes that’s in my mind too. What i wrote was again a dirty, inefficient
code. I considered on the way you saw me. Here it is:

class AmicableNumbers
def initialize(max = 1000)
@numbers = Hash.new
findFriends(max)
end
def findFriends(max)
1.upto(max) do |j|
t1, t2 = 1, 1
2.upto(j / 2) {|i| t1 += i if j % i == 0}
2.upto(t1 / 2) {|i| t2 += i if t1 % i == 0}
@numbers.store(t1, t2) if j == t2 && t1 != j
end
end
def has_friend?(number)
return @numbers.has_key?(number)
end
def friend_of(number)
return @numbers[number]
end
end

amicable_numbers = AmicableNumbers.new(5000)
1.upto(5000) {|i| print i, “\t<->\t”, amicable_numbers.friend(i), “\n”
if amicable_numbers.has_friend?(i)}

I know there are a lot of improvement points here but they are not so
important for me right now. I can improve later after constructing the
true structure.
In above example i have a class definition of amicable numbers. When i
call its constructer method with no arguments it finds the amicable
numbers within the range of 0…1000 and stores them at @numbers
instance variable. By “has_friend?” and “friend_of” methods i’m
looking for a friend number and getting the value of the friend number
for a given number.

I’ll improve it or maybe write a new one and post it here as i gain
more info and experience on Ruby.

sempsteen wrote:

  t1, t2 = 1, 1
  2.upto(j / 2) {|i| t1 += i if j % i == 0}
  2.upto(t1 / 2) {|i| t2 += i if t1 % i == 0}
  @numbers.store(t1, t2) if j == t2 && t1 != j
end

This is very inefficient compared to creating a hash of all sums of
divisors. You need to realize you are recomputing certain quantities
over
and over again.

Maybe you would consider profiling this method and comparing it to the
approach of creating a hash of all the sums.

On 26.11.2006 21:04, sempsteen wrote:

I know there are a lot of improvement points here but they are not so
important for me right now. I can improve later after constructing the
true structure.
In above example i have a class definition of amicable numbers. When i
call its constructer method with no arguments it finds the amicable
numbers within the range of 0…1000 and stores them at @numbers
instance variable. By “has_friend?” and “friend_of” methods i’m
looking for a friend number and getting the value of the friend number
for a given number.

Just let me note this: doing heavy calculations in constructors feels
not right. A constructor’s main purpose is to do initialization - which
you can argue is exactly what you do. But since this involves quite a
bit of calculation I would do this differently. (For example, create a
class method that does the calculation and returns a new instance that
stores the result.)

Kind regards

robert