Re: collections or methods? - C# Discussion Boards

15-Apr-04 9:39

A co-developer and I were in a discussion today about the right way to do this, I'd like some outside opinions on the subject.

Let's say we have a control that contains children of type Foo. Each child Foo can also contain children of type Foo.

To model the following, here's how I believe it should be modeled:

//////////// example #1, my solution ///////////////////

public class Foo
{

   private FooCollection children = new FooCollection();
   
   public Foo()
   {
      children.ItemAdded += SomeDelegate(this.ChildAddedCallback);
   }
   
   private void ChildAddedCallback(Foo addedChild)
   {
   	// do something special
   }
      
   public FooCollection Children
   {
      get
      {
      	return this.children;
      }
   }
}

<pre>public class FooCollection
{
   public event SomeDelegate ItemAdded;
   
   public FooCollection()
   {
   }
   
   public void Add(Foo item)
   {
   	if(this.ItemAdded != null)
   	   this.ItemAdded(item);
   }
}

// adding a child to the control
FooBarControl.Children.Add(foo);

// adding a child to the child
foo.Children.Add(anotherFoo);

The developer I was discussing this with disagreed with the above code. He instead believes it should look something like:

//////////// example #2, the other developer's solution ////////////////

public class Foo
{
   private FooCollection internalChildren = new FooCollection();
   
   public Foo()
   {
   }
   
   public void AddChild(Foo child)
   {
      internalChildren.Add(child);
      
      // do something special
   }
   
   public FooCollection GetChildren()
   {
   	FooCollection externalChildren = new FooCollection();
   	
   	foreach(Foo child in internalChildren)
   	   externalChildren.Add(child);
   	   
   	return externalChildren;
   }
}

<pre>public class FooCollection
{
   public class FooCollection()
   {
   }
   
   public void Add(Foo item)
   {
   }
}

// adding a child to the control
FooBarControl.AddChild(foo);

// adding a child to the child
foo.AddChild(anotherFoo); // note that Foo.AddChild is a different method than FooBarControl.AddChild

His argument was that the above code is object-oriented, and all the functionality for adding a child is contained in a single class.
His main argument was that my code (sample #1) would allow external objects to invalidate the inner workings of Foo children. With his solution (example #2), the inner workings of Foo would be completely concealed. His solution would protect the inner workings and inner data of the Foo children. His solution also gives more of a de-coupling of the collection-to-Foo.

My argument was that my solution (example #1) follows that of the .NET FCL (look at TreeView w/ TreeNode for example, or any other control with children/items). I also argued that my solution is truely object oriented, using a seperate object to store and manipulate the children.
Additionally, my solution would allow for code reuse, in that the main FooBar control containing Foo items can also utilize the collection, without the need to write .AddChild methods for both the FooBar control and Foo item.

So, I'd like to hear from you all on how you guys would implement such a structure. Is example #1 or example #2 closer to what you would do/recommend?

---------------------------
He who knows that enough is enough will always have enough.

-Lao Tsu

Heath Stewart15-Apr-04 9:59

15-Apr-04 9:59

He's making a case for encapsulation of the collection, which does give you better control. But ask yourself, do you need that much control over callers? Very few classes within the FCL encapsulate their lists and collections like this and instead expose them as you were doing: as properties. Yes, a caller could add, remove, and even clear your collection but decent code would accomodate that. That's the question you really have to answer in this case.

Another question is whether or not you want callers to be able to enumerate your collection. If not, then encapsulation is the way to go; otherwise, why waste the time?

Microsoft MVP, Visual C#
My Articles

Judah Gabriel Himango15-Apr-04 10:22

15-Apr-04 10:22

Thanks for the response Heath.

We certainly do want to be able to iterate over and enumerate the Foo items of the FooBar control. In fact, there are many methods in our main app that already do this, so that is a definite must.

Heath Stewart wrote:
Yes, a caller could add, remove, and even clear your collection but decent code would accomodate that.

Right. Our FooCollection already has methods for clearing, insert, remove, add, etc. and handles them all. As best as I can tell, there is no way for an external caller to break the control or corrupt the internal children via manipulation of the FooCollection.

As far as encapsulation goes, heck aren't we already doing some encapsulation via get/set accessors? I always thought that's one of the main uses of properties. Confused | :confused:

Anyway, with that in mind, and given the fact that Microsoft seems to expose collections as properties throughout the entire framework class library, I think I'll stick with my initial reaction, unless someone can point out how to break my code via access to the collection. (w/o using reflection of course!) :P

---------------------------
He who knows that enough is enough will always have enough.

-Lao Tsu

Heath Stewart15-Apr-04 10:51

15-Apr-04 10:51

It depends what you mean by "breaking" your collection. For instance, I could reassign an object in your collection to another object. This - like many other things - comes down to a matter of trust. Do you trust your callers? If not, use code access security so that only your assemblies can call them, while all others get a SecurityException.

Microsoft MVP, Visual C#
My Articles

Judah Gabriel Himango15-Apr-04 11:23

15-Apr-04 11:23

By breaking it, I meant manipulating the collection in such a way as to cause the collection to either throw an exception, display incorrect data, function improperly, open a security hole, etc. You bring up an interesting point in reassignment, that's something I didn't consider.

I suppose as long as FooCollection stores only Foo types, you'd only be able to reassign a Foo instance to another Foo instance. That is not a problem, since there is already checking going on to ensure each Foo can belong to only one FooBar control or other Foo item at a time. However, assigning a new Foo instance to an existing Foo in the FooCollection might not display correctly until the FooBar is repainted. I noticed the Sys.Win.Form.TreeView has such a case (bug?) in which the following code makes 2 nodes show up:

this.Show();
TreeView v = new TreeView();
this.Controls.Add(v);

v.Nodes.Add("hello");
	
TreeNodeCollection tnc = v.Nodes;
tnc[0] = new TreeNode("hello again");
MessageBox.Show(tnc.Count.ToString()); // outputs 1, despite 2 nodes showing up in the tree

On the security side, currently there are no plans to use FooBar and Foo outside the main application. The only caller will be our main application. Still, it might be a good idea to eventually use CAS to ensure only fully trusted callers can manipulate the collection in the event other callers will use it.

Thanks again Heath for the response, you've been quite helpful as always.

---------------------------
He who knows that enough is enough will always have enough.

-Lao Tsu

Steven Campbell15-Apr-04 11:17

Steven Campbell

15-Apr-04 11:17

There is a paper that discusses this subject, at Martin Fowler's website. See

http://martinfowler.com/ieeeSoftware/dataAccessRoutines.pdf

Judah Gabriel Himango15-Apr-04 16:41

15-Apr-04 16:41

Interesting paper indeed, thanks for the resource.

---------------------------
He who knows that enough is enough will always have enough.

-Lao Tsu

scadaguy15-Apr-04 11:42

scadaguy

15-Apr-04 11:42

I like your solution better.

His argument about that the coupling is incorrect. His solution is no more or less coupled than yours. Your solution breaks the coupling by using events which is a version of the observer design pattern developed to break the coupling of two classes in situations like this.

You can tweak you solution to make the inner workings of the Foo class just as concealed as his by creating a private nested subclass of FooCollection inside Foo. You can then override the Add method to do the necessary validation. This would also eliminate the need for the event as well and results in a very elegant design.

Aside from the nonstanard interface of his solution, which is awkward to me, the real problem I have with his code is the O(n) operation required to return a reference to the child collection.

Judah Gabriel Himango15-Apr-04 16:53

15-Apr-04 16:53

His main argument is that my solution exposes the inner workings of the children, allowing for the possibility of breaking the whole thing, which is something I've discussed with Heath above. It is a valid point if I allow assignment of items from the collection.

I totally agree as far as the collection returning goes. That's gotta be terrible for performance for a tree with thousands of Foo items in it.

Good thinking on the subclass, I'd then be able to access the private methods of the parent from the subclass, which would eliminate the need for events as you mentioned.

Thanks for your input Brian.

---------------------------
He who knows that enough is enough will always have enough.

-Lao Tsu

bjoernen24-Apr-04 23:06

bjoernen

24-Apr-04 23:06

I prefer your solution (#1).

I don't buy the argument that external objects could break the inner workings of Foo children. Foo is a class that happens to have property named Children. It's up that object (Children) to maintain it's own integrity, just as any other property of Foo. Foo's responsibility is to make sure all it's property values make sense as a whole, not that they make sense within themselves. If the Foo class is interested in knowing about special events in the Children object, it can listen to them using an eventhandler, just as you have done in your design.

I would say #2 is a bad design, because Foo must know too much about the Children object type (class). That's the FooCollection's responsibilities, and can be easily done with indexers, properties and events. If your friend has a problem with your solution not being OO enough, I would say his solution is less OO.

Is there a special circumstance in that the Children object is of type FooCollection, making this a tree structure? Really not. I think the same design patterns should apply.

A sligthly better solution would be that FooCollection contains objects that implement the IFoo interface. Possibly overkill, but could be useful.

Regards,
/Björn Morén

Hotkeys

rickrf15-Apr-04 7:43

rickrf

15-Apr-04 7:43

I am using c# to create web forms in .net and am trying to add hotkeys. I have been unsuccessful in locating any reference material. If anyone knows of any good reference sights on this subject it would be greatly appreciated or if it is a simple process and explanation would be even better.

Thanks,
Rick

Tom Clement15-Apr-04 7:51

Tom Clement

15-Apr-04 7:51

Here's some sample code for you.

private void MyControl_KeyDown(object sender, System.Windows.Forms.KeyEventArgs e)
{
  switch (e.KeyCode)
  {
    case Keys.Insert:
      if (e.Modifiers == Keys.Shift)
      {
           // Do a shift Insert action.
          e.Handled = true;
      }
      return;
    case Keys.Enter:
      // Do an Enter Action.
      e.Handled = true;
      return;
    case Keys.Up:
      if (e.Modifiers == Keys.Shift)
      {
        if (CanMoveItemUp)
        {
	  MoveItemUp();
	  e.Handled = true;
        }
      }
      return;
    default:
      //all other keys
      return;
  }
}

Tom Clement
Apptero, Inc.

Heath Stewart15-Apr-04 8:41

15-Apr-04 8:41

You can also implenent hotkeys in your menus easily by simply assigning the MenuItem.Shortcut property to the key you want.

For global hotkeys in your application, see the IMessageFilter interface. Implement that and catch the WM_KEYDOWN message (0x0100), then use the Message.WParam member to get which key was pressed (matches up with the Keys enumeration). Using this concept, hotkeys are available throughout your application (if you desire) and you could even implement a simple key mapping to make them easy to change (for example, from a user interface for the benefit of a user). This is common in games, for example, like Quake that allow you to bind functions to practically any key you want.

You can add your implementation at any time using Application.AddMessageFilter.

Microsoft MVP, Visual C#
My Articles

Mazdak15-Apr-04 9:24

Mazdak

15-Apr-04 9:24

You and other CPains didn't consider that original poster want to do that in a WEB form. Smile | :)

Mazy

"I think that only daring speculation can lead us further and not accumulation of facts." - Albert Einstein

Heath Stewart15-Apr-04 9:25

15-Apr-04 9:25

True. Far be it for us to assume that someone would use the right forums. Roll eyes | :rolleyes:

Microsoft MVP, Visual C#
My Articles

rickrf16-Apr-04 4:36

rickrf

16-Apr-04 4:36

Mazy is correct I am talking about a Web application not a Window application.

rickrf16-Apr-04 5:16

rickrf

16-Apr-04 5:16

I would like to thank everyone for their input. I was able to work out my Hotkey issue. If anyone is interested, here is an example of a button control that is executed by Alt + Q

<asp:button id="Button2" style="Z-INDEX: 103; LEFT: 316px; POSITION: absolute; TOP: 166px" accesskey="Q" runat="server" text="Button">

Rick

C#: Question referring Generics

Bee Master16-Apr-04 17:46

Bee Master

16-Apr-04 17:46

You should use AccessKey property to assign hotkey if you are using asp button control. This way you don't have to code anything.

Cheers

Never comment ur code. If it was hard to write, it should be hard to understand !!!

WinForms bug?

Tom Clement15-Apr-04 7:02

Tom Clement

15-Apr-04 7:02

I sort of hesitate to say this, but I think we've run across a Winforms bug. I suppose the alternative is that I have a brain bug. Anyway, here's the situation:

Create a trivial application (Form1) with a button that launches a modeless dialog using:

Form2 frm = new Form2();
frm.Owner = this;
frm.Show();

In Form2, a button launches Form3 modally like this:

Form2 frm = new Form3();
frm.ShowDialog();

another button hides Form2 like this:

this.Hide();

Now, if you run the application (Form1), then switch focus to some other application (e.g. Outlook). Then click the button that launches Form2 (modeless), then click the button that launches Form3 (modal). Close Form3, and Hide Form2. Form1 goes into the background and Outlook gets focus.

Does anyone know what underlying mechanism is happening here?
Thanks as always for your help.

Tom Clement
Apptero, Inc.

SJ_Phoenix15-Apr-04 6:46

SJ_Phoenix

15-Apr-04 6:46

if I have a generic class like this for example:

public class GenericExample<T>
{
public GenericExample(T parameter) {}

//...
}

when I instantiate it then:

GenericExample<string> ge1 = new GenericExample<string>(String.Empty);

GenericExample<SolidBrush> ge2 = new GenericExample<SolidBrush>(null);

so here is my question:

how can I verify, whether parameter is null,
or whether it is string.Empty, although I don't know,
if T is an value-Type or reference-type. (yes...I know string is a reference type but I had no other example in mind Wink | ;-)

Re: C#: Question referring Generics

Heath Stewart15-Apr-04 6:55

Re: C#: Question referring Generics

15-Apr-04 6:55

[EDIT: Note, this was posted while the < and > brackets were not appearing in the root thread. I leave it here for posterity.]

This is not a template class, which would look like:

public class GenericExmaple<T>
{
  public GenericExample(T parameter) {}
  // ...
}

And would be declared as a variable like so:

GenericExample<string> ge = new GenericExample<string>(null);

If you want a class that takes a "generic" parameter, use object:

public class GenericExample
{
  public GenericExample(object parameter)
  {
    if (parameter == null) throw new ArgumentNullException("parameter");
  }
}

When you define the parameter as an object, even if it's a value type it will be boxed as an object and can be compared to null, although it will never be null since value types can't be null. A reference could and thus is subject to the exception if it is null.

Microsoft MVP, Visual C#
My Articles

Heath Stewart15-Apr-04 7:02