Criticise me

[Matthew_Graeme]

Hi all, busy working on my classes and would appreciate any consturctive or
negative advice on wether I am doing the “correct” thing here, or better if
there are better ways, before I start getting entrenched in bad habits some
experienced persons advice may save me a lot of pain feel free to say
what you want , it wont offend

require “C:/dev/cgi-bin/DbHighlander.rb”

class Project

SQL_PROJECT = "SELECT prj_id, prj_name, prj_description,

prj_commenced, prj_completed, prj_status,prj_acc_id FROM project "

def initialize(id=nil)

	@data = nil
	
	if defined? id
		
		db = DbHighlander.Connection
		qry = db.prepare(SQL_PROJECT + " WHERE prj_id =

#{project_id} ")
qry.execute
@data = qry.fetch_hash
qry.finish

	end
	
end


#Class Attributes

def Code
	return @data['prj_id']
end

def Name(name)
	if defined? name then @data['prj_name'] = name end
	return @data['prj_name]
end

def Description(description)
	if defined? name then @data['prj_description'] = description

end
return @data['prj_description]
end
end

Graeme Matthew
Analyst Programmer
Mercer Investment Consulting
Level 29, 101 Collins Street, Melbourne, VIC, 3001, Australia
Tel - 61 3 9245 5352 Fax - 61 3 9245 5330
visit http://www.merceric.com

···

__

---

This e-mail and any attachments may be confidential or legally privileged.
If you received this message in error or are not the intended recipient, you
should destroy the e-mail message and any attachments or copies, and you are
prohibited from retaining, distributing, disclosing or using any information
contained herein. Please inform us of the erroneous delivery by return
e-mail.

Thank you for your cooperation.

---

ec03/04

[Stefan_Schmiedl]

ok, here we go:

require “C:/dev/cgi-bin/DbHighlander.rb”

class Project

SQL_PROJECT = "SELECT prj_id, prj_name, prj_description,
prj_commenced, prj_completed, prj_status,prj_acc_id FROM project "

def initialize(id=nil)

  @data = nil
  
  if defined? id
  	
  	db = DbHighlander.Connection
  	qry = db.prepare(SQL_PROJECT + " WHERE prj_id =

#{project_id} ")
qry.execute
@data = qry.fetch_hash
qry.finish

  end

end

By makeing the parameter id optional, it will always be defined:

irb(main):001:0> def a(b=nil)
irb(main):002:1> if defined? b
irb(main):003:2> puts “defined”
irb(main):004:2> end
irb(main):005:1> end
nil
irb(main):006:0> a
defined
nil
irb(main):007:0> a(1)
defined
nil

And you might want to wrap your database connection with “begin … ensure … end”
to, well, ensure that it is really closed.

Right now I am not sure if you do something terribly wrong by naming
your methods with uppercase initials, but it is at least very unusual
to do so as uppercase initials usually denote constants.

#Class Attributes

def Code
return @data[‘prj_id’]
end

def Name(name)
if defined? name then @data[‘prj_name’] = name end
return @data['prj_name]
end

there is a ’ missing above, and I think you don’t mean “name” below.
I guess it could be “description” which you’re testing, in which case
I have two remarks:

• name and description will always be defined or an ArgumentError
  will be raised:
  irb(main):008:0> def c(d)
  irb(main):009:1> if defined? d
  irb(main):010:2> puts “defined”
  irb(main):011:2> end
  irb(main):012:1> end
  nil
  irb(main):013:0> c(1)
  defined
  nil
  irb(main):014:0> c(nil)
  defined
  nil
  irb(main):015:0> c
  ArgumentError: wrong number of arguments(0 for 1)
  from (irb):15:in `c’
  from (irb):15
  irb(main):016:0>

• you could save some typing by using the following helper method
  for setting up all of your similar accessor methods. Keep DRY

  irb(main):001:0> def caching_attr_reader(symbols)
  irb(main):002:1> symbols.each do | sym |
  irb(main):003:2 module_eval “def #{sym}(arg) @data[‘prj_#{sym}’] ||= arg; end”
  irb(main):004:2> end
  irb(main):005:1> end
  nil
  irb(main):006:0> class A
  irb(main):007:1> def initialize() @data={}; end
  irb(main):008:1> caching_attr_reader :one, :two, :three
  irb(main):009:1> end
  [:one, :two, :three]
  irb(main):010:0> a = A.new
  #<A:0x401c5b64 @data=>
  irb(main):011:0> a = A.new
  #<A:0x40213d40 @data={}>
  irb(main):012:0> a.one(“hi”)
  “hi”
  irb(main):013:0> a.inspect
  “#<A:0x40213d40 @data={"prj_one"=>"hi"}>”
  irb(main):014:0>

def Description(description)
if defined? name then @data[‘prj_description’] = description
end
return @data['prj_description]
end
end

HTH
s.

···

On Fri, 20 Sep 2002 00:44:00 GMT, Matthew, Graeme Graeme.Matthew@mercer.com wrote: