Design Decisions

  • Thread starter Thread starter Mike Labosh
  • Start date Start date
M

Mike Labosh

Code is easy. These are two OOP design issues that I am chewing on. Coming
from classic VB, I'm not great at OO design. I prefer to write mile long
spaghetti methods in a Temp module, and then refactor them.

Issue # 1

I have an Employee class that represents the state of an Employee record,
and an EmployeeListItem class that represents Employee stuff to be loaded
into a ListBox, e.g. EmployeeID, FirstName, LastName, ReadOnly FullName,
Overrides ToString().

In both classes, the ReadOnly FullName Property returns a concatenation of
the FirstName and LastName. Also, both classes have a FullNameOrder
property which is a type of the Enum below:

Public Enum FullNameOrdering
FirstLast = 1
LastFirst = 2
LastCommaFirst = 3
End Enum

This way, an application can "choose" how to configure an employee object to
return an already built full name built the desired way.

This enumeration will only be used in these two classes, but it is
definitely used by both of them.

The enumeration must be defined somewhere, and I can't decide whether I
should put the enumeration in the Employee class, EmployeeListItem class, or
in some other global place. I guess I'm asking for where is the correct,
most OO, most modular place to put it?

Issue # 2

Both Employee and EmployeeListItem share the code below. This annoys me.
It smells like this should be common functionality through inheritance, but
it does not make sense for either of the two classes to inherit from the
other. I am thinking that I should make a "Public MustInherit EmployeeBase"
class and put this stuff in there, and then derive Employee and
EmployeeListItem from EmployeeBase. Can someone validate me on that
thought?

Public Enum FullNameOrdering
FirstLast = 1
LastFirst = 2
LastCommaFirst = 3
End Enum

Private _fullNameOrder As FullNameOrdering _
= FullNameOrdering.FirstLast

Private _employeeID As Integer = 0
Private _lastName As String
Private _firstName As String

Public Sub New()
Public Sub New(ByVal employeeID As Integer, _
ByVal lastName As String, _
ByVal firstName As String)

Public Sub New(ByVal employeeID As Integer, _
ByVal lastName As String, _
ByVal firstName As String, _
ByVal fullNameOrder As FullNameOrdering)

Public Property FullNameOrder As FullNameOrdering
Public Property EmployeeID As Integer
Public Property LastName As String
Public Property FirstName As String
Public ReadOnly Property FullName As String

Public Overrides Function ToString() As String

--
Peace & happy computing,

Mike Labosh, MCSD
Owner, vbSensei.Com
"Escriba coda ergo sum." -- vbSensei
 
#1:
It really depends on the reusability. For the most reusable, you would make
it more global. Then, multiple classes can use it. If Employee is really the
only one using it, then you can tone it down. I would generally leave it out
of the class, myself, as enums have a way of finding reuse.

Now, where you place the actual code is largely your choice, as exposure and
placement are different things. You can make a .cs file that simply contains
enums, which will make it more maintainable. Do not confuse placement of
source code with exposure.

#2:
Do not inherit a common base class as there is no common "is a"
relationship.

Example 1: CORRECT
An employee is a type of person, so we derive employee from person.

Example 2: INCORRECT
An old car and a factory both smoke, so we derive from the same base class.

If you find common signatures, but different implementation, try interfaces.
If you find that both use identical code, a helper method, or enum exposed
enough for both to use (your case), is the better method.

To your example:

EmployeeBase

Ask these questions
1. An employee is an EmployeeBase? NO
2. An EmployeeListItem is an EmployeeBase? NO

Proper OO says do not do this.

--
Gregory A. Beamer
MVP; MCP: +I, SE, SD, DBA

**********************************************************************
Think Outside the Box!
**********************************************************************
 
Mike,

May I make the following suggestion first ditch the EmployeeListItem class
why do you need it when you can simply load the Employee object instances
into the listbox?

Dan
 
#2:
Do not inherit a common base class as there is no common "is a"
relationship.
Click to expand...

I could easily be wrong here, but I'm not sure that I can agree with you.
If I do this inheritance hierarchy:

Abstract EmployeeBase
Concrete Employee
Concrete EmployeeListItem

This gets me these advantages:

1. EmployeeBase provides the state, properties and constructors for the
values of employee that are required by the system (EmployeeID is an
identity, FirstName & LastName are non-null)

2. EmployeeBase can also provide for all the validation rules (EmployeeID
can't be changed, and length overrun checking for FirstName and LastName)

3. Employee and EmployeeListItem receive all this plumbing for free

4. It gives me the flexibility to load a ListBox with EmployeeBase objects
so that in one circumstance, the app might load EmployeeListItems into it,
and in another circumstance, the app might load Employees into it, so that
the list box contains all data that a form requires without (a) having to
make more round trips and (b) the front-side app in a distributed system
having to use ADO.NET

5. "Junior Dev Guy" can use my class library in terms of working with
well-ordered strongly typed collections of my business objects, rather than
coding to ADO .NET or having to understand the schema of the database. To
get his data, he simply calls a method of a middle tier object that returns
him an Employee or a collection of them, then to save his data, he simply
passes his modified Employee to my middle tier method.
If you find common signatures, but different implementation, try
Click to expand...
interfaces.

Common signatures AND identical implementation, which suggests that I should
use either inheritance, or a Friend helper method.
If you find that both use identical code, a helper method, or enum exposed
enough for both to use (your case), is the better method.
Click to expand...

I thought of that, but it seems like spaghetti to me. If I were going to
fully modularize these two classes with helper methods, then I would need a
friend class (or module) that has a friend helper method for each property
get, each property set, and each constructor. All the calls back and forth
would make me cry anytime I had to step through it all.
To your example:

EmployeeBase

Ask these questions
1. An employee is an EmployeeBase? NO
2. An EmployeeListItem is an EmployeeBase? NO
Click to expand...

In this case, I feel like I want to disagree here, but I did try to validate
my thoughts by digging through object browser to see if MS used a similar
tactic anywhere in the Framework class library. I took a look at items in
System.Collections, because there are all those classes that represent a set
of items, ArrayList, Dictionary, HashTable, BitArray, etc. None of those
items inherits from any common thing other than System.Object, but they all
do implement common interfaces.

If I used a similar approach, I would need an IEmployee interface, my two
Employee classes AND all the spaghetti to re-implement all the interface
items with friend methods in a module. I think this would turn into SO many
lines of code that it would make the issue of code reuse a moot point.
Proper OO says do not do this.
Click to expand...

Got any good book references on "Proper OO" other than GoF Design Patterns?
I already have that one.
--
Peace & happy computing,

Mike Labosh, MCSD
Owner, vbSensei.Com
"Escriba coda ergo sum." -- vbSensei
 
May I make the following suggestion first ditch the EmployeeListItem class
why do you need it when you can simply load the Employee object instances
into the listbox?
Click to expand...

That thought did occur to me, but when the listbox only needs to have an ID
and a name in each item, I really should have to eat all the memory required
to load whole giant employee objects.

Especially, since in this case, the system is a training aid that's based on
the Northwind database, and each of these employee objects contains a big
fat IMAGE item and a big fat NTEXT item from the database.

BTW, I'm sure that to some degree, this whole issue is just because of my
obsessive compulsive perfectionist flaws, and I now have about a dozen
napkins here with employee class diagrams scribbled out. If I were using
Visio to design this, I'd have had several more birthdays by now.
--
Peace & happy computing,

Mike Labosh, MCSD
Owner, vbSensei.Com
"Escriba coda ergo sum." -- vbSensei
 
Mike,
The distinction between Employee and EmployeeListItem is unclear. If you
are going to create a base class I would think that you would have something
like:

Employee
-->FullTimeEmployee
-->PartTimeEmployee
-->Consultant

EmployeeListItem sounds to me like a GUI specific object that should proably
not be in you Object Model hiearchy.
Dan
 
Mike,

It sounds like you need to look at either creating
a proxy class
implement a lazy load on the larger properties.
and Or creating an EmployeeFactory Class

I believe that Inheritance is not the answer for this domain problem

Dan
 
Hi Mike,

Is this a Christmas story

There was Adam and Eve and as snake.

Not only for fun, because that snake is how that listitem looks for me in
this.

:-))

Cor
 
EmployeeListItem sounds to me like a GUI specific object that should
proably
not be in you Object Model hiearchy.
Click to expand...

AHH! Now there's a thought.

But then it won't be as easy to code my middle tier method GetComboList()
that returns an array or collection of EmployeeListItem
--
Peace & happy computing,

Mike Labosh, MCSD MCT
Owner, vbSensei.Com
"Escriba coda ergo sum." -- vbSensei
 
My turn ...

If I read the thread correctly you are saying that you think it would be a
good idea to have a base class, (EmployeeBase), and 2 classes, (Employee and
EmployeeListItem), both of which are derived from the base class.

My read on this is that:

EmployeeBase would be the engine.

Employee would have ALL of the functionality
of EmployeeBase plus some extra that EmployeeListIem does not have.

EmployeeListItem would have ALL of the functionality
of EmployeeBase plus some extra that Employee does not have.

You then seem to be saying that for EmployeeListItem, you only need a small
part of the functionality of EmployeeBase.

It would appear that the only things that are common to both Employee and
EmployeeListItem are the ID and Name properties.

If this is so then these are the only properties that should be in
EmployeeBase and the rest of the implementation should be in the Employee
and EmployeeListitems as required.

In my opinion, (real world practicality rather than acedemic correctness), I
wouldn't waste my time creating 3 classes when I can create 2 classes
regardless of whether some of the code is repeated or not.

If indeed, you intend to have an 'engine' class and then have drived classes
with minor overrides/additions to what the 'engine' provides then I would
agree that inheritance is the way to go.
 
Back
Top