Variable validation

I’ve got a class that has a number of instance variables, some of which must
be set prior to calling a save function to write these off somewhere else.
I’m doing the following:

  def check_required
    if @password == nil or @password.empty?
      or @firstName == nil or @firstName.empty?
      or @lastName == nil or @lastName.empty?
      or @nickName == nil or @nickName.empty?
      or @email == nil or @email.empty?
      raise RequiredDataMissingError('')
  end

… but this smells.

I thought about putting them all in an array, then iterating over it – one
downside is I can only put them in an array for the duration of the call
because:

irb(main):001:0> @a = ‘hey’
"hey"
irb(main):002:0> @b = ‘you’
"you"
irb(main):003:0> vars = [@a, @b]
[“hey”, “you”]
irb(main):004:0> @a = ‘yo’
"yo"
irb(main):005:0> vars
[“hey”, “you”]

… shows that there’s not a link kept between the @a in vars and @a.

I guess this is better:

  def check_required
    vars = {
      'password' => @password,
      'firstName' => @firstName,
      'lastName' => @lastName,
      'nickName' => @nickName,
      'email' => @email,
    }

    missing = ''
    vars.each do |name, value|
      missing << ', ' if !missing.empty?
      missing << name if value == nil || value.empty?
    end

    raise RequiredDataMissingError.new("Required fields not set:

#{missing}") if !missing.empty?
end

Any better ideas?

Chris
http://clabs.org

Chris Morris wrote:

I’ve got a class that has a number of instance variables, some of which must
be set prior to calling a save function to write these off somewhere else.
I’m doing the following:

  def check_required
    if @password == nil or @password.empty?
      or @firstName == nil or @firstName.empty?
      or @lastName == nil or @lastName.empty?
      or @nickName == nil or @nickName.empty?
      or @email == nil or @email.empty?
      raise RequiredDataMissingError('')
  end

… but this smells.

If all these are accessible from the outside in reader methods, you
could use an array of symbols and send:

class RequiredDataMissingError < Exception end

class X
MANDATORY_FIELDS = [ :password, :firstName, :lastName,
:nickName, :email ]

attr_accessor *MANDATORY_FIELDS

def check_required
if MANDATORY_FIELDS.find{|field|
value = self.send(field)
value == nil or value.empty?
} then
raise RequiredDataMissingError
end
end
end

This does add a methodcall indirection, which could hurt you
performancewise, but you seem to be (rightfully) trying to optimize for
eyeball time :slight_smile:

···


([ Kent Dahl ]/)_ ~ [ http://www.stud.ntnu.no/~kentda/ ]/~
))_student
/(( _d L b_/ NTNU - graduate engineering - 5. year )
( __õ|õ// ) )Industrial economics and technological management(
_
/ö____/ (_engineering.discipline=Computer::Technology)

Kent Dahl wrote:

Chris Morris wrote:

I’ve got a class that has a number of instance variables, some of which
must be set prior to calling a save function to write these off somewhere
else. I’m doing the following:

  def check_required
    if @password == nil or @password.empty?
      or @firstName == nil or @firstName.empty?
      or @lastName == nil or @lastName.empty?
      or @nickName == nil or @nickName.empty?
      or @email == nil or @email.empty?
      raise RequiredDataMissingError('')
  end

… but this smells.

If all these are accessible from the outside in reader methods, you
could use an array of symbols and send:

class RequiredDataMissingError < Exception end

class X
MANDATORY_FIELDS = [ :password, :firstName, :lastName,
:nickName, :email ]

attr_accessor *MANDATORY_FIELDS

def check_required
if MANDATORY_FIELDS.find{|field|
value = self.send(field)
value == nil or value.empty?
} then
raise RequiredDataMissingError
end
end
end

This does add a methodcall indirection, which could hurt you
performancewise, but you seem to be (rightfully) trying to optimize for
eyeball time :slight_smile:

Or perhaps:

class C
REQD = [‘password’, ‘firstname’, ‘lastname’] #etc.

def check_required
missing = REQD.map {|r| (eval “@#{r}”).to_s == ‘’ ? nil : r}.compact
missing.empty? or raise "How 'bout these, punk? #{missing.join ', '}"
end
end

Or if you hate using eval, you can check instance_variables to see if your field exists.
To check if it’s ‘’ though, you’d need eval, or store a mapping from the variable name to
the value somewhere, or use an accessor method, or … (?)

Another tip: If you’re using constant strings as hash keys, I think it’s better style
and more efficient to use symbols instead: {:a => 1, :b => 2, :c => 3} instead of {‘a’
=> 1, ‘b’ => 2, ‘c’ => 3}. If you need the string value, you can use Symbol#id2name
(aliased to Symbol#to_s) to convert it.

Thanks for the replies - I used a bit of stuff from both and am doing this:

  RequiredAttributes = [:loginName, :password, :firstName, :lastName,
    :nickName, :email]

  attr_accessor *RequiredAttributes

  def check_required
    missing = ''
    RequiredAttributes.each do |sym|
      value = send(sym)
      missing << ', ' if !missing.empty?
      missing << sym.id2name if value == nil || value.empty?
    end

    raise RequiredDataMissingError.new("Required fields not set:

#{missing}") if !missing.empty?
end

Better :slight_smile:

Chris
http://clabs.org

Hi Chris, just one minor bug in the code (see below)

Thanks for the replies - I used a bit of stuff from both and am doing
this:

  RequiredAttributes = [:loginName, :password, :firstName,
  :lastName,
    :nickName, :email]

  attr_accessor *RequiredAttributes

  def check_required
    missing = ''
    RequiredAttributes.each do |sym|
      value = send(sym)
      missing << ', ' if !missing.empty?

the line above should be inside the following if

      missing << sym.id2name if value == nil || value.empty?
    end

    raise RequiredDataMissingError.new("Required fields not set:

#{missing}") if !missing.empty?
end

Regards,
Pit

      missing << ', ' if !missing.empty?

the line above should be inside the following if

      missing << sym.id2name if value == nil || value.empty?

Yeup - good catch. Thanks!

Chris
http://clabs.org