Forum:

Java in General

Need help to review my code

Ranch Hand

Posts: 1037

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

I refer to this exercise : https://www3.ntu.edu.sg/home/ehchua/programming/java/J3f_OOPExercises.html#zz-8.

the code given is :

and the requirements ;

Try:

Modify the push() method to throw an IllegalStateException if the stack is full.
Modify the push() to return true if the operation is successful, or false otherwise.
Modify the push() to increase the capacity by reallocating another array, if the stack is full.

I just want to find out if the way i did my code that try to modify the push method is correct or not cos I will not write to the author, after I did not get any reply at all.

public class MyIntStack {
		
	   private int[] contents;
	   private int tos;  // Top of the stack
	   //create an index
	  // private int top;
	   
	   // constructors
	   public MyIntStack(int capacity) {
	      contents = new int[capacity];
	      tos = -1;
	   }
	 
//	   public void push(int element) {
//	      contents[++tos] = element;
//	   }
	   
	   //modified code
	  // public void push(int element) {
		  // if(isFull()) {
			 //  throw new IllegalArgumentException("stack is full");
		  // }else {
		 // contents[top] = element;
		 //  contents[++tos] = element;
		//   }
	  // }
	   
	   //modified push to use boolean
	   public boolean push(int element) {
		   if(isFull()) {
			  // return false; //there is no need to return false
			   throw new IllegalArgumentException("stack is full");
		   
		   }else {
		  contents[++tos] = element;
		  return true;
	   }
	   }
		   
		   //
		public void pushToAnotherArray(int element) {
		if(isFull()) {
		 // return false; //there is no need to return false
		   throw new IllegalArgumentException("stack is full");
			   }
		  int[] arr = new int[100];
			  
		  arr[++tos] = element;
		  }
	   
	   
	 
	   public int pop() {
	      return contents[tos--];
	   }
	 
	   public int peek() {
	      return contents[tos];
	   }
	 
	   public boolean isEmpty() {
	      return tos < 0;
	   }
	 
	   public boolean isFull() {
	      return tos == contents.length - 1;
	   }
	}

Thanks alot alot.

Campbell Ritchie

Marshal

Posts: 79422

377

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

The three instructions you quoted are mutually exclusive, so I think you want three solutions.

tangara goh wrote:. . .public class MyIntStack { private int[] contents; private int tos; // Top of the stack // constructors public MyIntStack(int capacity) { contents = new int[capacity]; tos = -1; } public void push(int element) { contents[++tos] = element; } public int pop() { return contents[tos--]; } public int peek() { return contents[tos]; } public boolean isEmpty() { return tos < 0; } public boolean isFull() { return tos == contents.length - 1; } } . . .

How many errors did you find in that code? I have found three, at least one of them potentially serious. An error similar to one of them in a Stack<E> might become a serious error. I also think the // comment in line 5 constitutes incorrect style.
In line 32, I think you are throwing the wrong exception. There is nothing wrong with the argument, but with the implementation. Did you read the instruction properly?
The method starting line 41 is incorrect. I think it is beyond redemption and you should start again. I also think that it is wrong to create another method name; you were told to modify push(int).
The three instructions you quoted are mutually exclusive, so I think you want three solutions.

tangara goh

Ranch Hand

Posts: 1037

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

Actually, I changed the push name for convenience sake.

Sorry I thought you could tell.

So, basically, I created 3 solutions in the same class.

Please ignore the comment, as I was first struggling how to push the element to the array without an index first.

So, the thing is I check with ai. I was told that the exceptions should be thrown at the peek() and pop() and I thought I better hear the experts' opinion.

Stephan van Hulst

Saloon Keeper

Posts: 15620

366

posted 3 weeks ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

All three of your solutions have issues.

Your first modified push() method throws the wrong exception type.

Your second one throws an exception when instead, it's supposed to return false.

Your third solution throws an exception when instead, it's supposed to increase the capacity of the existing stack.

Campbell Ritchie

Marshal

Posts: 79422

377

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

tangara goh wrote:Actually, I changed the push name for convenience sake. . . .

You now have a method which your users will not know when to use, so you are adding confusion. That method is incorrect regardless; I shall let you work out why.

So, the thing is I check with ai.

Don't rely on AI.

I was told that the exceptions should be thrown at the peek() and pop() . . .

Yes, pop() and peek() should be able to throw exceptions, but you don't seem to have implemented that, nor does the original, nor is it mentioned in your instructions.
I still think you are supposed to produce three different classes, each using a different solution to a possible error. That is the error I noticed straight away.

Liutauras Vilda

Marshal

Posts: 8880

638

I like...

posted 3 weeks ago

1
Number of slices to send:

Optional 'thank-you' note:

Send

private int tos; // Top of the stack

1. rename variable to topOfTheStack or similar
2. remove now redundant comment explaining what the variable is

tangara goh

Ranch Hand

Posts: 1037

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

Stephan van Hulst wrote:All three of your solutions have issues.

Your first modified push() method throws the wrong exception type.

Your second one throws an exception when instead, it's supposed to return false.

Your third solution throws an exception when instead, it's supposed to increase the capacity of the existing stack.

Yes. Silly me on not reading the question carefully. I guess I should also did a test ...

I am puzzled about the capacity that part.

If there is no capacity stated in the constructor, how will the stack know that it's full ?

Or this class is a 'utility' class that is to be made use of with other List like ArrayList, LinkedList, or Tree(am not sure about this one as I haven't go deep into it yet)

Stephan van Hulst

Saloon Keeper

Posts: 15620

366

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

tangara goh wrote:I am puzzled about the capacity that part.

If there is no capacity stated in the constructor, how will the stack know that it's full ?

But there IS a capacity set in the constructor, and the stack DOES know that it's full, by calling the isFull() method. It's really the same as the first two solutions.

The difference is that when the stack is full, you don't throw an exception and you don't return false, but instead you increase the capacity by creating a new larger array, and then copying the existing elements to the larger array.

How much larger? That's up to you, but it's convention to just double the size of the previous array.

Or this class is a 'utility' class that is to be made use of with other List like ArrayList, LinkedList, or Tree(am not sure about this one as I haven't go deep into it yet)

No. It's meant to use an array, not a List. Using a List instead of an array is what I would normally recommend, but it defeats the purpose of the assignment, which is to teach you how to do it with an array.

And using a List will not make your class a 'Utility class'. That means something completely different.

Campbell Ritchie

Marshal

Posts: 79422

377

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

. . . increase the capacity by creating a new larger array, and then copying the existing elements to the larger array. . . .

There is a way to do both in one fell stroke.

And using a List will not make your class a 'Utility class'. That means something completely different.

A utility class usually means a class designed to help other objects; its members are all static and it should have a private constructor to prevent instantiation.
I think Stephan and I agree that you should produce three solutions, one for each of the three instructions.

Campbell Ritchie

Marshal

Posts: 79422

377

posted 3 weeks ago

Number of slices to send:

Optional 'thank-you' note:

Send

Liutauras Vilda wrote:. . . now redundant comment explaining what the variable is

That was my least serious mistake in my first post; a comment explaining a variable name always constitutes proof that the variable name is wrong.