[Mono-list] System.Collections.Stack

[David Menestrina]

Here are my comments.  Take them with a grain of
salt because I've only been programming in C# for
a couple days.  (But I do know Java, so that helps.)

- I think you're totally right that Stack can return
  "this" for SyncRoot.  Like you said, nobody will
  ever see the internal Array.

- Maybe the whole thing should be implemented using
  ArrayList.

- In Resize(), it might be faster to use   
  System.Array.Copy() instead of a loop.

- There are a couple of things in SyncStack that
  probably don't need to be locked.  For example:

  >   public override int Count {
  >     get { lock(this) { return count; } }
  >   }

  Locking here is unnecessary because reading from
  an int should be atomic. (It is in Java...)

- What happens if you do something like this:
  Stack s1 = new Stack();
  Stack s2 = Stack.Synchronized(s1);
  s1.push(1);
  s2.pop();

  Should s2 be able to pop the element that was pushed
  into s1?  I think the MS implementation supports   
  this.  (Guessing from playing with ArrayList.)

- Here's one way to solve the above problem, if 
  indeed it is a problem.  Have SyncStack be a pure 
  wrapper class, that has one extra member, which is a
  Stack.
  
  All the methods are overridden to call the 
  same method on m_insideStack.  The member variables
  inherited from Stack are never used, which sucks,
  but I don't know how to get around that.

  Other than the fact that it solves the above
problem,
  it has the benefit that you don't need member
  variables for readonly and synchronized.  You can
  just have the base class return false for both, then
  have the wrappers override it to return true.

  SyncRoot, then would return
m_insideStack.SyncRoot().

Hope these comments are helpful,
david

__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail
http://personal.mail.yahoo.com/