views:

502

answers:

7

NOTE: Sorry for the long question!

I'm trying to understand some key areas behind object orientation and I couldn't decide one way or another about my particular question.

Let's say I have an object full of lovely data. Class bob.

Bob myBob = new Bob("This string is data");

Let's say I want to save the contents of myBob to an xml file (bob.xml)

Should I have an object act on bob to write the contents out, or should I have myBob do this?

Case 1: Act on object

Writer myWriter = new Writer(myBob, "C:\\bob.xml");

Case 2: Save method

myBob.Save("C:\\bob.xml");

Some people are siding with option one as it means if the code for writing files is changed, it doesn't need to updated across every Save method; promoting code reuse I suppose. My problem with this is getting all the data out of objects which may have private data with no accessor.

The case for option two is that the method only acts on the data held by the object and that's the way it should be. No interference from other objects.

Or is the answer to my question one of those "case dependent" issues? If so, how do you know when one method is prefered over the other?

+12  A: 

I would make Bob know how to serialize itself since it has private data. Another object (such as your Writer) would take that and put it on disk. Bob knows how to deal with its data best but it need not care about how or where that is stored. Your Writer knows how to save data best but it need not care about how that data is created.

Colin Burnett
I like it. I didn't think to use serialize as a way to handle my question.
Andrew Weir
+1. When you consider information hiding and encapsulation, the only object that could (in general) serialize Bob is Bob. Generalize "saving" as writing to a stream to abstract away the coupling to a file as advised.
Daniel Paull
+20  A: 

The correct approach, in general, is your Case 1. This maintains a single responsibility for the class (whatever it does) without coupling it to a specific persistence mechanism (a disk).

You're looking at a specific case of a more generalized problem: Serialization. It's good and OK for an object to have some means to indicate how it should be serialized-- it's the only entity that knows what's necessary to deserialize it, after all. But if you make the object save itself to disk, you've tightly coupled that object to a specific implementation.

Instead, consider creating an interface that a generalized "writer" can use to "serialize" the object to whatever that writer serializes to. This way, you'll be able to serialize to disk, to the network, to memory, to whatever you actually need to serialize to. :)

Greg D
Your answer is in the same vein as Colin's and it's a great answer. Thank you for your help.
Andrew Weir
Well, case 2 might as well be correct, it really depends... There is no clear yes or no in OO-design. For example, you might also think scenarios where it's perfectly fine that your object just implements an IWriter interface.
0xA3
@divo: There are always exceptions, that's the nature of design. It's not something that should have to be explicitly called out. (And it's also why I qualified my statement with, "in general".)
Greg D
@Divo - to be fair, both Greg and Colin's answers gave me enough information to start implementing something new. I can't give both answers their full merit sadly :(
Andrew Weir
What is the recommended strategy to deserialize the object? Would it be ok then to pass in say a BinaryReader object and have the class deserialize itself?
Joe Doyle
@joe http://www.switchonthecode.com/tutorials/csharp-tutorial-serialize-objects-to-a-file this should help you; as it helped me. I just want to say thanks again to Greg and Colin. That's a great solution to the problem.
Andrew Weir
+2  A: 

I used to prefer option 2; however, as I have started really trying to understand and model the domains I am working on, I prefer option 1.

Imagine if your modelling Vehicles. Why would a vehicle know how to persist itself? It might know how to move, how to start and how to stop, but what is Save within the context of a vehicle.

JoshBerke
I think it depends on what you are modeling. Imagine if you're modeling text documents, which are typically persisted in some form. Then it would be natural to have a Save method (although you still may want to go with option 1).
0xA3
A: 

I think the correct approach is the Case 1, but your class may be defined this way to take advantage of both approaches:

class Bob {

    IWriter _myWriter = null;

    public Bob(){
        // This instance could be injected or you may use a factory
        // Note the initialization logic is here and not in Save method
        _myWriter = new Writer("c://bob.xml")
    }

    //...
    public void Save(){

        _myWriter.Write(this);    

    }
    // Or...
    public void Save(string where){

        _myWriter.Write(this, where);

    }
    //...
}

This could be easily modified to put the writing logic and initialization in a base class so Bob class is even cleaner and independent of the persistence.

Gerardo Contijoch
Why was this downvoted?
0xA3
Perhaps the typo in "c://bob.xml" is enough reason to downvote this answer... :P
Gerardo Contijoch
I didn't downvote, but there are many problems here: each Bob has the added cost of a Writer reference, each Bob has only one Writer allocated at construction time when the invoking code might want to write it to multiple Writers or no Writer, and what should be a nice pure domain object is unnecessarily interlinked with IO mechanisms. The approach where each Bob knows how to serialize itself to and from one (or perhaps more) String formats allows the IO to be defined elsewhere, for much greater modularity.
Jim Ferrans
@Gerardo: Even if C://bob.xml looks a little strange it works.
0xA3
@Jim: The added cost of a Writer reference would be evened out by the advantage that the Strategy pattern might give you. See http://en.wikipedia.org/wiki/Strategy_pattern. However, it might be better to pass the Writer in as an argument to the constructor (as mentioned in the source code). But it's just a simple sample and the basic idea is illustrated well enough.
0xA3
@divo: I was just kidding with my comment :P@Jim: I totally agree with you, but that's why I left a comment in the code indicating that the Writer may be injected (the same writer for all Bob instances if you like). Besides, that code (which is only an example) depends on IWriter (an interface). You may create a CompositeWriter that consumes several other writers. If you want to solve the reference issue, you may use a static property somewhere (in a Context class for instance) similar to CurrentContext prop in HttpContext class.
Gerardo Contijoch
BTW, the different approaches I mention in my previous comment don't deal with the fact that the 'pure' domain object wouldn't be that pure anymore...
Gerardo Contijoch
+3  A: 

This is an example of where the Strategy Design Pattern could be used. You myBob object could have an instance of the class that will write it out. You may want the writer to implement and interface or derive from an abstract class so that the save routine can be easily changed. Today you are saving to xml, but you might need to eventually persist the object to a database as well. This pattern will allow you to change the save routine easily. You would even have the option to change how you save at runtime.

Guster_Q
Up vote for you!!! Good call on Strategy Pattern as it allows for extension without modification.
David Robbins
A: 

Do this:

public interface Writable {
    public void Save(Writer w);
}

public interface Writer {
    public void WriteTag(String tag, String cdata);
}

public class Bob : Writable {
    private String ssn = "123-23-1234";
    public void Save(Writer w) {
        w.WriteTag("ssn", ssn);
    }
}

public class XmlWriter : Writer {
    public XmlWriter(Sting filename) {...}
    public void WriteTag(String tag, Sting cdata) {...}
}

Obviously this isn't a complete solution but you should get the general idea.

+2  A: 

One other method is to use the visitor pattern. Have your object contain an Accept method that goes through the members you want to process/serialize, and have a visitor be your serializer. Whenever you update or change your serialization (plain text to xml to binary to whatever), you don't need to update the object.

We've had good experiences at work doing it this way. It's pretty powerful.

Steve the Plant