This is a general dicussion point regarding the passing of Entity objects around the application and how best to code APIs which may modify the passed Entity.

Let's start with a typical simple example, say we have a Person entity with Name, DOB, Hobbies and CreatedDate...

We create a method which creates new instances of Person entities as follows:

public PersonEntity CreatePerson(string name, DateTime dateOfBirth)
{
    PersonEntity person = new PersonEntity();
    person.Name = "Joe Bloggs";
    person.DateOfBirth = "17-May-1971";
    person.Hobbies = "";
    person.CreatedDate = DateTime.Now;
    return person;
}

Now say we have another method which sets the person entity's Hobbies as follows:

public void SetHobbies(PersonEntity person, string hobbies)
{
    person.Hobbies = hobbies;
}

This kind of method would in reality set many more fields on the entity, but let keep it short for the example....

I read somewhere once that Best Practice for class library design states that you should not modify an object that is passed in. Instead you should Clone the object, modify the clone and return the Cloned version like:

public PersonEntity SetHobbies(PersonEntity person, string hobbies)
{
    PersonEntity modifiedPerson = (PersonEntity)person.Clone();
    modifiedPerson.Hobbies = hobbies;
    return modifiedPerson;
}

The caller would look something like:

person = SetHobbies(person, "Tennis,Chess,Beer");

Now Entities don't implement ICloneable (for good reason) but you can see that if they did (and if there were a low 'cost' of performing the clone) then this would be a very neat and reliable way of making the modifications to the object. After the caller is at the mercy of the SetHobbies method with regard to what exact modifications have been made inside the Entity.

So what then is the best way to implement the API for this method... We could:

public void SetHobbies(PersonEntity person, string hobbies)
{
    person.Hobbies = hobbies;
}

The above is bad in my opinion since it doesn't tell me anything about what is happening...

public PersonEntity SetHobbies(PersonEntity person, string hobbies)
{
    person.Hobbies = hobbies;
    return person;
}

The above is good for testing as test experts will tell you to always try to return a modified object and is good for readability since it 'implies' a new modified person entity is returned. Is doesn't tell me however whether the person entity I passed in is going to be modified...

public void SetHobbies(ref PersonEntity person, string hobbies)
{
    person.Hobbies = hobbies;
}

The above gives me a clue the person entity may be modified since it explicitly defines the object to be passed by reference. All object are actually passed by reference by default, but this just make a point of it...

mmm....

Feel free to comment!

Marcus

My first point is that, why should we copy an object just to modify it and pass it along? If we dont need a copy, dont make one.

My second point is that the day that a QC person / tester tells me how to write my code, is the day that I tell that person to pound sand. My point, it isnt a testers job to optimize and refactor code. Their purpose in life is to make sure that the test case works and meets the business / user requirement.

IMO, using the function to return the modified object is cleaner to read visually, be we as good developers should know that the void, has modified the reference, and the values are different. I am not saying that one is better than the other.

I have written the code may ways. For example, I will have a web user control that creates an entity. Then passes the entity to other web user controls, and the web user controls attach their data to the entity passed in. At the end of the operation, I save the original entity, and all of the data added by the user controls is saved.

I have also written the code using the function approach. I lean toward the void approach. But, I think that I lean to the void approach because I use voids for actions and I use functions more like properties.

So the word "Set" indicates action. So this would always be a void.

Let me also throw in my 2 cents.

I read somewhere once that Best Practice for class library design states that you should not modify an object that is passed in. Instead you should Clone the object, modify the clone and return the Cloned version like:

Must be a FX Cop thing.

I dont particularly like this idea.. As you are going to be leaking objects (memmory) all over the place, yeh and you might say that we are now using .net and no longer have to worry about those things - but the older languages taught us to clean up behind ourselfs and keep memmory usage in mind.

Doing these type of things are going to make your app very memmory hungry or slow...remember the garbage collector will have to kick in at some stage to free the memmory of the lost objects and that will slow your app down.

I have to agree with DevilDog creating one instance and sticking to it till the end sounds and works better.

Sometimes the requirement that FX Cop puts in place makes our life's very difficult.