views:

160

answers:

5

In my mind the ID field of a business object should be read-only (public get and private set) as by definition the ID will never change (as it uniquely identifies a record in the database).

This creates a problem when you create a new object (ID not set yet), save it in the database through a stored procedure for example which returns the newly created ID then how do you store it back in the object if the ID property is read-only?

Example:

Employee employee = new Employee();  
employee.FirstName="John";  
employee.LastName="Smith";  

EmployeeDAL.Save(employee);

How does the Save method (which actually connects to the database to save the new employee) update the EmployeeId property in the Employee object if this property is read-only (which should be as the EmployeeId will never ever change once it's created).

It looks like the Id should be writable by the DAL and read-only for the rest of the world. How do you implement this especially if the DAL classes and the Business object ones are in different assemblies?

I don't want to create a Save method in the Employee class as this class should have nothing to do with the database.

A: 

How about:

Employee employee = new Employee(EmployeeDAL.GetNextID());

This should also make your save code simpler.

DanDan
or..Employee employee = EmployeeDAL.NewEmployee();
Anwar Chandra
Yes, I prefer Andra's solution.
DanDan
With this solution the NewEmployee() method would create a new record in the database straightaway. The benefit is that you will have the EMployeeId but what if the user decides not to save the Employee in the end? You would have to go back to the database to delete the record which doesn't seem right.
Anthony
It would not be too bad. It may even simplify concurrent access to your database.
DanDan
+1  A: 

You can make your DAL method just return an updated object:

public class EmployeeDAL
{
    Employee EmployeeDAL.Save (Employee employee)
    {
        // Save employee
        // Get the newly generated ID
        // Recreate the object with the new ID and return it
    }
}

Alternatively, you can generate a new ID in code, instantiate an object with this ID then ask your DAL to save it.

If you wish that your object was updated during Save operation you will have to make this property public.

I personally like to create immutable objects, those you can only setup once by passing all the values into constructor. With this approach you would just create an object to be saved, then retrieve it back along with the assigned ID from the database and return it to the caller.

Developer Art
What if the object has 50 properties? You would create a new object, copy the value of all the properties over just so you can update the ID property. It would work but it doesn't seem right to me.Retrieving back from the database would work but it means you connect to the database twice: once to save the object and once to retrieve it (when you already have it in your client code). Again that doesn't seem right.
Anthony
If it has 50 properties, use something like http://www.codeplex.com/AutoMapper to map those from object A to object B
marc_s
@Anthony: That's true. Here is though one advantage: by retrieving again from the database you get exactly what got saved there - maybe a string was truncated or something else. If you made an error in code, you in application will believe you have now this set of data while in fact you don't. But again, these are various design approaches.
Developer Art
What would have really helped in this situation if we had a concept of "friends" in .NET as wehad it in C++. Then you problem would have been easily solvable.
Developer Art
@Anthony: instead of connecting twice, you could have your stored procedure give it back to you directly
Juri
@New in town: 'internal' is the equivalent of friend. By making the ID 'public get' and 'internal set' then the DAL (assuming it in the same assembly) can update it but it would remain read-only for the rest of the world. That's probably a good solution.
Anthony
@Anthony: I'm sceptical on this one. We just want to grant access to one particular class, not to all residing in the same assembly. But at the end of the day, you decide how you would like it to be.
Developer Art
+1  A: 

You could make your setter for the ID allow set only if it has not already been set before:

public class Employee
{
 private int? m_ID;

 public int? ID
 {
  get { return m_ID; }
  set
  {
   if (m_ID.HasValue())
    throw ...
   m_ID = value;
  }
 }
}

Alternatively, I think some frameworks support this type of functionality (for example, I think NHibernate will allow you to have a private setter on an ID field).

Chris Shaffer
I like this idea but you will only realise you can't set the ID if it has already been set at runtime. It would be nice that at compile time the property is read-only (except for the DAL)
Anthony
+1  A: 

Another possible solution is to declare Employee as :-

public class Employee
{
    public int Id { get; internal set; }
}

... provided that the Employee and DAL classes are in the same assembly

I don't claim to like it but I have used it.

MikeJ-UK
and if they're not in the same assembly, it's possible to use: [assembly: InternalsVisibleTo("DAL_Assembly")]
Anthony
+1  A: 

How about only allowing code outside the DAL to refer to the object through an interface which doesn't supply a setter for the Id field (and any other immutable fields):

public interface IEmployee
{
    Int32 Id {get;}
    String Name {get;set;}
    // ... and so on ...
}

public class Employee: IEmployee
{
    Int32 Id {get;set;}
    String Name {get;set;}
}

The DAL can set it as required, but the consuming code can't.

Dave W
As your Employee class is public (as it should be as the DAL which should be in a separate assembly needs to access it), nothing prevents the consuming code from creating an instance of the Employee class (and therefore from modifying the ID).
Anthony