Re: A tiny little integer stack package from a novice.

[Jeffrey Carter <jrcarter@acm.org>]

Stapler wrote:
> 
> As a newbie, I would appreciate any critiques the more experienced among
> you might have to offer. Although I call it a stack, it's more like a
> fixed buffer, a psuedo-stack if you will.

It looks exactly like a stack to me.

This is an example of a package-as-object implementation. There's 
nothing wrong with this approach, but it's often easier for the client 
to use an ADT.

> 
> with Ada.Unchecked_Deallocation;
> generic
> 
> 	Size : Positive;
> 		
> package int_stack is
> 	
> 	type Int_Stack is limited private;
> 	type Stack_Ptr is limited private;

Why are these visible to the client?

> 	
> 	procedure Push(X : in Integer);
> 	procedure Free_Stack;

Why is Free_Stack visible to the client?

> 	
> 	function Pop return Integer;
> 	
> private
> 
> 	
> 	type Int_Stack is array(1..Size) of Integer;
> 	type Stack_Ptr is access all Int_Stack;
> 	The_Stack : Stack_Ptr := new Int_Stack;	-- Provides access for De-allocation of entire stack
> 						-- Is also the sole allocation point for this package.
> 	procedure Free is new Ada.Unchecked_Deallocation(Int_Stack, Stack_Ptr);

Why are these here rather than in the body?

> 	
> end int_stack;
> 	
> 
> 
> package body int_stack is 
> 	
> 	Counter : Positive := 1;
> 	
> 	procedure Push(X : in Integer) is
> 	
> 	begin
> 	
> 		The_Stack.all(Counter) := X;
> 		
> 		Counter := Counter + 1;
> 		
> 	end Push;

What happens if Counter > Size?

> 	
> 	procedure Free_Stack is
> 	
> 	begin
> 	
> 		Free(The_Stack); -- I'm learning to make the commands 
> 						 -- self-explanatory here. Heh.
> 		
> 	end Free_Stack;
> 		
> 	function Pop return Integer is
> 	
> 		X : Integer := 0;
> 		
> 	begin
> 		Counter := Counter - 1; -- The Counter op is placed here because
> 								-- the Counter variable will point to 1 element
> 								-- past the end of the array on the first
> 								-- call.
> 		X := The_Stack(Counter);

What happens if Counter < 1?

> 		
> 		return X;
> 		
> 	end Pop;
> 	
> end int_stack;

There doesn't seem to be any need for access types for a bounded stack.

You might want to add such operations as Is_Empty, Is_Full, and Length 
to the package.

I suggest you consider how you could implement the following 2 packages:

generic -- Int_Stack
    Size : Positive;
package Int_Stack is
    Overflow : Exception;

    procedure Push (Value : in Integer);
    -- Adds Value to the top of the stack
    -- Raises Overflow if the stack contains Size values

    Underflow : Exception;

    function Pop return Integer;
    -- Removes the top value from the stack and returns it
    -- Raises Underflow if the stack is empty
end Int_Stack; -- No private part

package Int_Stack is
    type Stack (Size : Positive) is limited private;

    Overflow : exception;

    procedure Push (Onto : in out Stack; Value : in Integer);
    -- Adds Value to the top of Onto
    -- Raises Overflow if Onto contains Onto.Size values

    Underflow : exception;

    procedure Pop (From : in out Stack; Value : out Integer);
    -- Removes the top value from From and assigns it to Value
    -- Raises Underflow if From is empty
private -- Int_Stack
    type Int_Array is array (Positive range <>) of Integer;

    type Stack (Size : Positive) is limited record
       Top   : Natural := 0;
       Value : Int_Array;
    end record;
end Int_Stack;

-- 
Jeff Carter
"Death awaits you all, with nasty, big, pointy teeth!"
Monty Python & the Holy Grail