AddEventHooks() Possibe Issue

AddEventHooks() Possibe Issue

Old forum URL: forums.lhotka.net/forums/t/8103.aspx


rxelizondo posted on Sunday, December 06, 2009

While looking at the LoadPropertyValue method under Csla.Core.BusinessBase, I noticed that the EventHooks() method was only being applied when the framework detected that the new property value was being updated with a different object.

This will work fine if the object is being set using the property get/set methods but what happens when the value is set as a default value from within the RegisterProperty() call? Well, it looks like this will not work as expected.

I created a little test project to illustrate the problem. The project is overly simplified but it should get the point across.

One more thing, I realize that what I am doing here may or may not be a good practice. That’s beond the pint right now. All I am trying to do is just to point out possible issues with the CSLA code.

Thank you.

** Note: I tried to fix/escape the lower-than and greater-than symbols but gave up…. Probably should have tried harder but did not have the patient to deal with that.

Here is the code:
-----------------------------------
using System;
using Csla;

namespace ConsoleApplication1
{
class Program
{
class Employee : BusinessBase
{
private static PropertyInfo JobProperty = RegisterProperty(o => o.EmployeeJob, null, new Job());
public Job EmployeeJob
{
get { return GetProperty(JobProperty); }
set { SetProperty(JobProperty, value); }
}
}

class Job : BusinessBase
{
private static PropertyInfo IDProperty = RegisterProperty(o => o.ID);
public string ID
{
get { return GetProperty(IDProperty); }
set { SetProperty(IDProperty, value); }
}
}

static void Main(string[] args)
{
//This does not work.
Employee emp = new Employee();
emp.ChildChanged += emp_ChildChanged;
emp.EmployeeJob.ID = "xyz";
Console.Read();

////This works.
//Job job = new Job();
//Employee emp = new Employee();
//emp.EmployeeJob = job;
//emp.ChildChanged += emp_ChildChanged;
//emp.EmployeeJob.ID = "xyz";
//Console.Read();
}

static void emp_ChildChanged(object sender, Csla.Core.ChildChangedEventArgs e)
{
Console.WriteLine("Employee class detected a property changed in Job class");
}
}
}

RockfordLhotka replied on Sunday, December 06, 2009

Actually what you are doing is illegal. Remember that RegisterProperty() is a static method, so that default value you create will be used on every single object ever created - providing the same child instance to multiple parents.

It is not legal for an object to have multiple parent objects - this will cause all sorts of problems (wrong Parent value, messed up edit levels, etc).

I'm not saying you didn't find a "bug", but even if we fixed that bug it would simply lead to another bug, and another and another.

Perhaps the real bug is that RegisterProperty() should throw an exception when you try to set a non-valuetype default value. And if this was a common issue I'd do that - but I can't recall anyone actually reporting an issue with this at all, so it isn't worth "fixing".

rxelizondo replied on Wednesday, December 09, 2009

RockfordLhotka:

Actually what you are doing is illegal. Remember that RegisterProperty() is a static method, so that default value you create will be used on every single object ever created - providing the same child instance to multiple parents.



Ok, I can understand that, but there is also the possibility for the Employee object to be a singleton class in which case, it would be totally legal right?


RockfordLhotka:

Perhaps the real bug is that RegisterProperty() should throw an exception when you try to set a non-valuetype default value. And if this was a common issue I'd do that - but I can't recall anyone actually reporting an issue with this at all, so it isn't worth "fixing".



Two things.

---------------------------------------
1) The Csla.Core.Business object has a method called "SetParent()" that is implemented as:

internal void SetParent(Core.IParent parent)
{
_parent = parent;
}

for what I can see, all parents (BusinessBaes or BusinessListBase) will call this method to set themselves as the parents of the object. So in order to avoid this and other issues would it not be possible to throw an error in that method if the _parent is not null

internal void SetParent(Core.IParent parent)
{
if(_parent != null)
throw new InvalidOperationException("Object can't belong to more than one parent")
_parent = parent;
}


---------------------------------------
2) This one kind of troubles me a bit. The so it isn't worth>"fixing" part.

To me everything is worth fixing, I mean, why not?

Whats the problem? There's probably a bunch of people that are eager to contribute to the development of the CSLA, I know I am one of them... I mean, I would pay money to be able to be more than just a user of the CSLA. It would be really gratifying to see some of my code land on the CSLA...... ok, perhaps I am not worthy at this time but I am sure others are.

So rather than not fixing the issue, have you consider doing something to allow people to help you out with the core csla? is this being done at this time (other than the VB effort)?

Thanks.

tmg4340 replied on Wednesday, December 09, 2009

rxelizondo:
2) This one kind of troubles me a bit. The so it isn't worth>"fixing" part. To me everything is worth fixing, I mean, why not? Whats the problem? There's probably a bunch of people that are eager to contribute to the development of the CSLA, I know I am one of them... I mean, I would pay money to be able to be more than just a user of the CSLA. It would be really gratifying to see some of my code land on the CSLA...... ok, perhaps I am not worthy at this time but I am sure others are. So rather than not fixing the issue, have you consider doing something to allow people to help you out with the core csla? is this being done at this time (other than the VB effort)? Thanks.

There is a contributor agreement you can sign with Rocky which opens up opportunities to assist in maintaining CSLA.  I haven't seen it, so I don't know what's involved.

Having said that, there most certainly are bugs that aren't worth fixing.  I've found this blog entry to be pretty good at explaining my perspective on it:

http://www.joelonsoftware.com/articles/fog0000000014.html

In this case, the blog author is dealing with commercial software, which lives in a slightly different world than CSLA.  But I think the same principles still apply.  Basically, from this perspective, fixing a bug isn't worth it if it takes more time to fix than it saves.  Given the situation that Rocky has described - i.e. this is a "bug" that no one has encountered - there's nothing on the "time saved" column to balance out the "time to fix" column.  He also mentioned that tackling this issue opens up a whole other set of potential issues, which add even more time to the "time to fix" column...

(Having said that, if you read the article, you'll see that he did end up fixing every bug in his system.  "Cost" is sometimes a relative term... Smile [:)])

I also know that Rocky has his own hierarchy of how he determines what bugs he fixes, which has been laid out in posts before.  It doesn't follow the article I linked to, but I see elements of the article in his hierarchy.

I'm not trying to speak for Rocky here - just offering my opinion.

HTH

- Scott

RockfordLhotka replied on Wednesday, December 09, 2009

Triage of bugs is basic software engineering. Any sizable software effort ends up with decisions made about what to fix and not to fix based on a variety of factors.

Not meaning offense, but it is naive to think that everything can be fixed - there are many costs to changing software (real cost, time, impact on existing users, opportunity cost, etc). Regardless of whether the change is a bug or a new feature, these costs exist.

It is true that I accept some help in maintaining CSLA. Generally the help is limited, though there are a small number of people I've been working with (sometimes off and on) for years, where I have a high level of trust that they share my view on software development and their impact has sometimes been quite high. Developing that level of trust takes a long time, but I really appreciate the contributions of the people who've stuck with me.

My view on CSLA contributions is not unlike Torvald's view on Linux - this is a "benevolent dictatorship" :)

rxelizondo replied on Wednesday, December 09, 2009

Thanks for responding Rocky.

No offence taken!

It’s not that I am being naïve, in fact, it’s somewhat ironic that today I found a “bug” in one of the applications I built. It’s one of those issues that fall in that infamous gray area of “Is this a bug or a feature” type of thing.

When the users approached me with the issue, my response was, “This behavior is by design” … pretty original isn’t it :)

I also told them that the best way to avoid the issue is not to do what they were doing… that was pretty original too wasn’t it :)

The fact is that it wasn’t worth fixing the “bug” because it was a corner case and everyone agreed with me (including the users).

There are a couple of differences between my app and the CSLA though:

1) My app is used by a miserly 30 people all sitting in the same building, whereas the CSLA has a LOT more users than that.

2) My app does not have the potential of literally changing people’s lives. I know that the CSLA changed my life. The principles and practices that I have learned from using it have opened many doors for me (and I am just an idiot).

3) I don’t have a group of contributors that could help me make that change, I either make the change myself or it does not get done.

So you see, its not that I don’t get it, it’s just that when it comes to software such as the CSLA I picture you as having a “the greater good” type of philosophy/attitude, the “balls to the wall” sort of way of thinking.

On a more selfish note, users of the CSLA have a vested interest in seeing the CSLA get adopted by more and more developers. It would be a pity if the CSLA lost the framework fight and we saw it go bye-bye in favor of some other framework. So the “no bugs” / “clean code” statements are big sellers to someone deciding between frameworks. The more CSLA adopters the better, the CSLA is a marketable skill if it’s well known.

Anyway, that’s just my way of thinking.

PS: Special thanks to the person editing this response and fixing the 50 spelling/grammar errors that I had. Simply put, I am never going to get English.

RockfordLhotka replied on Wednesday, December 09, 2009

What's ironic is that, as you might expect, CSLA was much cleaner and tighter when

  1. It was smaller, so I was able to discuss literally every part in detail in the Expert Business Objects book
  2. I was the only developer

The first point is a big one, because you must have amazingly clean code if you are going to use it as a tutorial in a book. But if I restricted the size of the framework to what I could completely discuss in the book the framework would have become irrelevant to most people several years ago.

The second point is just as important. Two developers never share exactly the same sensibilities, and there are parts of the framework now that I didn't implement - and so they are somewhat in the same style, but aren't completely in the same style. The more developers that become involved, the less consistency (and thus in a sense the less clean) the code becomes.

In my observation over the years, that's just the way software works. Barring some sort of Vulcan mind meld, everyone has a different take on things and that impacts (negatively) consistency and so forth.

Which is not a negative reflection on any contributors - there's some excellent code in the framework from some very smart people - but it isn't always quite the way I'd have done it :)

In any case, I fully appreciate that CSLA impacts a lot of people and organizations in some very deep ways. That's quite humbling.

And because of that reality, the need to prioritize changes and fixes based on broad impact is all the more important. Fixing things that no one has ever encountered in the wild falls at the absolute bottom of the priority list in most cases.

ajj3085 replied on Thursday, December 10, 2009

rxelizondo:
On a more selfish note, users of the CSLA have a vested interest in seeing the CSLA get adopted by more and more developers. It would be a pity if the CSLA lost the framework fight and we saw it go bye-bye in favor of some other framework. So the “no bugs” / “clean code” statements are big sellers to someone deciding between frameworks. The more CSLA adopters the better, the CSLA is a marketable skill if it’s well known.

I'm curious.. what frameworks do you feel compete with Csla?  To my knowledge, there are none, Csla fills a niche seemingly ignored by everyone else.  DTO mapping is well covered, UI is well covered.... but business rules?  The only things close are external rules engines, or something like WF.  But these don't seem to be aimed at building a library to address particular use cases as much as they are a way to let "normal" people build "applications."

rxelizondo replied on Thursday, December 10, 2009

ajj3085:

I'm curious.. what frameworks do you feel compete with Csla?  To my knowledge, there are none, Csla fills a niche seemingly ignored by everyone else.  DTO mapping is well covered, UI is well covered.... but business rules?  The only things close are external rules engines, or something like WF.  But these don't seem to be aimed at building a library to address particular use cases as much as they are a way to let "normal" people build "applications."



Any framework out there is a competitor of the CSLA framework no matter how big/small or famous/unknown the other frameworks are, each framework has their pros and cons, that is why they exist.

And just to be clear, I realize that some will claim that the CSLA is not at war against other frameworks and I would completely agree with such statement. I was just approaching this form the “marketable skill” point of view. The more famous/successful the CSLA is the more marketable you are. Especially since frameworks are critical to the development of any serious applications.

ajj3085 replied on Thursday, December 10, 2009

rxelizondo:
Any framework out there is a competitor of the CSLA framework no matter how big/small or famous/unknown the other frameworks are, each framework has their pros and cons, that is why they exist.

That's a fair enough statement, but like I said, I'm not really sure there ARE other frameworks out there that tackle business objects.  I've looked, and have yet to find one that fits the same niche.

rxelizondo:
I was just approaching this form the “marketable skill” point of view. The more famous/successful the CSLA is the more marketable you are. Especially since frameworks are critical to the development of any serious applications.

I agree; of course, I've also had interviewers ask "what's Csla," and I give them an executive summary and explain why it's useful.  So even if potential employers don't know what it is, it still helps you be marketable.  Smile [:)]

Copyright (c) Marimer LLC