Java Forum / General / September 2006
Design Question
slantedwalker@gmail.com - 02 Sep 2006 17:01 GMT 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 - 02 Sep 2006 17:34 GMT > 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 [quoted text clipped - 3 lines] > 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.
-- Mike W
Mark Space - 02 Sep 2006 18:50 GMT > Hello, > [quoted text clipped - 6 lines] > 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 - 02 Sep 2006 19:48 GMT 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 - 02 Sep 2006 20:37 GMT > 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 [quoted text clipped - 8 lines] > 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.
> ------code------ Daniel Dyer - 02 Sep 2006 21:26 GMT >> I would choose a different, though more chumbersome, approch, which is >> based on seperating as much as possible. [quoted text clipped - 4 lines] > 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.
 Signature Daniel Dyer http://www.dandyer.co.uk
Daniel Dyer - 02 Sep 2006 20:44 GMT > I would choose a different, though more chumbersome, approch, which is > based on seperating as much as possible. [quoted text clipped - 16 lines] > 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.
 Signature Daniel Dyer http://www.dandyer.co.uk
Zaph0d - 03 Sep 2006 07:08 GMT > 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.
Daniel Dyer - 04 Sep 2006 20:25 GMT >> You need a reason for the separation, rather than just "separating as >> much [quoted text clipped - 6 lines] > 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 [quoted text clipped - 3 lines] > 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.
 Signature Daniel Dyer http://www.dandyer.co.uk
Zaph0d - 05 Sep 2006 07:10 GMT > ... 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@gmail.com - 05 Sep 2006 18:40 GMT 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
> > ... > [quoted text clipped - 8 lines] > For small projects (one/two man projects) or projects with short life > cycle (short development time), this is, as you've said, an overkill. Daniel Dyer - 02 Sep 2006 21:16 GMT > 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 [quoted text clipped - 3 lines] > 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.
 Signature Daniel Dyer http://www.dandyer.co.uk
Stefan Ram - 02 Sep 2006 20:50 GMT > // 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
ricky.clarkson@gmail.com - 04 Sep 2006 11:47 GMT 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.
> Hello, > [quoted text clipped - 42 lines] > shown above. What would be the advatage of using a Util class rather > than encapsulating the functionality in Record?
Free MagazinesGet these publications absolutely FREE for up to 12 months. There are no hidden fees and no obligation. Simply choose a title, complete the application form and submit it. Read more ...
|
|
|