From: Robert Klemme <shortcutter@...>
Date: 2010-04-15T15:00:06+09:00
Subject: Re: Is this good OOP structuring?

On 15.04.2010 02:46, Derek Cannon wrote:
> Hello everyone. I'm trying to get a hang of object-oriented programming
> and I have a question about the best practices for my program.
>
> I'm making a program to recommend courses to me for my school. To
> simplify things, let's just say every course has 3 variables (they
> actually have a lot more): a name, a day, and a time.
>
> I've made the following, let me know if I can improve on it or am making
> any conventional errors (note, again I've simplified the classes by
> omitting irrelevant methods):
>
> class Course
>    attr_reader :title, :days, :time
>    def initialize(title, days, time)
>      @title = title
>      @days = days
>      @time = time
>    end
> end

That's perfectly OK although you can shorten that by using Struct's power:

Course = Struct.new :title, :day, :time

If there are more methods you can do

Course = Struct.new :title, :day, :time do
   def another_method
   end
end

> class CourseController
>    attr_reader :courses_all, :courses_MW, :courses_TR
>    def initialize(html_file)
>      # gets the HTML course data
>      # creates array where each element is a instance of Course
>      # creates arrays using original array's select to get all courses on
> MW
>      #      and TR, so they can be manipulated separately if desired.

Depending on whether your CourseController is immutable or not: if it is 
immutable (i.e. the list of courses does not change) then there is no 
harm in separating courses.  But if the list can change you need to 
maintain consistency between various views on the courses.  In that case 
I'd start out with a single list and do the selection on demand.

>    end
> end
>
> class Main
>    def initialize
>      courses = CourseController.new("www.schoolcourselisting.com")
>      courses.courses_MW each { |i| puts i } # Shows MW courses
>    end
> end
>
> What do you think? Any improvements I should make? CourseController is
> doing a lot more work than I show. For example, it parses the HTML for
> data, and returns that data.

And apparently it is also downloading the data from a URL, or is it?  if 
it does then I would probably keep the IO out of the constructor.  If 
you make the constructor accept the raw text and provide class methods 
for convenient access you gain modularity:

class CourseController
   def initialize(...) # whatever it needs
   end

   def self.read_url(url)
     ...
     cc = new(...)
     ...
     cc
   end

   def self.read_file(file_name)
   end
end

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/