MarimerLLC / cslaforum

Discussion forum for CSLA .NET
https://cslanet.com
Other
31 stars 6 forks source link

Question on modelling setters of object "using" relationships #93

Open oatsoda opened 8 years ago

oatsoda commented 8 years ago

How do most people model their Business Objects when it comes to setting a relationship?

I have inherited a legacy WinForms app where CSLA has been (ab)used and primarily the business objects reflect data objects rather than properly modelling Business Logic. This means that the Business Objects have a lot of properties which are "IDs" which the UI/Presentation Logic then has to know the context of.

For me, it seems preferable to add a "using" reference property with a Getter to fetch the relevant Business Object and also supply a Setter for assigning the value. Is this something that others do?

As a secondary question, the WinForms app has a number of reference data objects modelled as NameValueListBase, which further invests in the idea of using the "IDs" (albeit less explicitly as binding can be utilised) which I feel I would be better off modelling with BusinessBindingListBase as they are more versatile.

I am proposing that I:

Does anyone have any experience of this or comments/suggestions?

Here is an example of how I would model this for a Person object which has a Marital Status:

public class Person : BusinessBase<Person>
{
       private static readonly PropertyInfo<int> s_MaritalStatusIdProperty = RegisterProperty<int>(c => c.MaritalStatusId);

       public int MaritalStatusId
       { 
              get { return GetProperty(s_MaritalStatusIdProperty); } // No Setter
       }

       private static readonly PropertyInfo<MaritalStatus> s_MaritalStatusProperty = RegisterProperty<MaritalStatus>(c => c.MaritalStatus);

       /* This assumes that we want the Reference property to be lazy loaded */
       // See CSLA:Objects:P36
       [NonSerialized]
       [NotUndoable]
       private MaritalStatus m_MaritalStatus = s_MaritalStatusProperty.DefaultValue;

       public MaritalStatus MaritalStatus
       {
              get
              {
                     // Do Lazy loading here
                     // Note that this is not a child, it is just a "Using" reference
                     if (m_MaritalStatus == null)
                           m_MaritalStatus = MaritalStatus.GetMaritalStatus(MaritalStatusId));

                     return GetProperty(s_MaritalStatusProperty, m_MaritalStatus);
              }
              set
              {
                     SetProperty(s_MaritalStatusProperty, value); // Will trigger business rule to update Id property
              }
       }      

       protected override void AddBusinessRules()
       {
              // See below...
              BusinessRules.AddRule(new UsingReferenceIdBusinessRule(s_MaritalStatusProperty, s_MaritalStatusIdProperty));
       }
}

public class UsingReferenceIdBusinessRule : BusinessRule
{
       // Business Rule to set the ID value when relationship object is set.
}

public class MaritalStatus : ReadOnlyBase<MaritalStatus> { //etc }
ajj7060 commented 8 years ago

I think instead of holding a reference to the MaritalStatus class, I'd have a SetMaritalStatus method that takes an instance. You'd then have a few properties, like the MaritialStatusId (the property itself can be private, so the ID can't be used from outside the Person class, and a string property MartialStatus. Its likely you don't need much more than. If you do need more data from the MartialStatus class for the Person class to do its job, then holding a reference like you have is fine, except I would not use the private backing field, unless you absolutely must keep the size of the data going over the network small.

However in this case if martial status is really just an ID and string, I'd probably keep it a NVL and have the ID property on Person. The reason is that in WinForms you'd probably have this particular property bound to a ComboBox, and the list items for the combobox would be bound to your NVL with DisplayMember and ValueMember set to the string and ID, respectively.

The thing to remember is that your UI heavily influences your use case, and so your business object will usually line up pretty closely to your UI. If you were doing say an MVC app, you'd probably be dealing with IDs even more since that's all that's likely to be coming in from your controller method, and it may not be effective to keep loading lots of read-only objects just to set a property on an editable property. Each approach (the NVL or your approach) has its own set of tradeoffs, so you'll have to figure out what works best for you and your team.

Ultimately what you do depends on where you think you might be; you can have a balance between the two if you think you'll be doing MVC web apps and desktop apps with the same business library. Or you can do more OO style if you're probably only going to be doing desktop apps.

oatsoda commented 8 years ago

Thanks @ajj3085, interesting points.

The plan is to move towards Web Apps in due course, so it is absolutely something I need to be thinking about. I understand what you mean about fetching read only objects just to assign relationships, but I guess that presupposes that I have passed an ID to the controller, which may not be the case. I am keeping a close eye on https://github.com/MarimerLLC/csla/issues/147 (https://github.com/MarimerLLC/cslaforum/issues/90) to see whether a Web API based around the Business Objects will come to fruition. But I agree that if it came to it, a simpler MVC or Web API wrapped over CSLA could benefit from not having to fetch objects just to assign them.

I'm interested as to why you would copy the Marital Status string over to the object? This seems to break the idea of encapsulation and modelling behaviours in one place.

I think also that my example was probably over simplified - and I would probably extend the pattern to User Data instead of just Reference Data - for example assigning an Address to a Person object.

I appreciate that just modelling my objects with IDs will be the simplest solution and probably the most flexible, but I just really hate the idea that I have gone to the trouble of modelling my Business Objects with Business Rules which makes sure that IsValid == true before I can Save() successfully, only for it to fall down on the basis that person.MaritalStatusId = -1235423 or person.AddressId = 23082423 throws a Foreign Key exception right down the stack when trying to persist the data.

ajj7060 commented 8 years ago

If you're moving toward web apps, you almost certainly will have a MartialStatusId propety on your Person, since you probably won't have serialized instances of MaritalStatus in the web page itself (you could probably try, but I suspect the effort will be more than the benefit).

I believe Rocky has stated something along these lines before when talking about OO design; OO is about behavior + data. The behavior comes first, the data your BO's have is only there to enable the BO to implement the use case its designed to implement. The string for marital status has no behavior; its just a piece of data. But as I said, having a MaritalStatus property (which is a reference to another class) or a SetMaritalStatus method wouldn't be the route I'd normally take.

I'd have a MaritalStausId property which is public get / set. On my WinForm (or MVC app) I'd bind that to a combobox. The ComboBox's DataSource will be the MaritalStatusList (a Csla NVL) with displaymember as the string description and the valuemember the Id. The ComboBox's SelectValue property would be databound to the Person's MaritalStatusId. No string MaritalStatus on Person at all, because its not needed to support the use case as the MaritalStatusList does that due to the way the ComboBox is setup.

That exact same setup can be done easily with Asp.Net MVC as well using the DropDownListFor HTML helper.

If you want to ensure that the martial status id set into the person is a valid value (defined as existing in your system), you'd have a business rule class that takes the MaritalStatusId property info, gets the value and ensures the value exists as a key in your MaritalStatusList.

Ensuring that a MaritalStatusId exists in your list is the behavior encapsulated by the business rule class (which you can reuse on any object which has a MartialStatusId), Person can fulfill its responsibility to save that status with the Person, and MartialStatusList serves as the single source of truth for the list of valid statuses. That's why I'm skeptical of #90 though; we services are great for serializing data. But how do you serialize behavior? Your Person class uses the ValidMaritalStatus class (a Csla PropertyRule subclass). Its not good enough to serialize the Person class' data, you'd need serialize the logic in ValidMaritalStatus (if I'm reading that ticket correctly).

Its interesting you bring up address, as I was going to write about that too. What you need to do again depends on your use case. Can your Person have one and only one address? If that's the case, I'd probably just have the properties AddressLine1, Line2, City, etc. right on person. Need to validate the address? A PropertyRule (or maybe ObjectRule) that takes all the propertyinfos making up the address, or perhaps create an IAddress interface and cast context.Target to that, and do the validation there. Need to send the address data to an online validator? Your rule calls another BO (probably a ValidAddressCommand) that does so.

If you have multiple addresses, a PersonAddresses (BusinessListBase) and PersonAddress. That same rule I mentioned above could be used still to valid each PersonAddress.

At the last place I worked at, you could create an account for an ecomm site. To register you had to provide your billing address. After you registered though you had an address book with as many addresses as you'd like to enter. You could mark which one was your billing and which was your default shipping. Only one of each though. But on registration, only the billing address was collected, so there (if we were using Csla) I'd just have the properties for the billing address, and no flag. It would be validated as I mentioned above and on save we'd ensure the IsBilling would be true (as you had to have a billing). Then there'd be separate classes for managing the address book (likely just a root PersonAddress, as there really wasn't a need to edit more than one at a time). There was also a StoreAddressInfo; it got its address data from the Address table in the DB (just like PersonData) but it was related to a store, not a person.

But it all really depends on your use cases; you pick what to reuse based on what behaviors the classes implement, not what data they have. You still only expose (publically) what you need to do allow the use case, and while you can get around exposing too many ids on a rich client, it will become very difficult to use such an object design doing an MVC app, or a WebApi. Even on a rich client, setting up the ComboBox for things that are restricted to a fixed list is easier as your BO doesn't need extra properties (MaritalStatus string on Person) just to display on the UI, which can be a pain. I think that's a more pragmatic way to go personally.

Sorry if this is rambling, there's just a lot of valid ways to do things and the best one to pick is really heavily influenced by your specific use cases.

oatsoda commented 8 years ago

Thanks @ajj3085 - apologies for the delay in replying :)

I agree that, pragmatically, dealing with the IDs appears to be simplest.

But using NVL feels dated. Why can't I bind my drop down to a BusinessBindingListBase?

If I do allow the assigning of relationships via an ID, but then have to validate that by a business rule that fetches all of the possible values then I would prefer that I don't allow setting via an ID - at least that way the consuming code that is setting the value will have to fetch the specific item with an ID before assigning the object into my business object.

I guess I am going to have to decide whether to take the plunge and do it in a manner that feels better modelled to me (and perhaps learn why it isn't a good idea the hard way :( ) or to take the pragmatic and leave my objects less "complete".

I do take your point about modelling behaviour - I have been striving to do this rather than trying to map to singular business objects. But I believe that this particular question is applicable to anywhere I try to make a relationship between objects.

ajj7060 commented 8 years ago

@oatsoda You can bind your dropdown to a ReadOnlyBindingListBase (I'm assuming the contents of the dropdown are not editable, you're just selecting one to set a value in another object). That will work just as well. The choice between NVL and ROBLB is whether or not you have (and need) more data that just an ID & text value. NVL is just a specialized version of ROBLB for the case when you just have the ID and value, so you don't need to write the ROB instance. So for MaritalStatus, that sounds like a great fit. For CountryList (if you're picking which country an address is in) you might using ROBLB if you have other data besides the country's ID and name, perhaps a postal code format or something additionally that you'd need.

BBLB would be odd as it imply that you can edit the contents of the dropdown list and pick them as well. Maybe you have a use case like that, but I've never come across this. Going with my country & address example, if you could select the country the address is in AND also add / remove countries available in the dropdown in the same use case and as part of the same unit of work, you would use BBLB for CountryList, but I would expect the list of countries to be managed separately than the editing of an address, probably in an admin application or section of the application.

But yes ultimately the choice is what you feel most comfortable with. Csla tends to be a pragmatic framework itself, but in a pure WinForms or WPF app using pure object relationships can be done, although I'm not sure if the binding can easily support that; maybe leaving ValueMember blank but setting a DisplayMember? I'm not sure, it's been a while since I've tried.