Hello,

I've got some code working but I'm not sure that the way I implemented it is
the best way. I have a custom collection class with a boolean property
HasChanged that tells me if any of the data has changed. An event is raised
on certain methods (Add, Remove, Insert, etc). Let me write a little code to
describe what I've done and see if there's room for improvement:

I want to represent a class that functions like this when instantiated.

Product P = new Product(productID);
P.Options.Name = "Gender";
P.Options.Values[0] = "Male";
P.Options.Values[1] = "Female;

I need to set events on both the custom "Options" collection as well as the
custom "Values" collection contained within Options so that I know if either
a new option has been added or if a value has been modified.

Here's my solution (which again, work just fine, looking for areas I can
improve on):

<< THERE ARE ONE OF THESE FOR ProductOptionsCollection and
ProductOptionValuesCollection. ONLY DISPLAYING ONE FOR BREVITY >>

namespace Sample
{
public delegate void ProductOptionChangedEventHandler(object sender,
EventArgs e);

public class ProductOptionCollection : CollectionBase
{
public event ProductOptionChangedEventHandler Changed;

private bool _HasChanged;

public bool HasChanged
{
get { return _HasChanged; }
set { _HasChanged = value; }
}

public int Add(ProductOption item)
{
OnChanged(new EventArgs());
return List.Add(item);
}

.. OTHERS REMOVED FOR BREVITY..

protected virtual void OnChanged(EventArgs e)
{
if (Changed != null)
Changed(this, e);
}
}
}


Then, within the calling class:

_Options.Changed += ProductOptions_OnChange;

IS THIS THE RIGHT WAY TO REGISTER FOR EVENTS WITHN A COLLECTION
OF A COLLECTION?
for (int i=0; i < _Options.Count; i++)
{
_Options[i].Values.Changed += ProductOptions_OnChange;
}

NOTICE I'M SETTING A BOOLEAN VALUE WITHIN THE CUSTOM COLLECTION
THAT RAISED THE EVENT IN THE FIRST PLACE -- I can't just set it within the
collection itself when Add is called because I need to programatically
register the event handler after the initial population of the collection.
Is there a better way to do this?
private void ProductOptions_OnChange(object sender, EventArgs e)
{
_Options.HasChanged = true;
}

All good points. I hammered out a couple of the example code lines as I was
in a rush out the door this morning. Yes, the first code portion (with the
indexers) is incorrect.

This is for a product inventory management system. I have products
(represented by the Product class) that can have various options. For
example, a golf club has

Product P = new Product();

Option O = new Option();
O.Name = "Shaft";
O.Values.Add("Graphite");
O.Values.Add("Steel");

P.Options.Add(O);

Option O1 = new Option();
O1.Name = "Gender";
O1.Values.Add = "Male";
O1.Values.Add = "Female"

P.Options.Add(O1);

So I can have a golf club with a graphite shaft made for a female.

When I create an instance of the class, the Options collection gets
populated in the constructor. What I need to know is, has the options
collection (or its Values sub-collection) changed since the object was
instantiated?

Since I'm adding products during initialization, I don't want the boolean
"HasChanged" value to be set there, so I'm wiring up the event handlers
after the collections are populated so I can detect any changes
post-initializaton.

If the Options.HasChanged flag is true when Product.Save() is called, I'll
persist those values back to the database, otherwise I'll leave them alone
and save the database call.

Sorry for being a little vague earlier, I was in a bit of a rush.

This is my first time exprimenting with some of this stuff, so forgive me if
my approaches or knowledge level leave a bit to be desired. That's why I'm
asking these questions in the first place.

"Peter Duniho" <(E-Mail Removed)> wrote in message
news(E-Mail Removed)...
> On Sat, 18 Oct 2008 07:01:53 -0700, Scott Stark <(E-Mail Removed)>
> wrote:
>
>> [...]
>> IS THIS THE RIGHT WAY TO REGISTER FOR EVENTS WITHN A
>> COLLECTION OF A COLLECTION?
>> for (int i=0; i < _Options.Count; i++)
>> {
>> _Options[i].Values.Changed += ProductOptions_OnChange;
>> }
>>
>> NOTICE I'M SETTING A BOOLEAN VALUE WITHIN THE CUSTOM
>> COLLECTION THAT RAISED THE EVENT IN THE FIRST PLACE -- I can't just set
>> it within the collection itself when Add is called because I need to
>> programatically register the event handler after the initial population
>> of the collection. Is there a better way to do this?
>
> It's impossible to say for sure, given that we have basically no details
> about the overall design. That said, some thoughts:
>
> -- Your subscription loop is not consistent with the code you posted
> first. That is, you are indexing "_Options" (whatever that is), but in
> the very first code example you gave, "Options" has no indexer. Probably
> the first code example was wrong, but inconsistencies create ambiguities,
> making it even harder to know what you're doing.
>
> -- I think the "HasChanged" property is superfluous and poor design.
> "HasChanged" since when? The "since when" is not defined by the class at
> all, especially if the class itself isn't maintaining the property. If
> you need to know this information, put the boolean somewhere else, in code
> where it can be clearly stated when the flag is set and reset. Even
> better, don't write code that relies on a state variable like that in the
> first place.
>
> -- You've declared a delegate type that is exactly the same signature
> as the EventHandler delegate type. You should just use EventHandler
> instead of making your own. Alternatively, make the event more useful by
> creating an EventArgs sub-class that will include information about the
> change (e.g. what option was added or changed), and then create a new
> delegate type based on _that_.
>
> -- The idea that an option might change after being added seems very
> odd to me, especially given the example you've provided ("Gender"). But
> granted, there's so little information about the bigger picture being
> provided here, it's hard to know for sure whether that's a valid
> criticism.
>
> Now, all that said, there is nothing fundamentally wrong with executing a
> loop that subscribes the same event handler to a particular event on a
> series of objects. So as far as your specific question goes, sure...that
> seems fine.
>
> Pete

Thanks.

I actually opted to go with your first suggestion. As I said, the events
work and it was a good experience for me just to practice creating and
wiring them up. I got it in my head that I needed to monitor the state of
something in another object changing so events just seemed the first choice
and I went with it.

Having said that, if I were to implement it as an event, would the method I
used be approrpriate or is there a more accepted practice to accomplish what
I was trying to do?

"Peter Duniho" <(E-Mail Removed)> wrote in message
news(E-Mail Removed)...
> On Sat, 18 Oct 2008 12:14:57 -0700, Scott Stark <(E-Mail Removed)>
> wrote:
>
>> [...]
>> When I create an instance of the class, the Options collection gets
>> populated in the constructor. What I need to know is, has the options
>> collection (or its Values sub-collection) changed since the object was
>> instantiated?
>>
>> Since I'm adding products during initialization, I don't want the
>> boolean "HasChanged" value to be set there, so I'm wiring up the event
>> handlers after the collections are populated so I can detect any changes
>> post-initializaton.
>
> I would advise one of two alternatives, both of which involving having the
> "changed" state updated exactly where the collections are modified rather
> than relying on events:
>
> -- simply clear the state after initialization in the constructor
> -- suppress changing of the state by including some private flag in
> the class set during initialization
>
> I think the first approach is the simplest, but if you had some specific
> reason for not even wanting the state to change during initialization, the
> second might be appropriate.
>
> The events seem like overkill to me.
>
> Pete

I just thought of something after I wrote my last post...

_Options.Changed += ProductOptions_OnChange;

for (int i=0; i < _Options.Count; i++)
{
_Options[i].Values.Changed += ProductOptions_OnChange;
}

private void ProductOptions_OnChange(object sender, EventArgs
e)
{
_Options.HasChanged = true;
}

With this code in mind, you can see that the same event handler
(ProductOptions_OnChange) is triggered when either the Options collection is
modified OR any of the Options.Values data is modified and flips the boolean
value of Options.OnChange to true.

So Options.HasChanged == true whenever ANY data within the collection or any
of it's sub-collections is modified. This means I only have to do a check:

if (Options.HasChanged) { //do stuff }

If I set a private boolean flag within each custom collection, when it comes
time to check for the state, I'd have to do this:

if (Options.HasChanged || someFunction(Options.Values)) { //do stuff }

private bool someFunction(ProductOptionValues POV)
{
// loop through the ProductOptionValues collection and check the
HasChanged value on all items in the collection returning true if ANY of
them are true
}

Would this change your mind on whether events are the best approach to use?
Since the ProductOptionValues collection has no intrinsic knowledge of the
Options class, I can't set that flag from within it and am forced to use a
more complicated check of the value.

"Peter Duniho" <(E-Mail Removed)> wrote in message
news(E-Mail Removed)...
> On Sat, 18 Oct 2008 12:14:57 -0700, Scott Stark <(E-Mail Removed)>
> wrote:
>
>> [...]
>> When I create an instance of the class, the Options collection gets
>> populated in the constructor. What I need to know is, has the options
>> collection (or its Values sub-collection) changed since the object was
>> instantiated?
>>
>> Since I'm adding products during initialization, I don't want the
>> boolean "HasChanged" value to be set there, so I'm wiring up the event
>> handlers after the collections are populated so I can detect any changes
>> post-initializaton.
>
> I would advise one of two alternatives, both of which involving having the
> "changed" state updated exactly where the collections are modified rather
> than relying on events:
>
> -- simply clear the state after initialization in the constructor
> -- suppress changing of the state by including some private flag in
> the class set during initialization
>
> I think the first approach is the simplest, but if you had some specific
> reason for not even wanting the state to change during initialization, the
> second might be appropriate.
>
> The events seem like overkill to me.
>
> Pete