## Apologies for pasting here but Stack overflow formatting is simply terrible.
class Person
attr_reader :first_name, :last_name, :separator
def initialize(first_name = nil, last_name = nil, separator = nil)
@first_name = set_first_name(first_name)
@last_name = set_last_name(last_name)
@separator = set_separator(separator)
end
## generally speaking from an OOP point of view, you should be
## deeply skeptical of conditional statements. Ideally, you
## only want to see them in factory methods.
## notice the below method:
## if first.name.nil? --------> this is a conditional
## and we way that if first name is nil then do this: xyz
## that is something which Object oriented programmer's would get
## extremely nervous about.
## an OOP programmer would simply want something like this:
## he/she would want a first name object to be returned.
## def set_first_name(name)
## first_name = FirstName.factory(name)
## return first_name.name
## end
## in other words your first method is x1000 better than your second
## version IMO.
def set_first_name(first_name)
if first_name.nil?
return "Enter Logic for deriving first_name here"
else
return first_name
end
end
def set_last_name(last_name)
if last_name.nil?
return "Enter Logic for deriving first_name here"
else
return last_name
end
end
end
Created
April 13, 2017 11:08
-
-
Save benkoshy/a6575d58e15f806c9d303f8270318210 to your computer and use it in GitHub Desktop.
Restructuring the person class - code review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment