Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Restructuring the person class - code review
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.