Design Question

[slantedwalker]

Hello,

A quick design question.

Suppose you have a class that looks something like this:

class Record {
private int recordId;
private String recordName;
private String data;

// getters & setters follow
....
}

Now I want to want to persist this "Record" to a database. I've seen 2
ways to do this:

1. I add a 'persist()' method to the Record class like this:
class Record {
private int recordId;
private String recordName;
private String data;

// getters & setters follow
....
public void persist() {
// do the persisting here...
}
}

2. Send the Record object to a Utility class like this:

class RecordUtil {
public static void persist(Record record) {
// do the persisting here...
}
}

I've read that if you are going to do work you should do the work in
the place where the data resides. In this case this would mean that
adding the persist() method to the Record class would be the proper
thing to do.
However, I see lots of programmers use the "Util" class approach
shown above. What would be the advatage of using a Util class rather
than encapsulating the functionality in Record?

 

[VisionSet]

Hello,

A quick design question. ....
I've read that if you are going to do work you should do the work in
the place where the data resides. In this case this would mean that
adding the persist() method to the Record class would be the proper
thing to do.
However, I see lots of programmers use the "Util" class approach
shown above. What would be the advatage of using a Util class rather
than encapsulating the functionality in Record?

It depends on what your record class is going to be used for and what other
responsibilities it has. If it is purely to hold state then you could say
you have a cohesive design by adding persistence responsibility. But if it
does a bunch of other stuff then perhaps not. The other important
consideration is decoupling. If your Record class is to be useful I imagine
you want to have it accessed by something in front of your domain/business
logic or that logic itself. Therefore you have Data access code in your
domain tier and your design ceases to be decoupled. It is good to be solid
on the fundamental reasoning behind cohesive and decoupled design. When
that is clear your choice in your situation will be apparent. You can
always subclass some base domain class eg Record or have a Record interface,
your subtypes can then implement various strategies such as persistence and
wrap non-persisting state only varieties, thus decoupling your design.

 

[Mark Space]

Hello,

A quick design question.
I've read that if you are going to do work you should do the work in
the place where the data resides. In this case this would mean that
adding the persist() method to the Record class would be the proper
thing to do.
However, I see lots of programmers use the "Util" class approach
shown above. What would be the advatage of using a Util class rather
than encapsulating the functionality in Record?

Hello Walker!

The first thing that occurs to me is that there is a practical limit to
the number of lines one wants in any given block of code. It's just a
maintenance issue. Once a chunk of code (whether {} block, method,
class or file) gets too big, it's just too tough to maintain.

So ideally, perhaps one could say that the persist() method really does
belong inside the Record class. But unless the Record class is fairly
small and simple, and expected to stay that way for a long time, it's
probably best to break out the functionality into smaller chunks where
it can be worked on, expanded, and generally mucked with easier.

What if your bean guy and your database gal are two different people?
Trying to change the same file is always a pain. Put those methods into
different files (and different classes) and you've got one less headache
to worry about. Of course, knowing how small of chunks to break your
functionality into is more of an art than a science.

There used to be a general guideline for the maximum size of a given
Java file, class, or method. I thought it was in the coding
conventions. I can't find it. You might look around on your own, to
see what kind of heuristics you can find.

And like you, I'm still rather a newbie. Further comments on my ideas
are welcome.

 

[Zaph0d]

I would choose a different, though more chumbersome, approch, which is
based on seperating as much as possible.
------code------
interface RecordPersistor{ //not public - package visibility only
persist(Record record);
}

class RecordDBPersistor implements RecordPersistor {} //for example

public interface Record { //all your data getters/setters }

class RecordImpl implements Record { //package visiblity
public RecordImpl(RecordPersistor rp){/*saves the rp object*/}
public persist(){ rp.persist(this);}
}

public RecordFactory {
Record createRecord(/*params, persist method*/){ return new
RecordImpl(new RecordDBPersistor());} //for example
------code------

 

[Mark Space]

I would choose a different, though more chumbersome, approch, which is
based on seperating as much as possible.

This is good, and very general. I recognize at least some of the ideas
here. However, if the designer/coder doesn't understand why things are
being implemented this way, it may not buy him or her much. Can I ask
what design patterns are being used here? Any good links to a reference
explaining when (and when not) to use this technique?

------code------
interface RecordPersistor{ //not public - package visibility only
persist(Record record);
}

class RecordDBPersistor implements RecordPersistor {} //for example

public interface Record { //all your data getters/setters }

class RecordImpl implements Record { //package visiblity
public RecordImpl(RecordPersistor rp){/*saves the rp object*/}
public persist(){ rp.persist(this);}

I'm curious about persist() here. Is it part of the Record interface,
or do you extend the interface here? (Or, does it matter?)

}

public RecordFactory {
Record createRecord(/*params, persist method*/){ return new
RecordImpl(new RecordDBPersistor());} //for example

This is good. I at least recognize the factory design pattern here.

 

[Daniel Dyer]

I would choose a different, though more chumbersome, approch, which is
based on seperating as much as possible.
------code------
interface RecordPersistor{ //not public - package visibility only
persist(Record record);
}

class RecordDBPersistor implements RecordPersistor {} //for example

public interface Record { //all your data getters/setters }

class RecordImpl implements Record { //package visiblity
public RecordImpl(RecordPersistor rp){/*saves the rp object*/}
public persist(){ rp.persist(this);}
}

public RecordFactory {
Record createRecord(/*params, persist method*/){ return new
RecordImpl(new RecordDBPersistor());} //for example
------code------

You need a reason for the separation, rather than just "separating as much
as possible" without giving it much thought.

Record would appear to be some kind of value object. I don't think you
gain anything by having a separate Record interface and concrete
implementation if you are not planning on having alternative
implementations of the interface. My own thoughts on this are: if you
cannot come up with a better name for the concrete implementation of an
interface "Xyz" than "XyzImpl", then it's questionable whether the
separate interface is a good idea. Without the separate interface you can
drop the extra complication of using a factory class. Better to keep
things simple (you can always make them more complicated later if
requirements change).

This criticism doesn't apply to your RecordPersistor interface, which is a
better idea since, as your example suggests, you could provide alternative
persistence strategies. For example you could have an
XMLRecordPersistor. This, in fact, is one of the main advantages of this
approach. You can have your Record objects and persist them without the
Record class having to know anything about *how* it is persisted. You can
have one object and persist it multiple times using a different approach
each time. And this is where I might consider changing your
implementation. By passing the persistor to the constructor, you are
restricting each record object to only ever being persisted in one way.
This is probably sufficient, but it would be more flexible to pass the
Record object to the persistor (this is supported by the persist method of
your RecordPersister interface anyway). Instead, you are asking the
Record object to do this for you, which is coupling (rather than
separating) the record object and its persistence.

Dan.

 

[Stefan Ram]

// getters & setters follow

The following article deals with the question from a special
point of view:

http://www.javaworld.com/javaworld/jw-01-2004/jw-0102-toolbox_p.html

The relation to you question is, that the approach used for
"Listing 4. Building an HTML representation" in this article
can also be used to write the Employee to a data base.

The above article is based on:

http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox_p.html

 

[Daniel Dyer]

The first thing that occurs to me is that there is a practical limit to
the number of lines one wants in any given block of code. It's just a
maintenance issue. Once a chunk of code (whether {} block, method,
class or file) gets too big, it's just too tough to maintain.

So ideally, perhaps one could say that the persist() method really does
belong inside the Record class.

This is a valid perspective but for me the key point is that the Record
does not need to know how it is persisted. Moreover, what if you want to
persist it in more than one way? Say you have a persist() method that
writes the record to the database. Then somebody decides that it would be
a good idea for some records to be saved as XML under some circumstances.
Then you have to add a persistXML() method. Then maybe you need a record
to be written to the file system as a serialised object. So you add
another method. The result of this is that the Record class needs to know
exactly how to do each type of persistence and you've had to modify and
add complexity to the Record class for each one, even though the concept
of what a Record *is* has not changed. Not only that, you now have three
differently named methods. So the calling class that invokes the
persistence needs to know about each of the persistence methods provided
so that it can invoke each one.

If instead you have a RecordPersistor interface (like the one described in
one of the other posts in this thread) and have one implementation for
each type of persistence (DB, XML, serialisation), then each will have an
identically named method that can be invoked without any knowledge of
*how* the persistence will be achieved. So your code that saves a record
can just iterate through a list of persistors and call the persist method
on each (passing it the record to be saved). Now when you add a fourth
persistence implementation to the list of persistors, nothing has to
change. Neither the Record class itself nor the code that invokes the
persist methods has to know anything about the new persistence. It
doesn't matter whether it is uploading the record to an FTP server,
dumping it to the printer or doing something that nobody has yet thought
of. Whatever new strategy you add will be supported without requiring
anything else to change.

There used to be a general guideline for the maximum size of a given
Java file, class, or method. I thought it was in the coding
conventions. I can't find it. You might look around on your own, to
see what kind of heuristics you can find.

It should be as big as it needs to be, no bigger. Beware of artificial
limits applied arbitrarily.

Dan.

 

[Daniel Dyer]

This is good, and very general. I recognize at least some of the ideas
here. However, if the designer/coder doesn't understand why things are
being implemented this way, it may not buy him or her much. Can I ask
what design patterns are being used here? Any good links to a reference
explaining when (and when not) to use this technique?

The key pattern, the one represented by the RecordPersistor interface, is
the "Strategy Pattern" (Google has lots of links for this search string).

Dan.

 

[Zaph0d]

You need a reason for the separation, rather than just "separating as much
as possible" without giving it much thought.

Well, it was late, dark and mainly, I was lazy
"separating as much as possible" is actually a combiniation of several
reasons. Mainly, it's the knowing that in big projects, murphey will
strike where you're least prepared to handle him. Not "seperating", or
not using a factory/interface pattern, would leave you hanging for a
refactoring later, when the need arises. In small applications the
direct creation method usually suffice. This is due to the short life
cycle/small amount of data types involved. When you're starting to
handle big and very big applications, especially if it's in a
multi-developer environment, the original specification of the data
type is bound to change.
Another reason is testability. Using a factory class makes if much
easier to unit-test the package. From the inside, you can just use the
factory as a main installation point. Since it controls what goes into
the constructor, it makes a nice point of entry for the unit-test. From
the outside, the replacing the factory will allow you to mimic/mock the
record class (and persistors and all other related class) without
getting into reflection/aspectJ related mock strategies.
My final reason is murphey again, but from a debugging point of view.
Each class should handle it's responsibilities and nothing more. While
Record should be able to handle data type get/set, it shouldn't be able
to handle it's own creation. And just as an example, think of a record
getting created from numerous sources - DB, socket, xml, file... It's
true they should all be in the Persist classes. However, the record
should not be coupled to a specific persist class. Also, your code
should not be coupled to the persist classes. It is better, imho, that
the factory is coupled to them.

Ran Biron.

 

[ricky.clarkson]

The vague way of answering this question:

Is persist something the Record does, or something that you do to the
Record? If the latter then persist() does not belong in Record.

The more precise way:

Can you persist() a Record via its public interface? If so, then
persist() does not belong in Record.

Record looks very much like a C struct, or a Pascal record. Ignoring
the convention of getters and setters in Java, so as to not give the
illusion of object-orientation:

class Record
{
public int id;
public String name;
public String data;
}

persist() can be implemented by reading those 3 variables, so it is an
operation on a Record, not an operation *of* a Record.

Another way of putting it is in terms of version control.

Record is something that should evolve around the same speed as your
database schema. When you add a column to your database schema you
probably need to add a field to Record.

persist() will tend to evolve faster, or even have multiple
implementations. If persist() was in Record, this would be annoying
and make version control information less useful, because there would
be changes to Record that are unrelated to the database schema. The
data itself is not related to the persistence. The persistence uses
the data, the data doesn't use the persistence.

A ball does not know how I will throw it.

 

[Daniel Dyer]

Well, it was late, dark and mainly, I was lazy
"separating as much as possible" is actually a combiniation of several
reasons. Mainly, it's the knowing that in big projects, murphey will
strike where you're least prepared to handle him. Not "seperating", or
not using a factory/interface pattern, would leave you hanging for a
refactoring later, when the need arises.

I think there's something to be said for leaving that refactoring until
the need arises, since the need may never arise and therefore it's wasted
effort now.

I agree that it is often useful to have an interface so that a stub
implementation can be created for testing. But in the particular scenario
in this thread, I don't see a need. The Record class presumably does
nothing particularly complicated and can be used "as is" for unit
testing. You could advocate that all public methods should be declared in
an interface, such that you would have no concrete class that did not
implement at least one interface. However, I don't think that this line
of reasoning is valid for all situations, particularly for classes like
our hypothetical Record class which is presumably not much more than data
and a whole load of getters and setters.

In small applications the
direct creation method usually suffice. This is due to the short life
cycle/small amount of data types involved. When you're starting to
handle big and very big applications, especially if it's in a
multi-developer environment, the original specification of the data
type is bound to change.

I want to make it clear that I am not arguing against the use of
interfaces or factories. I'm just saying that they shouldn't be used
blindly for every class. I'm also not sure that I am following your
argument in the above paragraph. Are you talking about the situation in
which a new field is added to some data type class or the type of the
field changes, or something else? How will extracting the getters and
setters into an interface and using a factory help in this situation? You
will have to add new getters and setters to the interface just as you
would to the class, and factory method signatures will change just like
constructor signatures would. All you are gaining is another level of
indirection.

Another reason is testability. Using a factory class makes if much
easier to unit-test the package. From the inside, you can just use the
factory as a main installation point. Since it controls what goes into
the constructor, it makes a nice point of entry for the unit-test. From
the outside, the replacing the factory will allow you to mimic/mock the
record class (and persistors and all other related class) without
getting into reflection/aspectJ related mock strategies.

In the approach I was advocating, the Record class would have no knowledge
of persistence. It therefore would not need to be mocked. You could
still mock the persistors if required, since there would be an interface
for those.

My final reason is murphey again, but from a debugging point of view.
Each class should handle it's responsibilities and nothing more. While
Record should be able to handle data type get/set, it shouldn't be able
to handle it's own creation. And just as an example, think of a record
getting created from numerous sources - DB, socket, xml, file...

My main argument is against the seemingly unnecessary Record interface (in
this particular instance where there would appear to be no need for
alternative implementations). I concede there may be good reasons for
using a factory, if you do need to do something more complicated than
invoking a constructor (such as cloning or deserialisation). However, I'd
still defer adding the complexity until there is a need for it. In other
words, if at present the only way to create a Record object is to invoke a
constructor, then I'll just invoke the constructor, rather than creating a
factory that has a method that takes all the same arguments as the
constructor and merely delegates. If I write a factory for every concrete
class I write, that's more places for your friend Murphy to strike.

It's
true they should all be in the Persist classes. However, the record
should not be coupled to a specific persist class. Also, your code
should not be coupled to the persist classes. It is better, imho, that
the factory is coupled to them.

If I'm following you, I agree (about not coupling the Record to the
persistence). But for this to be the case, the Record class you posted
would have to lose its persist method and reference to the persistence
strategy. Whether you have another class that manages the invocation of
the persistence (both loading and saving) would depend on exactly how you
were planning on using the classes. It may be that you need something
more like the EntityManager in JavaEE 5 rather than a traditional factory
(which, to me at least, implies loading but not saving).

Dan.

 

[Zaph0d]

...

Your arguments are valid and I agree with them in princpicle. However,
I'm currently working in a multi-team project. Classes that need to be
refactored are a big headache, since some of them are used in multiple
places in the project. In this situation it's best to make your design
extensible as possible and "general" as possible. I agree this might
not be the case for small projects. However, I think it's good to learn
to code that way.

For small projects (one/two man projects) or projects with short life
cycle (short development time), this is, as you've said, an overkill.

 

[slantedwalker]

Thanks to you all. I got something out of every post. By the way I
agree with both Zaph0d and Daniel - save refactoring for when it needs
to be done. Ultimately Zaph0d's design seems to satisfy the Open/Closed
principle the best by pushing all future change into the factory class.
Overall this discussion really clarified the use of Utility classes for
me.

Regards,
Slantedwalker