CSLA .NET 3.5 enhancement - property declarations

RockfordLhotka posted on Monday, November 26, 2007

I mentioned this in a recent blog post, and it generated some discussion, so I want to post it here because my blog isn't really a good discussion forum.

In CSLA .NET 3.5 I am planning to simplify the way properties are declared. In most of my business objects the majority of lines of code are in property declarations. While they are almost all the same, I have always thought they were too long.

Additionally, the fact that the System.Diagnostics trick to get the property name has become increasingly problematic, and I am marking the overloads of CanReadProperty(), CanWriteProperty() and PropertyHasChanged() that use the trick as obsolete in 3.5 - so you'll get compiler warnings/errors when you try to use them.

So I want to do two things: simplify/reduce the code and require explicit property names (but without requiring the use of string literals).

As a side-issue, I want to make it easier to manage property friendly names in a manner such that they are available to the object, to the UI (optionally) and are localizable via string resources in your business assembly.

The result is a property (in class Customer) like this:

Private Shared NameProperty As PropertyInfo(Of String, Customer) = _
  RegisterProperty("Name", "Customer name")
Private _name As String = NameProperty.DefaultValue
Public Property Name() As String
  Get
    Return GetProperty(Of String)(NameProperty, _name)
  End Get
  Set(ByVal value As String)
    SetProperty(Of String)(NameProperty, _name, value)
  End Set
End Property

GetProperty() and SetProperty() take care of authoriziation and calling PropertyHasChanged(), reducing all the old-style code down to these few lines. The Shared/static NameProperty field (an idea borrowed loosely from Microsoft's DependencyProperty scheme) minimizes the use of string literals throughout the code.

This scheme saves 6 lines of code and adds 1 - so there's a net savings of 5 lines of code per property. Over 30% code savings per property. I think that's awesome! There's no room to trim anything at this point (well, none I see at the moment anyway).

Notice that the field is still declared as an instance field in the object. I could have managed the "fields" in a Dictionary<string, object> in the base class, but that'd incur boxing/unboxing overhead on each use of the "field" and that seems too expensive to me. This technique maintains almost the same exact performance of the existing model, but improves maintainability a lot.

Also in class Customer:

Protected Overrides Sub AddBusinessRules()
  ValidationRules.AddRule(AddressOf MyRule, NameProperty)
End Sub

Protected Overrides Sub AddAuthorizationRules()
  AuthorizationRules.AllowWrite(NameProperty, "Supervisor")
End Sub

Again, NameProperty minimizes the use of string literals for both the property name and the property's friendly name. The new AddRule() overload uses the values from NameProperty to populate the fields in the RuleArgs object.

Also, note that the Shared/static NameProperty field could be Public in scope, thus providing the UI with the friendly property name as well. And notice that the friendly name is set in executed code (not attributes) and so it can be set from a resource string or from a database table or whatever.

The primary comment on the blog post was a dislike of this Shared/static field at all. And I don't plan to eliminate the exisitng overloads that accept a normal string literal. So if you don't like the Shared/static PropertyInfo field, you don't need to use it - but in that case it is up to you to figure out how to avoid scattering string literals throughout your code.

The other two primary schemes are:

1. Declare a constant for each property - which gets you the property name, but not the friendly name
2. Declare an enum with all your property names - which again gets you the property name, but not the friendly names

If anyone has a better overall solution than the Shared/static field concept, I'm open to ideas. But any solution must:

• Minimize the use of string literals for the property name
• Minimize the use of string literals for the property friendly name
• Allow for localization of the property friendly name
• Not have a negative impact on performance

Patrick replied on Monday, November 26, 2007

William replied on Tuesday, November 27, 2007

ajj3085 replied on Tuesday, November 27, 2007

Bob Matthew replied on Tuesday, November 27, 2007

How about using attributes?  Would something like this work?

[CslaField] // optionally specify various parameters to dictate friendly name, CanRead/CanWrite, etc...
private string _name;

public string Name {
    get { return GetProperty<string>(_name); }
    set { SetProperty<string>(_name, value); }
}

Then your Get/SetProperty would simply read the CslaField attribute to determine authorization rules, dependency rules, update notifications, etc...

RockfordLhotka replied on Tuesday, November 27, 2007

Bob Matthew:

How about using attributes?  Would something like this work?

[CslaField] // optionally specify various parameters to dictate friendly name, CanRead/CanWrite, etc...
private string _name;

That's basically what William is suggesting as well.

The problem with attributes is that they can't be localized. Their values are set at compile time, not runtime.

Also, one of the slowest (I've heard THE slowest) reflection operation is reading custom attributes. So reading them on every property get/set would be a bad thing. It would probably be possible to do some sort of caching of the values in a Dictionary or something, so as to minimize the reflection overhead, but then you'd have at least the Dictionary lookup on every get/set.

Finally, this doesn't remove the string literals used in AddBusinessRules() or AddAuthorizationRules(). Though I suppose you could do something like:

ValidationRules.AddRule(MyRule, GetPropertyName(_name));

where GetPropertyName() did the reflection call to get the attribute value. But you want the friendly name too, so it would be:

ValidationRules.AddRule(MyRule, GetPropertyName(_name), GetFriendlyName(_name));

or something along that line.

And even with all this, there's still no way for the UI to gain access to the friendly name, because it can't access the private fields.

ajj3085 replied on Tuesday, November 27, 2007

honus replied on Tuesday, November 27, 2007

Thinking about the set method, I often found myself doing something like the following:

Public Property childObject() as childObjectType
   Get
      CanReadProperty(True)
      Return mChildObject
   End Get
   Set(ByVal value As childObjectType)
      CanWriteProperty(True)
      If mChildObject.Equals(Nothing) OrElse Not mChildObject.Equals(value) Then
         mChildObject = value
      End If
   End Set
End Property

If I did not do this check, and the object had not been instantiated, I would get an exception (NullReference??).  This would not be an issue for simple properties like strings and integers, but as business objects become nested within other objects, it can become an issue. 

For my own purposes, I have created a shared set method (using Reflection, thanks to Rocky for introducing it to me ) that all of my properties now use, and this is one of the checks included.  This check might be a nice inclusion in your SetProperty.

RockfordLhotka replied on Tuesday, November 27, 2007

jtgooding replied on Thursday, November 29, 2007

This one is fairly long, but I think I have it stripped down to the minimum I can. 

First attributes as much as I love them I don't think are the way to go, I use them extensively on enums to get 'friendly' names, but unless you do caching the reflection hits can add up over time.

Secondly, true enums are NOT the way to go, basically because it provides no way to inherit an enum or do partial enums for codegen fields versus custom fields.

I use my own home brew enum class which then gives me the ability to write partial classes for codegen and also allows for inheritence of business classes.

I've stripped this down to the bare minimum, and specialized it towards my field info class (used by my dynamic SQL generator.

[Serializable]
public class DBFieldInfo
{
   public DBFieldInfo(string propertyName, PersistTypes persist, SqlDbType dbFieldType, string dbFieldName, int maxLength, bool isPrimaryKey, bool isIdentity)
{}

public DBFieldInfo(string propertyName, PersistTypes persist, SqlDbType dbFieldType, string dbFieldName, int maxLength, bool isPrimaryKey, bool isIdentity, EncryptionType encryptionMode)
{}

public DBFieldInfo(string propertyName, PersistTypes persist, SqlDbType dbFieldType, string dbFieldName, int maxLength, bool isPrimaryKey, bool isIdentity, EncryptionType encryptionMode, string certificate, string encryptionKey)
{}

// public void SetEncryptionType(EncryptionType encryptionType);
// public void SetMaxLength(int Length);
}

public struct EnumEntry
{
   private int _Ordinal;
   private string _Name;
   public int Ordinal
   {
      get { return _Ordinal; }
      set {_Ordinal = value; }
   }

   public string Name
   {
      get { return _Name; }
      set { _Name = value; }
   }

   public EnumEntry(int ordinal, string name)
   {
      _Ordinal = ordinal;
      _Name = name;
   }

   public int ToOrdinal()
   {
      return Ordinal;
   }

   public override string ToString()
   {
      return Name;
   }
}

public class EnumList : Dictionary<EnumEntry, DBFieldInfo>
{}

abstract class EnumBase<T>
{
   private static SortedDictionary<string, EnumList> _EnumCollections = new SortedDictionary<string, EnumList>();

   public static EnumEntry CreateEntry(string name, DBFieldInfo fieldInfo)
   {
      string EnumType = typeof(T).ToString();
      CreateCollection(EnumType);
      int Ordinal = GetNextOrdinal(EnumType);
      TestExists(EnumType, Ordinal, name);
      EnumEntry NewEnumEntry = new EnumEntry(Ordinal, name);
      _EnumCollections[EnumType].Add(NewEnumEntry, fieldInfo);
      return NewEnumEntry;
   }

   public static EnumEntry CreateEntry(string name, int ordinal, DBFieldInfo fieldInfo)
   {
      string EnumType = typeof(T).ToString();
      CreateCollection(EnumType);
      TestExists(EnumType, ordinal, name);
      EnumEntry NewEnumEntry = new EnumEntry(ordinal, name);
      _EnumCollections[EnumType].Add(NewEnumEntry, fieldInfo);
      return NewEnumEntry;
   }

   private static void CreateCollection(string enumType)
   {
      if (!_EnumCollections.ContainsKey(enumType))
         _EnumCollections.Add(enumType, new EnumList());
   }

   private static int GetNextOrdinal(string enumType)
   {
         int BiggestKey = -1;
         foreach (EnumEntry Key in _EnumCollections[enumType].Keys)
         {
            if (Key.Ordinal > BiggestKey)
               BiggestKey = Key.Ordinal;
         }
         return ++BiggestKey;
      }

   private static void TestExists(string enumType, int ordinal, string name)
   {
      foreach (EnumEntry Key in _EnumCollections[enumType].Keys)
      {
         if (Key.Ordinal == ordinal)
            throw new ApplicationException(string.Format("Ordinal [{0}] already exists in EnumBase", ordinal));
         if (string.Compare(Key.Name,name, true) == 0)
            throw new ApplicationException(string.Format("Name [{0}] already exists in EnumBase", name));
      }
   }
}

And here is a sample using it in the various ways.

public void TestEnums()
{
   Console.WriteLine("RealEnum.x  " + RealEnum.x);
   Console.WriteLine("MyEnum.Internal " + MyEnum.Internal);
   Console.WriteLine("MyEnum.Internal.ToOrdinal() " + MyEnum.Internal.ToOrdinal());
   Console.WriteLine("MyEnum.External.ToOrdinal() " + MyEnum.External.ToOrdinal());
   Console.WriteLine("MyEnum.Unknown.ToOrdinal() " + MyEnum.Unknown.ToOrdinal());
   Console.WriteLine("MyEnum.Custom.ToOrdinal() " + MyEnum.Custom.ToOrdinal());
   Console.WriteLine("MyEnum3.XXX.ToOrdinal() " + MyEnum3.XXX.ToOrdinal());
   Console.WriteLine("MyEnum3.YYY.ToOrdinal() " + MyEnum3.YYY.ToOrdinal());
   Console.WriteLine("MyEnum3.ZZZ.ToOrdinal() " + MyEnum3.ZZZ.ToOrdinal());
   Console.WriteLine("MyEnum3.ZZA.ToOrdinal() " + MyEnum3.ZZA.ToOrdinal());
   Console.WriteLine("MyEnum4.Internal.ToOrdinal() " + MyEnum4.Internal.ToOrdinal());
   Console.WriteLine("MyEnum4.ZZAA.ToOrdinal() " + MyEnum4.ZZAA.ToOrdinal());
}

private enum RealEnum
{
   x = 1,
   y = 2,
   Z = 3
}

//Note that when using partial classes it is recommended that  you use the overload that takes an ordinal to enforce numbering if using the enums as a true enum to guarantee ordering consistency in this sample i didn't since I'm name driven not ordinal driven.
partial class MyEnum : EnumBase<MyEnum>
{
   public static EnumEntry Internal = MyEnum.CreateEntry("Internal", new DBFieldInfo("Internal", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
   public static EnumEntry External = MyEnum.CreateEntry("External", new DBFieldInfo("External", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
   public static EnumEntry Unknown = MyEnum.CreateEntry("Unknown", new DBFieldInfo("Unknown", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
}

partial class MyEnum
{
   public static EnumEntry Custom = MyEnum.CreateEntry("Custom", new DBFieldInfo("Custom", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
}

class MyEnum3 : EnumBase<MyEnum3>
{
   public static EnumEntry XXX = MyEnum3.CreateEntry("XXX", new DBFieldInfo("XXX", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
   public static EnumEntry YYY = MyEnum3.CreateEntry("YYY", 11, new DBFieldInfo("YYY", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
   public static EnumEntry ZZZ = MyEnum3.CreateEntry("ZZZ", new DBFieldInfo("ZZZ", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
   public static EnumEntry ZZA = MyEnum3.CreateEntry("ZZA", new DBFieldInfo("ZZA", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
}

class MyEnum4 : MyEnum
{
   public static EnumEntry ZZAA = MyEnum3.CreateEntry("ZZAA", new DBFieldInfo("ZZAA", SHIP.Framework4.PersistTypes.Database, SqlDbType.VarChar, "Internal", 32, false, false));
}

RockfordLhotka replied on Thursday, November 29, 2007

jtgooding:

I use my own home brew enum class which then gives me the ability to write partial classes for codegen and also allows for inheritence of business classes.

That is very interesting, thank you for sharing!

I don't think your approach is all that different from what I'm doing, though my approach wouldn't allow for externalizing the PropertyInfo objects into an object outside the business object itself.

But you have me thinking.

The RegisterProperty current requires the containing type (business object type) as a generic parameter. That parameter is unused (I kept the concept from DependencyProperty), and if I removed that parameter then a PropertyInfo would be effectively decoupled from any specific business object type.

Also, I haven't yet posted on a related concept, that I think is at least as important: a ChildManager.

In my current vision, child properties would be similar, but slightly different from, regular properties. The reason is that child references really will be managed by CSLA. So you won't maintain your own fields to reference a child - the reference will be managed by CSLA.

There'll be a RegisterChild() method that will require the containing type parameter, because it will use it to index into a dictionary of dictionaries. The result is still a PropertyInfo object - but this one can't be decoupled from the containing business object type.

The reason for this ChildManager thing is that it allows CSLA to better automate things like the parent reference, cascading PropertyChanged events up the chain, etc. It also means you no longer need to override IsValid and IsDirty in the normal case. In short, it saves a lot of code in parent objects and reduces the potential for related bugs.

But I already don't like the inconsistencies between "normal properties" and "child properties" in what I'm doing, and would really prefer to bring them closer together. This is one of the motivations for leaving the unused type parameter in RegisterProperty() - to keep closer parity with RegisterChild(), in the hopes that as this evolves I can bring them closer to parity.

One solution would be to bite the bullet and have CSLA manage all field values - like DependencyObject does with DependencyProperty fields. That would allow me to provide complete parity between these concepts, but would incur boxing/unboxing on each "field" use.

And that would entirely eliminate the idea of externalizing PropertyInfo objects from a business object implementation.

Hmm. Unless we flip it around. Suppose it were like this:

private static PropertyInfo<string> NameProperty =
  new PropertyInfo<string>("Name", "Customer name");
private static PropertyInfo<Addresses> AddressesProperty =
  new PropertyInfo<Addresses>("Addresses");

static Customer()
{
  RegisterProperty<Customer>(NameProperty, PropertyValue.Field);
  RegisterProperty<Customer>(AddressesProperty, PropertyValue.Managed);
}

private string _name = NameProperty.DefaultValue;
public string Name
{
  get { return GetProperty<string>(NameProperty, _name);
  set { SetProperty<string>(NameProperty, _name, value);
}

public Addresses Addresses
{
  get { return GetProperty<Addresses>(AddressesProperty);
}

This might work. This way the PropertyInfo instances are decoupled, and could potentially be externalized into some other class. Also, it isn't sealed, so you could extend it with extra stuff that you might want.

The two different overloads for GetProperty() would allow distinguishing between field-based and managed (child) values.

jtgooding replied on Thursday, November 29, 2007

Yeah it isn't light years different, I like the enum style coding my enum class gives vs the fields approach, it provides a more logical intellisense approach, also having the persistence data in a centralized static collection reduces footprint given we can have a lot of managers in memory at times, and I can gain access to 'sub managers' for my dynamic SQL generator.

I've already done the changes you are looking to do in my own customized version of CSLA, I'm hoping the solution you end up with will mesh fairly well and be extensible enough to include all of my own custom metadata as well to bring me more back in line with yours than push me farther away.

Currently the biggest stuff I've done is:
Persistence metadata
Dynamic SQL generator
Context sensitive rules (i.e. create/insert/update/delete rules collections)
Realtime vs Save time rules (this is big for us, we have rules that are very SQL intensive and we want to only check them at save time)
Reflection caching system (I sent you an early version of this awhile ago)
AsyncBindingList (An almost feature complete version of list that is thread safe.)
All Criteria objects must inherit from IUniqueCriteria (We ran into developers passing an int instead of a Criteria class and due to the parameters being type object would compile and create runtime errors, so we created this flag to put on all criteria classes to catch it at compile time)
Some additional CommonRules (StringExactLength, NotEqualValue, MustEqualValue)

SonOfPirate replied on Friday, November 30, 2007

Just a quick FYI and comments:

We've been using centralized GetProperty/SetProperty methods for a couple of years now with great success.  Glad to see you're going the same route.  It certainly saves on the code, makes the code-gen templates a bit smaller and ramping up a new developer easier.

With that, we have handled the string literals issue with resource files.  I don't think I saw this option mentioned in any of the posts and am not sure why.  But, it allows us to strongly type the reference throughout our code while eliminating string literals, gives us global changes (for type-o's, etc) and has localization support.

Finally, we also have several custom attributes that are based on the "built-in" classes adding localization support.  So, it is certainly possible to have localization with attributes.  You are required to create your own and I am in complete agreement with Rocky that the performance overhead far outweighs the benefits of using attributes in many cases, especially for behavior such as what's being discussed here.

Just my two cents...

Jimbo replied on Thursday, December 06, 2007

tmg4340 replied on Thursday, December 06, 2007

From Rocky's first post:

Additionally, the fact that the System.Diagnostics trick to get the property name has become increasingly problematic, and I am marking the overloads of CanReadProperty(), CanWriteProperty() and PropertyHasChanged() that use the trick as obsolete in 3.5 - so you'll get compiler warnings/errors when you try to use them.

Basically, in the 3.5 version, you won't use them anymore.

EDIT: Should probably provide some clarification on that.  Since the NoInlining attribute was used to make the System.Diagnostics code work properly, you wouldn't need it for 3.5.  I don't expect it would hurt if you left it in, but you don't need it anymore.  Sorry...

HTH

RockfordLhotka replied on Thursday, December 06, 2007

Here's the latest syntax for what I'm doing - looking for comments.

Option 1 - private backing field

private static PropertyInfo<string> NameProperty =
  new PropertyInfo<string>("Name", "Customer name");
private string _name = NameProperty.DefaultValue;
public string Name
{
  get { return GetProperty<string>(NameProperty, _name); }
  set { SetProperty<string>(NameProperty, _name, value); }
}

Option 2 - managed (by BusinessBase ) backing field

private static PropertyInfo<string> NameProperty =
  new PropertyInfo<string>("Name", "Customer name");
public string Name
{
  get { return GetProperty<string>(NameProperty); }
  set { SetProperty<string>(NameProperty, value); }
}

Variation for SmartDate

private static PropertyInfo<SmartDate> HireDateProperty =
  new PropertyInfo<SmartDate>("HireDate", "Hire date");
public string HireDate
{
  get { return GetProperty<string, SmartDate>(HireDateProperty); }
  set { SetProperty<string, SmartDate>(HireDateProperty, value); }
}

Variation for child object (managed field)

private static PropertyInfo<LineItems> LineItemsProperty =
  new PropertyInfo<LineItems>("LineItems", "Line items");
public string LineItems
{
  get
  {
    if (!PropertyManager.FieldExists(LineItemsProperty))
      SetProperty<LineItems>(LineItemsProperty, LineItems.NewList());
    return GetProperty<LineItems>(LineItemsProperty);
  }
}

Variation I am considering for reflection-based loading of property metadata

private static PropertyInfo<string> NameProperty =
  new PropertyInfo<string>(typeof(Customer), "Name");

The idea here is that this constructor overload would reflect against the business object to get the Name property, and would then read an attribute to get the display name for the property, which would be used as the friendly name. I'd also call a virtual protected method like OnInitializing() so subclasses would have an opportunity to initialize themselves as well (using the System.Reflection.PropertyInfo object I'd have already retrieved).

ajj3085 replied on Friday, December 07, 2007

RockfordLhotka replied on Friday, December 07, 2007

Patrick replied on Friday, December 07, 2007

JoeFallon1 replied on Friday, December 07, 2007

I really like Patrick's suggestion a lot.

If we are going to manage this, then let's go all the way.

Patrick's suggestion opens up all sorts of possibilties. Dynamic SQL being one of them. Concurrency checking is another. I am sure there are more.

Joe

RockfordLhotka replied on Friday, December 07, 2007

JoeFallon1:

I really like Patrick's suggestion a lot.

If we are going to manage this, then let's go all the way.

Patrick's suggestion opens up all sorts of possibilties. Dynamic SQL being one of them. Concurrency checking is another. I am sure there are more.

Seriously? That's a big (HUGE) breaking change if I really pursued it - because things like IsDirty would change, etc.

Would you go the rest of the way? Elevate the "property" concept to a first-class citizen?

What I mean by that is that some things, like validation and authorization rules, are really property concepts, not object concepts. So AddBusinessRules() should exist at the property level, not the object level - at least for associating rules to properties. You can envision that cross-property dependencies are still object concepts.

I'm not entirely sure what this would look like, but it would be interesting because it would allow you to define a property and then use that property in different objects.

I thought about this long and hard, and decided the break was too big - but heck, let's discuss it briefly anyway.

You'd define a property like this:

Public Class NameProperty
  Inherits PropertyBase(Of String)

  Friend Sub New()
    MyBase.New("Name", "Customer name")
  End Sub

  Protected Overrides Sub AddBusinessRules()
    ' associate rules here
  End Sub

  Protected Overrides Sub AddAuthorizationRules()
    ' allow/deny read/write here
  End Sub
End Class

Then in your business class you'd use it:

Public Class Customer
  Inherits BusinessBase(Of Customer)

  Private Shared NameProperty As New NameProperty
  Public Property Name() As String
    Get
      Return GetProperty(Of String)(NameProperty)
    End Get
    Set(ByVal value As String)
      SetProperty(Of String)(NameProperty, value)
    End Set
  End Property

  Protected Overrides Sub AddBusinessRules()
    ' associate dependent properties here
  End Sub
End Class

The important thing is that you'd be able to use NameProperty in some other business class if that class requires a customer name property, but the business/authorization rules for the property would be in one location.

GeorgeG replied on Monday, December 10, 2007

Another yes vote for Patrick's and JoeFallon1's suggestion for IsDirty and even original values at field level.

It sounds pretty interesting for properties being first class citizen. I do like the fact that we will be able to inherit code. Will we be able to use composition of property classes ie
Class Address
 Address
  City
  Zip
  AddBusinessRules()

Class Customer
 address=new Address
 
 Public Property City() As String
     Get
        Return GetProperty(Of String)(address.City)
     End Get
     Set(ByVal value As String)
        SetProperty(Of String)(City, address.City)
     End Set
 End Property
Thanks

ajj3085 replied on Monday, December 10, 2007

Patrick replied on Monday, December 10, 2007

ajj3085:

I don't have a lot of code I'm re-using right now, and all these objects would add a huge amount of overhead.  My application already sends enough data across the wire.. all these objects (which need to be passed when remoting even if the "property" is not dirty) would kill performance I'd think.

ajj3085 replied on Monday, December 10, 2007

Patrick:

Where FieldData in most cases is just made up of
    private T _CurrentValue;
    private bool _isDirty = false;

I wouldn't think that this adds a lot to the size of the serialized object.
As well all the PropertyInfo fields are static and wouldn't be serialized.

Blarm replied on Thursday, December 13, 2007

I like Patrick's idea.

Now, just thinking out loud (and shoot me down if I'm being stupid).

Could the property IsDirty be reset to false when a property is changed back to the original value, say when a name is "Smith" and gets changed to "Jones" by operator mistake who then changes it back to "Smith". At the moment IsDirty is still true, but should be false.

The IsDirty at the property level could add/subtract from an IsDirtyCount at the object level when it changes to true/false. Then IsDirty at the object level could check IsDirtyCount <> 0 (or without the IsDirtyCount it could loop through all the properties to check thier IsDirty - which I think would be less efficient).

Patrick replied on Monday, December 10, 2007

 

RockfordLhotka:

Would you go the rest of the way? Elevate the "property" concept to a first-class citizen?

If that is too much of a jump then wouldn't

        Dictionary<String, FieldData> propertyData
OR  FieldsData propertyFields
be still very close to your original suggestion?

RockfordLhotka:

Option 2 - managed (by BusinessBase ) backing field

private static PropertyInfo<string> NameProperty =
  new PropertyInfo<string>("Name", "Customer name");
public string Name
{
  get { return GetProperty<string>(NameProperty); }
  set { SetProperty<string>(NameProperty, value); }
}

Thanks a lot
Patrick

ajj3085 replied on Monday, December 10, 2007

Patrick:

I guess one question is how many running CSLA.NET projects get actually upgraded to the latest version of CSLA.NET vs how big the long term benefit might be.

If they use code generation they might but then they could upgrade the generated code to reflect the changes.
If they don't use code generation and it's a bigger project it might be less likely that they will upgrade anyway. But rather that they would use the latest version on their next project.

Bob Matthew replied on Tuesday, December 11, 2007

I like Patrick's solution as well - it solves a lot of problems of determing if an object really is dirty or not.

Regarding the concern of too much data across the wire and performance, why not have store some sort of computationally-lightweight hash in the BusinessBase?

Example:

1. When the object is loaded from the DB, a simple CRC32 (or other algo) is computed against the business object and then stored into two member variables _originalHash and _currentHash.

2. When any property is modified, _currentHash is cleared.

3. When IsDirty is called, it first checks to see if _currentHash is empty and if so, it computes it and then stores that value into the _currentHash variable.  Finally we return _originalHash == _currentHash to indicate if the object is dirty.

In this way we determine if the object is truly dirty (within a reasonable mathematical probability) without incurring significant computational or network overhead.

The only caveat is how to compute a hash on a non-native type (something other than int, string, DateTime, etc) - perhaps we would just call the GetHash() method implemented on all .NET objects?

ajj3085 replied on Tuesday, December 11, 2007

mr_lasseter replied on Tuesday, December 11, 2007

I'm with ajj3085 on this one.  It sure seems like an awful lot of overhead to determine if the user changed a value then changed the value back to the orginal object.  And if it is marked as NonSerialized then it doesn't really do much good since the child objects cannot make use of it once they are on the server side. 

Bob Matthew replied on Tuesday, December 11, 2007

ajj3085:

I don't see how a hash solves the problem of lots of objects (representing properties) moving across the wire.  Remember, whatever is contained in BusinessBase WILL travel the wire (unless its not serialized). 

Obviously the current state or data of an object would have to be transferred across the wire - there's no getting around that.  Patrick's post talks about keeping a copy of the old, saved data when it differs from the current, unsaved data for each property to determine if that property is dirty.  The solution that I proposed above is a way to maintain only the current data and still be able to tell if the entire object truly IsDirty while only transferring a few extra bytes across the wire.  In my case, I don't particularly care if any one property is dirty, I just want to ensure that the object doesn't go back across the wire when Save gets called and the object hasn't actually changed.

Example:

As it currently exists in the CSLA:

1. User object - has username "test" when loaded. (IsDirty = false)

2. username is updated to be "testing123". (IsDirty = true)

3. username is changed back to "test" - IsDirty is still true and a Save() call would serialize the object, send it to the data portal to be saved even though it's not truly dirty.

Using the solution I've proposed:

1. User object - has username "test" when loaded. (IsDirty = false)

2. username is updated to be "testing123". (IsDirty = true)

3. username is changed back to "test". (IsDirty = false) a Save() call would exit before going to the Data Portal.

Rocky: I like Patrick's idea of being able to tell if the object IsDirty, but I don't think we need to make Properties first-class citizens either.

RockfordLhotka replied on Tuesday, December 11, 2007

Bob Matthew:

Regarding the concern of too much data across the wire and performance, why not have store some sort of computationally-lightweight hash in the BusinessBase?

The problem is that this isn't safe. It will work usually, but could fail (silently). The reason is that a hash value is not guaranteed to be unique, and it is possible that an original and new value will hash to the same value.

This is the same reason that timestamp concurrency is good, while CRC (hash) concurrency is not recommended - it will typically work, but if it doesn't work it fails silently.

Bob Matthew replied on Tuesday, December 11, 2007

RockfordLhotka:

Bob Matthew:

Regarding the concern of too much data across the wire and performance, why not have store some sort of computationally-lightweight hash in the BusinessBase?

The problem is that this isn't safe. It will work usually, but could fail (silently). The reason is that a hash value is not guaranteed to be unique, and it is possible that an original and new value will hash to the same value.

This is the same reason that timestamp concurrency is good, while CRC (hash) concurrency is not recommended - it will typically work, but if it doesn't work it fails silently.

 

Statistically speaking it is true that the hash values could be identical - an easy solution would be to introduce an additional set of hash values based upon another algorithm and thus maintain dual hashes.  In this scenario the probability of a collision would approach zero.

Even so, what are the consequences of IsDirty not being correct?  If Save() is called, it saves to the database.  This is exactly the way that it functions now as pointed out from the example I posted earlier.

Remember, the objective is to determine if an object has truly changed and thus avoid excess or unnecessary calls to the portal, but also to do so without introducing significant network/CPU overhead.

Patrick replied on Wednesday, December 12, 2007

I thought bringing up some of the original questions might help the discussion: 

dctc42 replied on Wednesday, December 12, 2007

Hi all,

Just read Buisness Objecs C# 2005. I've tried several BO approaches in my work and what I liked about CSLA was that is was a pretty light weight framework. Once you start getting into the 'field info' concept and metadata and all that, you core buisness object class starts getting bloated. I guess this is unavoidable since developers are always looking for more features per line of application code.

At what point do BuisnessBase and BuisnessBaseList become as generic and bloated (general puporse/feature rich) as a DataSet or and ADO.NET EO? What do we really gain besides a cozy feeling that we are using BUISNESS OBJECTS?

ajj3085 replied on Thursday, December 13, 2007

Patrick replied on Thursday, December 13, 2007

ajj3085 replied on Thursday, December 13, 2007

GeorgeG replied on Friday, December 14, 2007

May be Rocky can have two BusinessBase. One regular BusinessBase and the other one "BusinessBaseMoreFunctionaly" where BusinessBaseMoreFunctionaly will inherit from BusinessBase do some overides. BusinessBaseMoreFunctionaly will give us more functionality as per Patrick's suggestion. Developers will be able to choose one or the other. Having Rocky maintain it in the framework would mean consistency across the board and everybody can use it if they need the extra functionality.

What others and Rocky think?

david.wendelken replied on Friday, December 14, 2007

Patrick:

Hi, it would be a pity if the discussion evaporates due to concerns for the size of the data structure when in reality the difference might be very small (and I am not sure if we really got any statistics right now)... especially compared to the benefits which could be quite large.

 

Having collections of

   PropertyInfo fields which hold meta data about the fields

and

   PropertyData (or FieldData) which hold the field values and possibly some meta data

can basically be very helpful.

Just having this type of structure in place in CSLA.NET allows developers to extend it if they need to and if they don't they can just leave it as it is.

 

The current way of using instance properties makes it difficult to:

a) keep meta data about property e.g. friendly names, default value, primary key etc.

b) keep meta data about property values e.g. original value, is dirty as well as adding field type specific methods

Having collections on the other hand allows very easily to add all of these and more. Either in the core framework or later on by the individual developer.

I am very excited about the possibility of having property meta-data available upon request.  I've been getting very good use out of making business rule meta-data available to users.  I would love to have a built-in way to do that with properties (and business objects themselves, for that matter!)

Jimbo replied on Sunday, December 16, 2007

RockfordLhotka replied on Sunday, December 16, 2007

JoeFallon1 replied on Thursday, December 20, 2007

Rocky,

1. I warned you about those compiler directives! <g>
    I vote you abandon them for 3.5.

2. I can see both sides of the debate of the enhanced Property code you are proposing. My choice would be that you somehow make it an optional feature that can be switched on/off on a per BO basis. In other words if I inherit from BusinessBase(Of T) let me set a switch in the BO to use the original field tracking mechanism. e.g. UseFieldTracking = True.

3. It turns out that in many of my BOs I do have to track the original field value. (For some subset of the fields.) I even have rules that require knowing the current value vs. the original value. So I can definitely see the value in this.

Joe

DCottle replied on Thursday, December 20, 2007

This is a great discussion.  Conversations like this I think have greatly contributed the the quality of the frameworks that have been put out thus far.  That and Rocky is a stud...

My 2 cents:   I think most of us have customized the framework in some way or another...a new interface, a new method, something.

Could we not have the FieldData used as Patrick has suggested and simply add our own partial class to the Field Data object to expand it to track information that we deem necessary as a developer?

RockfordLhotka replied on Friday, December 21, 2007

Patrick replied on Friday, December 21, 2007

RockfordLhotka:

I am considering two models at the moment – a base class model like that, or an interface-based model like IFieldData.
...
But at a minimum I intend to allow you to create a custom base class that enables original value tracking or other enhanced (if sometimes costly) behaviors.

Nice .
So would you use:

RockfordLhotka:

Option 2 - managed (by BusinessBase ) backing field
based on: Dictionary<String, Object>

Patrick:

    public class FieldData { }
    public class FieldData<T> : FieldData
    {
        private T _CurrentValue;
        public T CurrentValue
        {
            get  { return _CurrentValue; }
            set  { _CurrentValue = value; }
        }

RockfordLhotka replied on Friday, December 21, 2007

Patrick replied on Friday, December 21, 2007

vdhant replied on Monday, December 31, 2007

Hey guys
Just wondering if any more progress has been made here in terms of a design or concept of how things are actually going to work.

Also I was just thinking that because of the Undo features that the businessbase class have, I was wondering how this would impact the design. In that, if the dictionary approach was taken would the whole dictionary be subject to the undo (including all the proposed metadata which is the same from undo point to undo point) or just the actual field value that dictionary item represents. Likewise if the field route is chosen would the situation be similar???

One thing that could be done is that this system is run using three different dictionaries. One that is <string, object> which holds the property name and the current value (maybe a little more and not just the value but to be able to hold the value and a type which indicates whether the property has changed). The next being <string, object> which holds the property name and the original value (so it can be used to determine if the value has changed and because its separate if would be easy to turn on and off if performance is a concern with holding the original value) and this field would not be undoable. And the last being <string, PropertyMetaData> or something similar which holds all the metadata for that property which the metadata data may be instance or static based (meaning that some of the meta data may be set specifically for this instance of the object and some may be static) and this too would be marked as not undoable.

An update would be good on the current design that you are working on and I thought I would share the idea.
Thanks
Anthony

vdhant replied on Tuesday, January 01, 2008

Hi again
Other people have probably already thought of this, but if the data is kept in a dictionary it makes undo processes considerably easier, potentially more flexible and should be more efficient. In that, at the moment it uses reflection to loop through all the different sub classes, to find all the fields and copy them for each undo. If the dictionary was used, the only thing that would need to happen is that the dictionary is copied. The fields that are needed to be copied are already known and hence don’t need to be discovered through reflection. Meaning that for the whole process you don’t need to use reflection (besides maybe GetType) when it comes doing the copy (note you will still need the Serialization to deal with reference types). This does mean that every field that wants to be subject to the undo must be in the dictionary but I don’t think that this is a bad idea.

Physically i imagine that this would work by the system looping through each item in the dictionary when a copy is required, adding each of the current values to a new dictionary except where the type of the current item is of baseundo. If it is the later then it calls the undo dictionary copy process on it, just like the current model does.

Also it opens up some interesting possibilities of conducting differential undo copies. In that when a copy occurs, it only stores what values have changed (I see this feature as possibly one that could be turned on and off by config and that an attribute can be used to do a forced copy in some situations regardless of change). It also adds some interesting ideas that a user could just undo changes for a partial field and another idea where the user could undo the history of one of the histories that are not current, just like Photoshop does. It allows you to remove the third last change you did without undoing the last two. Now granted these two things are not features that every user interface would have, but imagine some of the UI possibilities where the user could have a panel on the left which allows them to track the changes as they make then and see what they had and what it now is and undo just particular changes in history if required.

It might also mean that some of the limitations that you have previously mentioned with porting the undo functionality to silverlight won’t be a problem because it no longer relies on reflection in the way it did.

Also in terms of the initial base/default data being stored of what the object was, if memory serves me correctly peoples main objection was the about of extra data that this would mean that was travelling over the wire. What if the default data was stored in the dictionary like i have mentioned above and that dictionary is marked as nonserializable and that that dictionary only gets populates after the data comes back from the data portal on the fetch. Meaning that it adds no extra overhead to what’s going across the wire. It still means that more memory will be taken up on the client because of the copy but maybe if the copy was optional and an attribute was able to be set to say don’t take a default snapshot for this value. This might be used in cases where you have lots of text in a field or something. If that attribute is chosen then the system which checks whether the value has changed would have to conclude that a value has changed if the property is updated once (regardless of whether it was put back to what it was).

Just thought I would share a few more ideas I had.
If anyone has any ideas on this I would be more than happy to see what you think.
Thanks
Anthony

vdhant replied on Wednesday, January 02, 2008

Any updaes or ideas on this topic yet??
Thanks
Anthony

RockfordLhotka replied on Thursday, January 03, 2008

vdhant:

It might also mean that some of the limitations that you have previously mentioned with porting the undo functionality to silverlight won’t be a problem because it no longer relies on reflection in the way it did.

Yes, this is a potential benefit. Though based on recent ScottGu comments, it may be a non-issue. I think they ended up implementing more reflection in Silverlight than they originally planned, because data binding is incredible reflection-heavy, and they are implementing data binding (or a large subset of it anyway) in Silverlight.

Which is good news imo!

vdhant:

Also in terms of the initial base/default data being stored of what the object was, if memory serves me correctly peoples main objection was the about of extra data that this would mean that was travelling over the wire. What if the default data was stored in the dictionary like i have mentioned above and that dictionary is marked as nonserializable and that that dictionary only gets populates after the data comes back from the data portal on the fetch. Meaning that it adds no extra overhead to what’s going across the wire. It still means that more memory will be taken up on the client because of the copy but maybe if the copy was optional and an attribute was able to be set to say don’t take a default snapshot for this value. This might be used in cases where you have lots of text in a field or something. If that attribute is chosen then the system which checks whether the value has changed would have to conclude that a value has changed if the property is updated once (regardless of whether it was put back to what it was).

I'm not sure this is so clear. The dictionary will store IFieldData objects - which could be child collections, child objects or FieldData<T> objects (that contain a field and related metadata). Or they could be a custom-created implementation of IFieldData that stores original values or does other fancy things like you are describing.

But when the business object flows through the data portal, the dictionary must go too, because it now contains the object's state. The child objects, field values and so forth must all be available to the clone on the other end of the wire or we lose location transparency.

It is important to remember that the data portal is not designed as a datagram replacement. The data portal is all about location transparency and giving you the illusion that your objects didn't actually move (as much as possible).

If the goal is not location transparency, then the data portal should not be used. Instead, the data portal should always be run in local mode and a datagram-based DAL should be invoked to get across the wire. That provide architectural clarity, because such a DAL is clearly not trying to clone the object graph across the wire, it is merely moving optimized datagrams from the client to a server-side component.

So the data portal will clone the dictionary as part of the object. What I am hoping however, is that I can implement ISerializable by hand - either at the FieldDataManager level or more likely at the FieldData<T> level to minimize the size of the byte array. Unfortunately I am skeptical that this will work, because there's always the potential for a subclass of FieldData<T> or a custom implementation of IFieldData.

The primary reason the byte stream expands is that the type of each child object in the dictionary must be serialized, in addition to the data inside the child object. If I knew that all child objects were FieldData<T> I could optimize. But to support custom classes I can't know that.

I'm also leery of the premature optmization anti-pattern, and so I'll probably finish the prototype code without worrying about this issue overmuch, and then see if I believe it is a serious issue.

Bob Matthew replied on Thursday, January 03, 2008

I’ve been able to save a little bit on serialization through implementing a static dictionary and an instance array of values:

private static Dictionary<string, int> _properties = new Dictionary<string, int>(); // property name as key, value = index of FieldData value in _value array.

private FieldData[] _values = new FieldData[_properties.Count];

protected static PropertyInfo<TProperty> RegisterProperty<TProperty>( string name, string display )

{

                var prop = new PropertyInfo<TProperty>(name, display);

                _properties[prop] = _properties.Count;

                return prop;

}

By using the RegisterProperty function ( instead of "= new PropertyInfo<>()" ), we add all the properties the dictionary in the same order which will ensure that the properties are stored in the same location in the array on both sides of the data portal.

RockfordLhotka replied on Thursday, January 03, 2008

Bob Matthew replied on Friday, January 04, 2008

RockfordLhotka replied on Friday, January 04, 2008

The whole serialization thing is troublesome. Necessary of course, but troublesome.

I spent a lot of time yesterday, for example, trying to work around the fact that a dictionary's OnDeserialized method is called before the contents of the dictionary have been deserialized. Unfortunately the dictionary itself is apparently "deserialized" before its contents. Then there's no way to know when the contents do finally get deserialized.

I did come up with a workaround, but I'm not sure I like it. So now I'm considering switching to a more complex data structure to avoid using a dictionary. Well "complex" isn't the right word - the data structure isn't complex, but the fact that I have to manually create the data structure is complex compared to just using a pre-built type...

RockfordLhotka replied on Friday, January 04, 2008

I put up a new preview release of 3.5 just now. The VB version has the new property functionality, which passes my tests and looks pretty good.

You can see the primary test business root class.

Declare a property with private backing field:

private static PropertyInfo<string> NameProperty = new PropertyInfo<string>("Name");
private string _name = NameProperty.DefaultValue;
public string Name
{
  get { return GetProperty<string>(NameProperty, _name); }
  set { SetProperty<string>(NameProperty, ref _name, value); }
}

Declare a property with a managed backing field:

private static PropertyInfo<string> NameProperty = new PropertyInfo<string>("Name");
public string Name
{
  get { return GetProperty<string>(NameProperty); }
  set { SetProperty<string>(NameProperty, value); }
}

Declare a SmartDate string property with private backing field:

private static PropertyInfo<SmartDate> BDateProperty = new PropertyInfo<SmartDate>("BDate");
private SmartDate _bDate = BDateProperty.DefaultValue;
public string BDate
{
  get { return GetProperty<SmartDate, string>(BDateProperty, _bDate); }
  set { SetProperty<SmartDate, string>(BDateProperty, ref _bDate, value); }
}

Declare a SmartDate string property with managed backing field:

private static PropertyInfo<SmartDate> BDateProperty = new PropertyInfo<SmartDate>("BDate");
public string BDate
{
  get { return GetProperty<SmartDate, string>(BDateProperty); }
  set { SetProperty<SmartDate, string>(BDateProperty, value); }
}

Declare a single child object property:

private static PropertyInfo<Child> ChildProperty = new PropertyInfo<Child>("Child");
public Child Child
{
  get
  {
    if (!FieldManager.FieldExists(ChildProperty))
      SetProperty<Child>(ChildProperty, Child.NewChild());
    return GetProperty<Child>(ChildProperty);
  }
}

Declare a single lazy loaded child object property:

private static PropertyInfo<Child> ChildProperty = new PropertyInfo<Child>("Child");
public Child Child
{
  get
  {
    if (!FieldManager.FieldExists(ChildProperty))
      SetProperty<Child>(ChildProperty, Child.GetChild(this.Id));
    return GetProperty<Child>(ChildProperty);
  }
}

Declare a child list object property:

private static PropertyInfo<ChildList> ChildListProperty =
  new PropertyInfo<ChildList>("ChildList");
public ChildList ChildList
{
  get
  {
    if (!FieldManager.FieldExists(ChildListProperty))
      SetProperty<ChildList>(ChildListProperty, ChildList.NewList());
    return GetProperty<ChildList>(ChildListProperty);
  }
}

Declare a lazy loaded child list object property:

private static PropertyInfo<ChildList> ChildListProperty =
  new PropertyInfo<ChildList>("ChildList");
public ChildList ChildList
{
  get
  {
    if (!FieldManager.FieldExists(ChildListProperty))
      SetProperty<ChildList>(ChildListProperty, ChildList.GetList(this.Id));
    return GetProperty<ChildList>(ChildListProperty);
  }
}

That covers all the variations currently supported in the VB version of the framework. Whether you develop in VB or C#, I'd appreciate it if you can write some code against the VB framework and let me know what you think, whether you encounter issues, etc.

Your feedback now is critical, as I intend on effectively finalizing this by January 11. Gotta move onto other areas, and also allow time to port all these changes to C#. Thanks!

vdhant replied on Friday, January 04, 2008

RockfordLhotka replied on Friday, January 04, 2008

vdhant:

In FieldData maybe this could test a config setting before simply setting the dirty as true if the object data changes. It might make things easier if people want to enhance the field isDirty.
 
Public Property Value() As T
Get
Return mData
End Get
Set(ByVal value As T)
mData = value
If ApplicationContext.UseSimpleFieldIsDirty Then  mIsDirty = True
End Set
End Property

The extensibility model isn't done, but the basic concept is that if you want to extend/change the default behaviors you need to create custom IPropertyInfo and IFieldData classes of your own design. These two interact to make it all work.

So you'd (at a minimum) create a subclass of PropertyInfo<T> that returns the right kind of IFieldData as needed.

And you'd create an IFieldData like my FieldData<T> that does things like store original values, or provides alternate implementations of IsDirty.

But to be honest, I haven't thought all the way through that, so there could be holes I haven't figured out yet. My goal thus far has been to get the core functionality working, which I've now done. Next I'll work through extensibility in detail.

But I'm not overly worried about making this "simple". Extensibility can be complex - it is an advanced scenario. Common usage must be simple, because that's a common scenario.

vdhant replied on Friday, January 04, 2008

That’s fine. Off the top of my head one thing to consider is that if you are imagining that if people want to record the default data and that FieldData is the place to do that, one thing to consider is that I cannot see a scenario where I would want that to be copied with each undocopy. All being equal and this is the place where you imagine that you would want the default data to go, with my understanding of the default process and the way it walks the object tree structure there would be no way to get it not to copy this data for each undocopy, is that correct??? That’s the one thing that comes to mind and it may be something that you have already catered for.
Thanks for the quick feedback on the above.
Anthony

RockfordLhotka replied on Saturday, January 05, 2008

JoeFallon1 replied on Sunday, January 06, 2008

I recall reading about the dictionary serialization timing bug a while back. I'm not sure if you pointed it out or Rick Strahl did. It is pretty nasty. You'd think it would be fixed by now as it makes no sense.

Joe

Patrick replied on Tuesday, February 12, 2008

Hi,

I was just wondering if the naming of LoadProperty is a bit counter intuitive?

At the moment it is:
GetProperty (with checks)
SetProperty (with checks)
ReadProperty (without checks)
LoadProperty (without checks)

Wouldn't this be more intuitive?
GetProperty (with checks)
SetProperty (with checks)
ReadProperty (without checks)
WriteProperty (without checks)

For me Read and Load are just too similar (nearly the same thing).

To change it at the moment would just be search and replace for a few people but once it's released it’s set in stone.

Thanks for considering it,
Patrick

vdhant replied on Wednesday, February 13, 2008

Load makes senesce to me as when you call load it marks the property as not being dirty... thus one would know that when "loading" a value the field will marked as not dirty. I don’t think that Write gives this same impression. If anything you would have read, load and write, where the difference between load and write is that load sets the property as not dirty and write sets it as dirty...

Just a thought.
Anthony

Patrick replied on Friday, January 11, 2008

Bob Matthew:

I wonder if Patrick has any ideas.

couldn't give any feedback.

I trust though that it is very good
Thanks & all the best,
Patrick

RockfordLhotka replied on Thursday, January 03, 2008

JoeFallon1:

1. I warned you about those compiler directives! <g>
    I vote you abandon them for 3.5.

Yes, you did

And I do plan to abandon them in 3.5, because it is impractical to use the new language features and allow for <3.5 support. And there's no way I'm going to avoid using the new language features - they are far too cool/useful.

But I do think that using the NET20 directive for CSLA 3.0 was the right choice, because it means 3.0.x supports .NET 2.0-3.0 and so a single code base covers a wide range of users.

JoeFallon1:

2. I can see both sides of the debate of the enhanced Property code you are proposing. My choice would be that you somehow make it an optional feature that can be switched on/off on a per BO basis. In other words if I inherit from BusinessBase(Of T) let me set a switch in the BO to use the original field tracking mechanism. e.g. UseFieldTracking = True.

3. It turns out that in many of my BOs I do have to track the original field value. (For some subset of the fields.) I even have rules that require knowing the current value vs. the original value. So I can definitely see the value in this.

My current plan/prototype makes it optional - though not by a switch. You either use the feature or you don't. If you don't use the feature you don't pay the cost, nor do you reap the reward. If you do use the feature you pay the cost and get the benefit.

More specifically, I'm implementing two overloads of GetProperty() and SetProperty() - one that accepts a private field and one that manages the "field" value for you.

If you opt for a private field you'll get better performance and smaller serialized data over the data portal.

GetProperty<string>(NameProperty, _name);
SetProperty<string>(NameProperty, ref _name, value);

If you opt for a managed field you'll get the potential for original value tracking and more intelligent IsDirty behavior.

GetProperty<string>(NameProperty);
SetProperty<string>(NameProperty, value);

But just storing the field values in a Dictionary<string, IFieldData> increases the size of the serialized object byte stream - even before you add in the original value and any other meta-data you want to store on a per-property basis... I'm still trying to figure out how to minimize this impact. The next step is to manually implement ISerializable in my FieldData<T> class to see if that helps - but I'm a bit skeptical at this point.

I want to be clear though - I am not doing original value tracking in CSLA. I am enabling the concept by allowing you to subclass something called FieldDataManager and/or to provide your own implementation of IFieldData. To be honest, the specifics of how you'll do this are a bit vague just now, as my focus has been on getting the basic implementation working in a way I like. Once that's done I'll focus on extensibility.

In all cases, you will be able to (and should) store child references using the managed overload. The benefits there are tremendous and the serialization overhead is minimal. The benfits include:

• No need to manually override IsDirty or IsValid in the parent
• No need to cascade child events through the parent (no hooking and rehooking events manually)
• Automatic edit level fix-ups for lazy loaded child objects
• More standardized code structure for a child property implementation - making code-gen easier

RockfordLhotka replied on Friday, January 04, 2008

Well Bob, I did end up doing something similar to what you are doing. I'm storing the values in a List<IFieldData> and manually maintaining a keyed Dictionary to index back into it. My implementation isn't order-dependant and so I think it is safe.

I had to do this due to that nasty deserialization issue with Dictionary<> where the dictionary claims to be deserialized before the children it is supposed to contain are actually restored to the dictionary. Since I need to walk through and reset the parent reference and rehook child events on deserialization this was a serious issue.

I'm not sure that the new scheme makes the serialized object much (any) smaller on the wire, but at least it actually works

RockfordLhotka replied on Friday, December 07, 2007

Patrick replied on Friday, December 07, 2007

RockfordLhotka:

I have so far rejected tracking original values, primarily because of the bloat it incurs over the wire. Doing this doubles the size of the object graph as it moves from app server to/from client.

david.wendelken replied on Friday, December 14, 2007

RockfordLhotka:

It is true that having the original value is a really nice and powerful feature, but it isn’t clear to me that it is valuable enough (to everyone) to be worth the costs.

(I don't want to derail the topic, I solved the above problem a different way because I had a specific purpose in mind.  If people want to discuss it, start a new thread and link to it from here.)

If I just want to go back to the original values, I can ask the database for the values again.  No need to tote them around everywhere until then.

However, I did want to know which business rules were already broken when the object was instantiated from the database.  They might be broken because of severity levels allow it, or they might be broken because it's a new (or newly-debugged) rule and the users haven't corrected the data yet.

For example, let's say that we have three new required fields for the Person business object, but those fields have not been filled in.  We now need to know their nickname, their hair color and their favorite band.  Typically, if a user tried to change the person's address, they would not be able to, until they also filled in the 3 new fields. 

Because I know that those three rules were broken before the user fiddled with the record, it's ok for them to save the data as long as they don't break a new rule.  The concept of "incremental improvement" is missing from most business rule frameworks.  Plus, of course, the UI can direct the user to fix stuff BEFORE they try to save the data.

RockfordLhotka replied on Thursday, December 06, 2007

RRorije replied on Monday, January 28, 2008

Hi,

This architecture reminds me of the structure JP boodhoo (www.jpboodhoo.com) uses in his project (http://code.google.com/p/jpboodhoo/) for his table-classes. I do not know whether it completely resembles this structure given by Rocky. But it might be worthwhile to take a look at it.

Below I have given some sample code, which I just copied and pasted out of the source of JP Boodhoo. I think the FieldData which is discussed in this thread and TableColumn do have some similarities.

Take a look at the code from JP, I think it is very nice!!!

Here a (shortened) table class:

public class CustomersTable{
   public static readonly string TableName = "Customers";

   public static readonly TableColumn<string> LastName = new TableColumn<string>("LastName");
   public static readonly TableColumn<string> FirstName = new TableColumn<string>("FirstName");
}

And the Tablecolumn class.

public class TableColumn<ColumnType>{
   private string columnName;
   public TableColumn(string columnName){this.columnName = columnName; }
   public static implicit operator string(TableColumn<ColumnType> column)
   {
      return column.columnName;
   }
   public string ColumnName{ get { return columnName; } }
   public object RetrieveRawValueFrom(DataRow row){
      return row[columnName];
   }

   public ColumnType MapFrom(DataRow row) {
      return (ColumnType) RetrieveRawValueFrom(row);
   }

   public bool IsNullFor(DataRow row)   {
      return RetrieveRawValueFrom(row) == DBNull.Value;
   }
}

Jimbo replied on Saturday, January 05, 2008

RockfordLhotka replied on Saturday, January 05, 2008

dmostert replied on Friday, January 18, 2008

RockfordLhotka replied on Friday, January 18, 2008

dmostert:

I have a slightly different suggestion:

Some suggestions allow for Properties to be re-used by different objects - which can be good.
But, what if you "registered" the property in the class? Use a static method and list and "register as you declare" - this will take care of the dictionary lookup and the only penalty is the boxing / unboxing:

I originally did implement a static RegisterProperty() approach (more like the DependencyProperty idea in WPF/WF). But when I was done I found that I wasn't actually "registering" anything. The "resgister" process was simply adding the PropertyInfo<T> to a dictionary, and that was never used for anything.

So, as an optimization, I removed RegisterProperty() and that (essentially useless) dictionary of PropertyInfo<T> objects.

One thing that did occur to me was to go even further than what I'm doing here. Something so radical it would pretty much eliminate any upgrade path.

The idea is to make a "property" a first-class citizen. An object in its own right. Each property would have its own authorization and business rules. You'd write each property as a class, including code much like a current business object (to set up authorization and validation rules, etc). Then a business object would look much like it does today, but would register properties with a static RegisterProperty() so when an object instance is created it could loop through that list and create instances of each property object.

The amount of code you'd write would go up quite a lot (I think I showed some of it earlier in this thread), and the memory footprint on a per-business object basis would also go up, both in RAM on when serialized on the wire (though I think I could solve the issue on the wire by implementing custom serialization).

But the model is extremely elegant

On the whole though, my focus for 3.5 was to reduce the code needed to define a business object, not to increase the code required, so I decided (with input from the forum) to go with the current scheme.

vdhant replied on Saturday, January 26, 2008

Hi Rocky
I just noticed something that may be a posible problem. In a couple of places you are doing the following:

              OnPropertyChanging(propertyName);
              field = newValue;
              PropertyHasChanged(propertyName);

And then within PropertyHasChanged you are doing:

      OnPropertyChanging(propertyName);
      ValidationRules.CheckRules(propertyName);
      MarkDirty(true);
      OnPropertyChanged(propertyName);

Which means OnPropertyChanging is fireing twice. Now this might be by design but if it is it would seem rather strange. My first instinct was to think that the two OnPropertyxyz method calls should be removed from the method since now the control within CSLA over when properties are set you could always have the OnPropertyChanging fireing before the value changes and OnPropertyChanged after. But in saying this I am not sure if there are other situations where people might need to all propertyhaschanged and need the OnPropertyxyz methods  to run. So maybe you could create an overload that has a bool as the second param that can be used to switch whether the OnPropertyxyz methods should run (i.e. suppressEvent).

Also just wondering with the advent of OnPropertyChanging & OnUnknownPropertyChanging shouldn't these type of methods now have OnUnknownPropertyChanging included or perhaps they could be changed to OnPropertyXyz and have "isDirty" passed in (maybe that could have a few issues thought when changing multiple fields in these methods)?

    protected void MarkDirty(bool suppressEvent)
    {  
      if (!suppressEvent)
        OnUnknownPropertyChanging();
      _isDirty = true;
      if (!suppressEvent)
        OnUnknownPropertyChanged();
    }


Just an idea if this is a problem.
Anthony

RockfordLhotka replied on Monday, January 28, 2008

RockfordLhotka replied on Monday, January 28, 2008

I hate to do something like this at the 11th hour, but the benefits are too great to ignore...

Specifically, I'm going back to the use of a RegisterProperty() method:

private static PropertyInfo<string> NameProperty =
  RegisterProperty<string>(new PropertyInfo<string>("Name"));

Private Shared NameProperty As PropertyInfo(Of String) = _
  RegisterProperty(Of String)(New PropertyInfo(Of String)("Name"))

To allow for external creation/maintenance of the PropertyInfo objects, notice how I'm requiring that you create the object. The RegisterProperty() method itself is generic so you don't have the cast the result. It returns the object you passed in, which is required because this must be used as shown - on the declaration of a static field (or I suppose in a static constructor, in which case you could probably technically ignore the result of RegisterProperty).

The core reason for the change is a 30% performance improvement. This technique means I can store managed field values in an array rather than a dictionary, so all get/set operations avoid a dictionary lookup (a couple in many cases) and this increases performance just a lot.

A side-effect is that I should be able to (though this won't be in Beta 1) do some cool stuff in DataMapper as well, since I'll be able to directly get a list of properties in an object without needing to use reflection, attributes or other slower technologies.

William replied on Monday, February 04, 2008

I am trying to follow through the discussions in this thread, and have the following questions and comments.

1) Comparing the usage syntax of "Declare a property with private backing field" and "Declare a property with a managed backing field", the only different is the declaration of the private variable. Thus, can I make the assumption that when using private backing field, BO has duplicated state in the storage. One on the private backing field; another one on the managed backing field. This would increase the size of the serialized stream?

2) When using "Declare a property with a managed backing field", neither GetProperty() nor SetProperty() is given any reference to the current object. How does CLSA knows which set of field values belong to which BO instance? My first guess would be that GetProperty() and SetProperty() are using reflection or stack tracing? Would this a good practice?

3) In term of syntax, I am thinking that the field declaration syntax is a bit too long. Not very significant but just a comment.

   private static PropertyInfo<string> NameProperty =
      RegisterProperty<string>(new PropertyInfo<string>("Name"));

   When compare with regular private field declaration, which is sort and concise.

   private static string _name = String.Empty;

I haven't go through the actual code for GetProperty() and SetProperty() but these are my first take.

Thanks.

Regards,
William

RockfordLhotka replied on Monday, February 04, 2008

William replied on Tuesday, February 05, 2008

Patrick replied on Wednesday, February 06, 2008

vdhant replied on Wednesday, February 06, 2008

Sounds interesting...
Anthony

Bob Matthew replied on Thursday, February 14, 2008

Rocky,

I noticed in your example you had the RegisterProperty signature as follows:

private static PropertyInfo<string> NameProperty =
  RegisterProperty<string>(new PropertyInfo<string>("Name"));

Whereas in the Csla.Core.BusinessBase class it had an additional parameter of Type objectType.

I was wondering if would be possible for you to add an overload in the Csla.BusinessBase class that simply did this:

protected static PropertyInfo<P> RegisterProperty<P>( PropertyInfo<P> info)

{

  return RegisterProperty( typeof(T), info ); // where T is the generic parameter provided to BusinessBase.

}

RockfordLhotka replied on Thursday, February 14, 2008

RockfordLhotka replied on Saturday, February 16, 2008

ben replied on Thursday, October 16, 2008

Hi all,

We are using CSLA v3.0.2. In our application we have implemented property accessors following some considerations related to business logic, security and performance.

We are studding to jump to CSLA v3.6.0 and it would be very helpful for us your opinion about these considerations and your advise about how to implement properties accessors with the new philosophy of PropertyInfo and our requirements.

Thanks in advance

Benjamin

Protecting object’s surface

Security considerations

Any business object provides business logic through its properties and methods. That business logic may be used by the user or by other process. A business process may be composed by other sub processes.

Security authorizations must guarantee the user only access those properties and methods allowed to him/her. But there may be a conflict when some business process may be called by the user and by other processes. Let see an example:

There is a business rule that say: “When a customer is assigned to an order, all billing data associated with that customer must be set (copied) to the new order”.

There may be an actor (an administrative) than can create an order but can’t see or change the billing data from the customer (it may be sensitive data).

In this example the user must assign the customer to the order and the customer billing data must be copied to the order without letting the user seeing or changing that information.

The properties accessors must guarantee the user can’t read and write their values but there is a process that needs to access those values to copy them from the customer to the order.

Performance considerations

Before starting any process (by the user) the security system must be checked to see the user rights. But after the process is initialized the security system should not be rechecked for every sub process required by the main process.

In the example explained above, the process that copy the billing data from the customer to the order may need an authorization to read every property from the customer and to write the same properties to the order. Those security checks may affect performance with no need.

Conclusion

These considerations illustrate the need of having two surfaces for the business logic. One for the user, protected by the security system, and other for internal processes, protected by “Friend” members.

Example code

Public Class Customer

   Inherits Csla.BusinessBase(Of Customer)

   Private mCustomerID As Guid = Guid.Empty

   Private mBillingData As String = String.Empty

   Public ReadOnly Property CustomerID() As Guid

      Get

         CanReadProperty("CustomerID", True)

         Return Me.get__CustomerID()

      End Get

   End Property

   Friend Function get__CustomerID() As Guid

      Return Me.mCustomerID

   End Function

   Public Property BillingData() As String

      Get

         CanReadProperty("BillingData", True)

         Return Me.get__BillingData()

      End Get

      Set(ByVal value As String)

         CanWriteProperty("BillingData", True)

         set__BillingData(value)

      End Set

   End Property

   Friend Function get__BillingData() As String

      Return Me.mBillingData

   End Function

   Friend Sub set__BillingData(ByVal Value As String)

      If Me.mBillingData Is Nothing Then

         If Value IsNot Nothing Then

            Me.mBillingData = Value

            PropertyHasChanged("BillingData")

         End If

      ElseIf Not Me.mBillingData.Equals(Value) Then

         Me.mBillingData = Value

         PropertyHasChanged("BillingData")

      End If

   End Sub

#Region " Factory Methods "

   Public Shared Function NewCustomer() As Customer

      ' Security checks go here

      Security.AccessControl.GoingTo("CreateCustomer")

      Return Do_NewCustomer()

   End Function

   Friend Shared Function Do_NewCustomer() As Customer

      Dim obj As Customer

      obj = Csla.DataPortal.Create(Of Customer)()

      Return obj

   End Function

   Public Shared Function GetCustomer(ByVal CustomerID As Guid) As Customer

      ' Security checks go here

      Security.AccessControl.GoingTo("GetCustomer")

      Return Do_GetCustomer(CustomerID)

   End Function

   Friend Shared Function Do_GetCustomer(ByVal CustomerID As Guid) As Customer

      Return CType(Csla.DataPortal.Fetch(New Criteria(CustomerID)), Customer)

   End Function

#End Region ' Factory Methods

   Private Sub New()

      mCustomerID = Guid.NewGuid

      mBillingData = "This is an example."

   End Sub

   Protected Overrides Function GetIdValue() As Object

      Return mCustomerID

   End Function

End Class

Public Class Order

   Inherits Csla.BusinessBase(Of Order)

   Private mOrderID As Guid = Guid.Empty

   Private mCustomerID As Guid = Guid.Empty

   Private mBillingData As String = String.Empty

   Public ReadOnly Property OrderID() As Guid

      Get

         CanReadProperty("OrderID", True)

         Return Me.get__OrderID()

      End Get

   End Property

   Friend Function get__OrderID() As Guid

      Return Me.mOrderID

   End Function

   Public Property CustomerID() As Guid

      Get

         CanReadProperty("CustomerID", True)

         Return Me.get__CustomerID()

      End Get

      Set(ByVal value As Guid)

         CanWriteProperty("CustomerID", True)

         set__CustomerID(value)

      End Set

   End Property

   Friend Function get__CustomerID() As Guid

      Return Me.mCustomerID

   End Function

   Friend Sub set__CustomerID(ByVal Value As Guid)

      If Not Me.mCustomerID.Equals(Value) Then

         Me.mCustomerID = Value

         ' copy customer's billing data.

         Dim cust As Customer = Customer.Do_GetCustomer(Me.mCustomerID)

         Me.mBillingData = cust.get__BillingData

         PropertyHasChanged("CustomerID")

         PropertyHasChanged("BillingData")

      End If

   End Sub

   Public Property BillingData() As String

      Get

         CanReadProperty("BillingData", True)

         Return Me.get__BillingData()

      End Get

      Set(ByVal value As String)

         CanWriteProperty("BillingData", True)

         set__BillingData(value)

      End Set

   End Property

   Friend Function get__BillingData() As String

      Return Me.mBillingData

   End Function

   Friend Sub set__BillingData(ByVal Value As String)

      If Me.mBillingData Is Nothing Then

         If Value IsNot Nothing Then

            Me.mBillingData = Value

            PropertyHasChanged("BillingData")

         End If

      ElseIf Not Me.mBillingData.Equals(Value) Then

         Me.mBillingData = Value

         PropertyHasChanged("BillingData")

      End If

   End Sub

   Private Sub New()

      mOrderID = Guid.NewGuid

   End Sub

   Protected Overrides Function GetIdValue() As Object

      Return mOrderID

   End Function

End Class

Namespace Security

   Public Class AccessControl

      Public Shared Function CanI(ByVal PermissionName As String) As Boolean

         'TODO: Implement this method.

         Return False

      End Function

      Public Shared Sub GoingTo(ByVal PermissionName As String)

         'TODO: Implement this method.

         Throw New System.Security.SecurityException( _

               "TODO: Implement security cecks")

      End Sub

      Private Sub New()

      End Sub

   End Class

End Namespace

RockfordLhotka replied on Thursday, October 23, 2008

ben replied on Thursday, October 23, 2008

Many thanks Rocky. I’m going to start studying CSLA version 3.6.

I’m looking forward reading your new book.

Thanks again.

Benjamin