Int_Stack rewritten.

Int_Stack rewritten.

[Stapler]

As per the suggestions, I've rewritten my simple Int_Stack package. Hope
it looks better.

package int_stack is
	
	pragma Pure(int_stack);
	
	Stack_Overflow : Exception;
	Stack_Underflow : Exception;
	
	-- As far as I can tell these procedures are squared away.
	-- 12/02/02
	type Int_Stack(Size : Natural) is limited private;
	procedure Push(The_Stack : in out Int_Stack; X : in Integer);
	procedure Pop(The_Stack : in out Int_Stack; X : out Integer);
	
	-- These functions and procedures are new and untested.
	function Is_Empty(The_Stack : in Int_Stack) return Boolean;
	function Is_Full(The_Stack : in Int_Stack) return Boolean;
	function Length_Of(The_Stack : in Int_Stack) return Natural;
	function Sum_Of(The_Stack : in Int_Stack) return Integer;
	-- I will add more functions and procedures as soon as I'm sure
	-- this basic package isn't hosed.
	
private
	
	type Integer_Array is array(Positive range <>) of Integer;

	type Int_Stack(Size : Natural) is 
	limited record
		Elements : Integer_array(1..Size);
		Top : Natural := 1;
	end record;
	
	
end int_stack;


package body int_stack is
	
	procedure Push(The_Stack : in out Int_Stack; X : in Integer) is
		
	begin
		if The_Stack.Top > The_Stack.Size then
			raise Stack_Overflow;
		end if;
		
		The_Stack.Elements(The_Stack.Top) := X;
		
		if The_Stack.Top < The_Stack.Size then
			The_Stack.Top := The_Stack.Top + 1;
		end if;
	
	end Push;
	
	procedure Pop(The_Stack : in out Int_Stack; X : out Integer) is
		
	begin
		If The_Stack.Top < 1 then
			raise Stack_Underflow;
		end if;
		
		X := The_Stack.Elements(The_Stack.Top);
		
		if The_Stack.Top > 1 then
			The_Stack.Top := The_Stack.Top - 1;
		end if;
		
		
	end Pop;
	
	function Is_Empty(The_Stack : in Int_stack) return Boolean is
	
	begin
	
		if The_Stack.Top = The_Stack.Elements'First then
			return True;
		else
			return False;
			
		end if;
		
	end Is_Empty;
	
	function Is_Full(The_Stack : in Int_Stack) return Boolean is
	
	begin
	
		if The_Stack.Top = The_Stack.Elements'Last then
			
			return True;
		else 
			return False;
		
		end if;
		
	end Is_Full;
	
	function Length_Of(The_Stack : in Int_Stack) return Natural is
	
		Length : Natural;
	
	begin
		-- This entire function seems somewhat rendundant
		-- considering the "Length" attribute. But it's
		-- being included as suggested
		
		Length := The_Stack.Elements'Length;
		
		return Length;
		
	end Length_Of;		

	function Sum_Of(The_Stack : in Int_Stack) return Integer is
		
		The_Sum : Integer := 0;
	
	begin
	
		for J in 1..The_Stack.Top loop
		
			The_Sum := The_Sum + The_Stack.Elements(J);
			
		end loop;
		
		return The_Sum;
	
	end Sum_Of;

end int_stack;

I hope this one looks better.

So, have I improved somewhat? Heh.

Thanks for your patience, and try not to laugh to hard. Heh.


Stapler

Re: Int_Stack rewritten.

[Björn Lundin]

Stapler wrote:

>     
>                 if The_Stack.Top = The_Stack.Elements'First then
>                         return True;
>                 else
>                         return False;
>                         
>                 end if;

It's a matter of taste, but the above lines could be written as
   return (The_Stack.Top = The_Stack.Elements'First);

Then again, the original migth be easier to read...
/Bj�rn
  

Re: Int_Stack rewritten.

[Jeffrey Carter]

Stapler wrote:
> 
> package int_stack is
> 	
> 	pragma Pure(int_stack);
> 	
> 	Stack_Overflow : Exception;
> 	Stack_Underflow : Exception;
> 	
> 	-- As far as I can tell these procedures are squared away.
> 	-- 12/02/02
> 	type Int_Stack(Size : Natural) is limited private;

Some vertical whitespace here would make this type declaration easier to 
see. Also, you might find it easier to understand things if your package 
and type don't have the same name. I'd suggest Stack_Info for the type, 
and Stack for the parameters of the type (see below for more about this).

> 	procedure Push(The_Stack : in out Int_Stack; X : in Integer);
> 	procedure Pop(The_Stack : in out Int_Stack; X : out Integer);
> 	
> 	-- These functions and procedures are new and untested.
> 	function Is_Empty(The_Stack : in Int_Stack) return Boolean;
> 	function Is_Full(The_Stack : in Int_Stack) return Boolean;
> 	function Length_Of(The_Stack : in Int_Stack) return Natural;
> 	function Sum_Of(The_Stack : in Int_Stack) return Integer;
> 	-- I will add more functions and procedures as soon as I'm sure
> 	-- this basic package isn't hosed.
> 	
> private
> 	
> 	type Integer_Array is array(Positive range <>) of Integer;
> 
> 	type Int_Stack(Size : Natural) is 
> 	limited record
> 		Elements : Integer_array(1..Size);
> 		Top : Natural := 1;

It's customary to use a Top of zero to indicate an empty stack. You 
might find it simplifies some things.

> 	end record;
> 	
> 	
> end int_stack;
> 
> 
> package body int_stack is
> 	
> 	procedure Push(The_Stack : in out Int_Stack; X : in Integer) is
> 		
> 	begin
> 		if The_Stack.Top > The_Stack.Size then
> 			raise Stack_Overflow;
> 		end if;
> 		
> 		The_Stack.Elements(The_Stack.Top) := X;
> 		
> 		if The_Stack.Top < The_Stack.Size then
> 			The_Stack.Top := The_Stack.Top + 1;
> 		end if;

Because of this if, you will never raise Stack_Overflow. You will just 
keep replacing the top element when the stack is full.

> 	
> 	end Push;
> 	
> 	procedure Pop(The_Stack : in out Int_Stack; X : out Integer) is
> 		
> 	begin
> 		If The_Stack.Top < 1 then
> 			raise Stack_Underflow;
> 		end if;
> 		
> 		X := The_Stack.Elements(The_Stack.Top);
> 		
> 		if The_Stack.Top > 1 then
> 			The_Stack.Top := The_Stack.Top - 1;
> 		end if;

Similar to the Push case, this never raises an exception and continally 
returns the same value when the stack is empty.

The value 1 you compare to here is a "magic number" and would be better 
replaced by an attribute. This is true in other places below, too.

> 		
> 		
> 	end Pop;
> 	
> 	function Is_Empty(The_Stack : in Int_stack) return Boolean is
> 	
> 	begin
> 	
> 		if The_Stack.Top = The_Stack.Elements'First then
> 			return True;
> 		else
> 			return False;
> 			
> 		end if;

This is better written as

return The_Stack.Top = The_Stack.Elements'First;

If you use Top = 0 to indicate an empty stack, then it becomes

return The_Stack.Top = Empty;

Naturally, you define Empty as a constant to avoid having zero be a 
magic number all over the place.

> 		
> 	end Is_Empty;
> 	
> 	function Is_Full(The_Stack : in Int_Stack) return Boolean is
> 	
> 	begin
> 	
> 		if The_Stack.Top = The_Stack.Elements'Last then
> 			
> 			return True;
> 		else 
> 			return False;
> 		
> 		end if;

Ditto.

> 		
> 	end Is_Full;
> 	
> 	function Length_Of(The_Stack : in Int_Stack) return Natural is
> 	
> 		Length : Natural;
> 	
> 	begin
> 		-- This entire function seems somewhat rendundant
> 		-- considering the "Length" attribute. But it's
> 		-- being included as suggested
> 		
> 		Length := The_Stack.Elements'Length;
> 		
> 		return Length;

This always returns the same value. What you want to return is a value 
related to Top (which changes). (If you use Top = zero to indicate an 
empty stack, this function is simpler.)

> 		
> 	end Length_Of;		
> 
> 	function Sum_Of(The_Stack : in Int_Stack) return Integer is
> 		
> 		The_Sum : Integer := 0;
> 	
> 	begin
> 	
> 		for J in 1..The_Stack.Top loop
> 		
> 			The_Sum := The_Sum + The_Stack.Elements(J);
> 			
> 		end loop;
> 		
> 		return The_Sum;

This is OK. It is, however, a special case of an iterator. It might be 
better to provide a generic iterator and let the client decide what he 
wants to do with each element in turn.

Then again, it might be better to make the element type of the stack be 
generic and let the client decide what he wants to push onto a stack. 
That will be your assignment when you finish this one.

> 	
> 	end Sum_Of;
> 
> end int_stack;

There are results from psychology that the first few characters of an 
identifier are the most important in distinguishing it from other 
identifiers. Since so many of your identifiers begin with the same 4 
characters, it makes your code more difficult to read. It would be 
better to leave the "The_" off of identifiers.

-- 
Jeff Carter
"Hello! Smelly English K...niggets."
Monty Python & the Holy Grail