3
\$\begingroup\$
  1. Build a system for entering and displaying the allergies that patients may have.
  2. The allergy will have its own set of symptoms reactions. The allergy will also have a spectrum of severity which the clinician should be aware of, and allergies can be reported by the patient or by next of kin.

Please review my code and tell me how I can improve my object-oriented design.

class Allergy 

    attr_accessor :name, :reporter, :time_reported
    attr_reader :symptoms

    def initialize(name, reporter, time_reported=Time.now)
        @name = name
        @reporter = reporter
        @symptoms = []
        @time_reported = time_reported
    end


    def add_symptoms(symptom, severity)
        symptom = Symptom.new(symptom, severity)
        @symptoms.push(symptom)
    end


end

class Symptom
    attr_accessor :symptom, :severity

    def initialize(symptom, severity)
        @symptom = symptom
        @severity = severity
    end

end
\$\endgroup\$
1
  • \$\begingroup\$ You tagged this as ruby-on-rails. Is this part of a Rails app? If so, why isn't it an active-record model? \$\endgroup\$ Commented Jul 25, 2016 at 5:27

1 Answer 1

1
\$\begingroup\$

Two things to notice..

    def add_symptoms(symptom, severity)      

Actually, you don't add many symptoms, but only one :)

Secondly, but this has nothing to do with object oriented. Why is the sympton name given to attr_accessor? Do you expect to change it at runtime (e.g: changing "Rash" to "Sneezes"?) Same thing with time_reported, do you expect it to be updatable? :/

Secondly, but I think this is more a "taste" of matter.. I'd honestly prefer to pass something similar:

def add_symptoms(symptom) 

with symptom already being the object instantiated. Why? Because if you are to then have multiple subclasses of Symptom (e.g:

class HeavySimptom
class RespiratorySimptom 
class AmbiguousSymptom

) your code would be quite a pain to change

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.