From: Derek Cannon on
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

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.
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.

-- Derek
--
Posted via http://www.ruby-forum.com/.

From: Robert Klemme on
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/
From: Robert Dober on
On Thu, Apr 15, 2010 at 8:00 AM, Robert Klemme
<shortcutter(a)googlemail.com> wrote:
> 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
>
But please be aware that the Struct based implementation is a super
implementation of OP's as it exposes the attributes write-ably too.
Cheers
R.
<snip>

--
The best way to predict the future is to invent it.
-- Alan Kay

From: Derek Cannon on
>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

"def self.name" makes the method static, right? Is there any particular
reason why it is best to do this than have the url part in the
constructor? What exactly is "modularity"? What do you gain from doing
this?

Thanks,
Derek
--
Posted via http://www.ruby-forum.com/.

From: Josh Cheek on
[Note: parts of this message were removed to make it a legal post.]

On Thu, Apr 15, 2010 at 11:39 PM, Derek Cannon
<novellterminator(a)gmail.com>wrote:

> "def self.name" makes the method static, right?


Yes (in the Java sense of the word).


> Is there any particular reason why it is best to do this than have the url
> part in the constructor?


Then you can create objects of this class that are not based off of that
url. What if you decide you want to group courses together, under different
controllers? Maybe the website isn't updated, and you want to enter them
from the course catalogue instead of the website, or maybe you want to bring
them in from a second site, or maybe store them locally rather than querying
the web every time you run the program. This way the class gives you the
ability to create through that website, but doesn't mandate it, or tie the
class itself to this particular implementation of how to get it started.

What exactly is "modularity"?


Code that doesn't know about, or rely on internals of other code. Thus it
will not break if used differently, or in a different context. It does not
rely on special knowledge of implementation, or details that may change.
When this happens it is called coupling, when your code is not coupled, you
can take it and combine it together with many other pieces of code and it
will operate as desired. For example, if you download the web data into a
file, you can then use it to pull from the file rather than the page,
because it is modular enough to support a different source.

Think how unix commands are small with a specific function, but you can
combine them in powerful ways. These small programs are modular.

Or relative vs absolute file paths. When they are relative, you can place
them anywhere, and they still work. Drag and drop that directory, give it to
friends, etc. If they are absolute, you must go alter that file path every
time you move it. The absolute file path is coupled to the file's location.


What do you gain from doing this?
>

On a program that will never change, or that is very small, you don't really
see the benefits. But when you have to go back later and make a change, you
want to be able to only make it in the places that logically deal with that
change. Everything else should still work fine.

If your file is big, and coupled, and you need to make a change, you will
know, because you will have this scared feeling in your stomache that you
may have just made a change that could break something completely different
somewhere else that you can't even conceive of. Or you may want to make some
specific change, but then wind up going through every line of code you have
in order to update all sorts of things all over the place. This shouldn't
happen.