[Mono-list] System.Collections.Stack

[Garrett Rooney]

On Mon, Jul 16, 2001 at 11:59:29AM -0700, David Menestrina wrote:
> 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.

ok, cool.

> - Maybe the whole thing should be implemented using
>   ArrayList.

that might be easier.  i'll look at it.

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

true, i hadn't discovered System.Array.Copy() when i wrote that part.

> - 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...)

I wasn't sure, so I was playing it safe.  In retrospect, I imagine you're
correct.

> - 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.)

I'll play around and see if it works that way.  If so, your solution seems to
be the best way to handle it.

> - 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,

Very much so.  Thanks.  An updated version will be available soon.

-- 
garrett rooney                     Unix was not designed to stop you from 
rooneg@electricjellyfish.net       doing stupid things, because that would  
http://electricjellyfish.net/      stop you from doing clever things.