OOP issue: should I pass an object or an object property?

  • Thread starter Thread starter csharper
  • Start date Start date
Well, isn't that just as simple as employee.EmployeeID? I mean, it's not
a search.
Click to expand...

Searching for contacts based on employee id is a search no matter
if a ref to the employee object or just the employee id is passed.

It would be different if the employee object actually contained
refs to the contacts, but that does not seem to fit the description.

Arne
 
He is getting the contact info from the Employee object, which I presume
is the FirstName, MiddleName, LastName properties.
Click to expand...

Give that contact info usually means phone numbers, email addresses
etc. and that the method does not return ContactInfo but
IEnumerable<ContactInfo>, then I don't think that is the case.

Arne
 
I have been wondering about this for a while. Suppose I have a class
Employee which has defined the following properties:

EmployeeId
FirstName
MiddleName
LastName
... ...

Suppose now in another class, I need to use the employeeId in a method
GetContactInfo.

Now, should I pass an Employee object to GetContactInfo like below?

// Version 1.
public IEnumerable<ContactInfo> GetContactInfo(Employee employee)
{
// The implementation goes here.
}

Or should I simply pass the EmployeeId int like below?

// Version 2.
public IEnumerable<ContactInfo> GetContactInfo(int employeeId)
{
// The implementation goes here.
}

Both will work, but

Q1: From an OOP perspective, which is more OOP-like?
Click to expand...

Assuming that the contact info are looked up outside the employee
object then #2.
Q2: Which version has better performance? Version 1? Version 2? No
difference?
Click to expand...

No difference that has any significance.

Arne
 
I have a third suggestion that you could use instead of options 1 and 2:
Place the GetContactInfo method INSIDE the Employee class. In that way
you would not have to pass any argument. You would simply invoke
result=theEmployee.GetContactInfo();
The GetContactInfo method would then fetch the Id as this.employeeId.

The reasoning from the OOP point of view is the following: A class is
supposed to hold data plus the methods that operate on that data.The
principles of encapsulation and abstraction require that the
implementation details be kept internal to the class. I presume that the
EmployeeID, in practice, is the primary key of a database table that
stores the employees, and it is a foreign key for the ContactDetails
table. All of these are implementation details, which should probably be
kept internal to the class that abstracts the Employee. So you don't
want to be passing around the ID. Let the Employee class know how to get
its own contact details, and don't expose outside of the class the
specific key that is needed to get them.
Click to expand...

That suggestion *combined* with actually storing the contact
info in the employee object make sense.

Just adding the method but not have the data does not look good
to me.

Arne
 
Arne Vajhøj said:
[...]
I have a third suggestion that you could use instead of options 1 and 2:
Place the GetContactInfo method INSIDE the Employee class. In that way
you would not have to pass any argument.
Click to expand...
[...]
That suggestion *combined* with actually storing the contact
info in the employee object make sense.

Just adding the method but not have the data does not look good
Click to expand...

You may, or may not, store the contact data inside the Employee objet.
The point I wanted to make when I made the suggestion was that such storing
of the data would be an internal implementation detail, and it should be
transparent to the callers of Employee.GetContactInfo(). The implementation
details are kept hidden inside the class; the callers should not have to
care about how the class operates internally. In fact, you could write a
first version that fetched the data from the database every time the method
was called, and if the performance happened to be unacceptable, you could
then rewrite it so that the contact info was cached in memory. This change
in implementation would be transparent to the callers; no part of the code
that uses the Employee class would have to be rewritten due to this internal
change in the class.
 
Give that contact info usually means phone numbers, email addresses
etc. and that the method does not return ContactInfo but
IEnumerable<ContactInfo>, then I don't think that is the case.
Click to expand...

Right, and I think this is an excellent case of where a good naming
convention (which this code does NOT have) is critical. Look at all the
developers here who made assumptions about the code not based on the
description the poster gave but on the way things were named!
 
csharper said:
Yes, I agree, decoupling is the key. A lot of times, I only think of
decoupling from the database, which I have tried my best to decouple.
I haven't been aggressively pondering about and designing decoupled
objects. Although I think what you said make very good sense, we do
very often see methods which take an instance of classes, does this
mean many of these may not be well-designed?
Click to expand...

Well, decoupling is only one of many criteria. Certainly if a class is
strongly coupled, it is unlikely to be usable for anything else. This
tends to make the code "throw-away". Sometimes this is acceptible,
other times not. It is certainly short sighted. I think in general,
most programmers are lazy and take the easy way out, so they don't think
in terms of reusablity or ease of testing. It is much easier to test
functions and classes that have weak coupling than those that have
strong coupling. However, sometimes encapsulation makes it more
desirable to pass an object around. This is a business where you have
some guidelines and then you apply common sense plus those guidelines to
achieve a result that meets your criteria. The balance will depend upon
how long lived the project is expected to be (a throw away utility would
have a different balance than a library which is part of a tool kit you
expect to use for years to come.) Things to keep in mind:

Coupling - this affects reusability, testability
Encapsulation - hiding implementation
Maintainability - how easy is it to add/change things
Ease of use

There are probably others I am forgetting at the moment, but these
can often collide with each other and you need to find the proper
balance for your project.

joe
 
Arne Vajhøj said:
[...]
I have a third suggestion that you could use instead of options 1 and 2:
Place the GetContactInfo method INSIDE the Employee class. In that way
you would not have to pass any argument.
Click to expand...
[...]
That suggestion *combined* with actually storing the contact
info in the employee object make sense.

Just adding the method but not have the data does not look good
Click to expand...

You may, or may not, store the contact data inside the Employee objet.
The point I wanted to make when I made the suggestion was that such
storing of the data would be an internal implementation detail, and it
should be transparent to the callers of Employee.GetContactInfo(). The
implementation details are kept hidden inside the class; the callers
should not have to care about how the class operates internally. In
fact, you could write a first version that fetched the data from the
database every time the method was called, and if the performance
happened to be unacceptable, you could then rewrite it so that the
contact info was cached in memory. This change in implementation would
be transparent to the callers; no part of the code that uses the
Employee class would have to be rewritten due to this internal change in
the class.
Click to expand...

Encapsulation is good. But it is not the only part of good OO practice.

He could also add a GetCurrentUSPresident method to the class and
consider it an implementation whether the data were in the class
or was fetched from Wikipedia.

The members of a class should have a logical connection with
the class. Both fields/propertys and methods.

If the contact info domain model wise is part of the class,
then a method to get information makes sense.

I believe that is the case.

If the contact info is not part of the class, then adding
the method would be a utility method.

And I don't like those in the domain model.

If the information is within the class then a good
ORM framework will support both eager and lazy loading
of the data to provide the necessary flexibility.

Arne
 
Back
Top