• Post Reply Bookmark Topic Watch Topic
  • New Topic

ArrayIndexOutOfBoundsException in my own stack class  RSS feed

 
akshat khandelwal
Greenhorn
Posts: 7
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stack class created by me is given below


And, its MyStackEmpty class is :



But its peek() and pop() are throwing ArrayIndexOutOfBoundsException .
Please help
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That is because inside your pop() and peek() methods, you throw a MyStackEmpty exception, but you also immediately catch the exception again. The code then continues after the catch block, and you get an ArrayIndexOutOfBoundsException because you're trying to access an element of the vector with an invalid index.

Don't catch the MyStackEmpty exception inside the pop() and peek() methods. Let the exception be thrown out of the methods, so that the user of your stack class has a chance to deal with the exception. So, remove the try { ... } catch (MyStackEmpty e) { ... } from these methods.
 
Vishakha Lele
Greenhorn
Posts: 5
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Your code is all correct. Only the issue is in what sequence did you call peek() and pop() methods after pushing the object into the stack? Because, it gives error of you first pop the element and then peek. But it runs properly if you first peek the element and then pop. The reason behind is, peek() method only peeks the object and return it. But pop actually removes the object from data structure as per your code.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I personally would do it completely different. I think the original Stack class was designed wrongly (Joshua Bloch says so, so that must be correct ‍), and I am not at all convinced that moving Stack functions to ArrayDeque made things much better. I think a Stack ISN'T A Deque and a Deque ISN'T A Stack.
I think you need a Stack interface, and then you can consider implementations. A Stack interface would have four methods for pushing popping peeking and seeing whether the stack is empty. I think you can create implementations based on a (singly‑)linked list, or an array.
Is searching a normal function of a Stack? I haven't see Stacks with a search method before.
The empty() method shou‍ld be called isEmpty() and you shou‍ld simplify it greatly. I shall give you a hint by referring you to the old Sun style guide.
Your MyStackEmpty class shou‍d have a name ending ...Exception.
 
Liutauras Vilda
Sheriff
Posts: 4928
334
BSD
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
1. Why the class isn't public? Methods too?

2. Campbell gave you hint, which referring to "Sun's style guide". Shoot another rabbit and learn how to format code properly.

3. There some constructs in your code, which look like:
It is quite dangerous to miss out braces, but if you don't like to use them, you can simplify all construct and be happy not using braces:

4. You have some constants: 0 and -1; Why not to define them somewhere at the beginning of the class and use them, so the code would be more professional?

5. You have two comments. Both are inconsistent (one starts with a lower case, another - upper case) and useless. First comment can be deleted and variable named more clearly. Maybe just "counter"?

6. Don't import everything from util package. Import classes you are going to use. That can give to somebody and instant clue how you have implemented your stack. Use wildcard imports if you using quite a few classes from a particular package.
 
Stephan van Hulst
Saloon Keeper
Posts: 7993
143
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm going to disagree with you on a few counts Liutauras.

Liutauras Vilda wrote:1. Why the class isn't public? Methods too?

Actually I think this is great style, and the OP should keep it up. Only make things public when they really have to be. This should be a habit for everybody.

4. You have some constants: 0 and -1; Why not to define them somewhere at the beginning of the class and use them, so the code would be more professional?

I don't think these will be necessary. The instances where 0 is used should be replaced with a call to an isEmpty() method. Using a constant for 0 inside this method adds an extra layer of indirection that is just confusing. The same goes for 1 if it's just used as an increment or a decrement. Replacing it with a constant such as INCREMENT, STEP, or even worse, ONE is just silly. 0 and 1 are such fundamental numbers, it's usually clear from the context what they mean.

6. Don't import everything from util package. Import classes you are going to use. That can give to somebody and instant clue how you have implemented your stack. Use wildcard imports if you using quite a few classes from a particular package.

This advise holds for packages that are not widely used. If you use wildcard import for packages such as java.util, java.io, etc. it actually becomes easier to see what classes are being imported that are NOT commonly used.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Maybe if Liutauras gave the class a package name, it would require public access to most methods.
The way I would write it differently (did I really say different earlier?) the class would not require any imports at all. I don't think I would write a stack class with a List field when I can use an array or similar directly.
I have seem problems with on‑demand imports. When I was doing my MSc, one chap complained he couldn't get Timer to compile. I couldn't understand the problem at first because I had used individual imports, but he had used import java.util.*: import javax.swing.*; and there were two Timer classes. Somebody on this website once said that you can break on‑demand imports by upgrading your implementations. Let's imagine somebody had written an Objects class under Java6: import java.util.*: import uk.co.critchie.resources.Objects; All is well until Java7 comes along and they get errors (or maybe even runtime errors) about this class. If they weren't on the ball, they would have no end of difficulty working out what broke the code.
The different (correct grammar this time ‍) way I would write Stacks wouldn't require magic numbers at all: I would use the ++ and -- operators throughout except in this method:-Note that those of us who have written Forth call the number of elements on a stack its depth. Also note that the old style guide permits 0 1 and −1 as literals in code.

Those of us who have written Forth recognise four basic operations on a stack: push pop peek isEmpty. Push and isEmpty don't require any number of elements on the stack beforehand, but pop and peek require there be at least one element on the stack already. But we also recognise advanced operations, which require different minimum depths:
swap (2)
drop (1)
drop2 (2)
nip (2)
duplicate (1)
duplicate2 (2)
over (2)
rotate (3)
rotateBack (3)
and some people will recognise swap2, over2, and nip2, which require a minimum depth of 4. At least I think that's right. Forth uses different spellings.
Most people writing stacks in Java® won't need the advanced operations, but they can all be implemented by a combination of push pop and peek.
 
Liutauras Vilda
Sheriff
Posts: 4928
334
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephan van Hulst wrote:Actually I think this is great style, and the OP should keep it up. Only make things public when they really have to be. This should be a habit for everybody.

In this particular case I still stay with my opinion. I'd agree with you if we were to talk about some ordinary classes. Such project looks like a potential candidate for OP's proprietary API, hence such advice.

Stephan van Hulst wrote:I don't think these will be necessary. The instances where 0 is used should be replaced with a call to an isEmpty() method.

That's even better, agreed.

Stephan van Hulst wrote:This advise holds for packages that are not widely used. If you use wildcard import for packages such as java.util, java.io, etc. it actually becomes easier to see what classes are being imported that are NOT commonly used.

Again, I'd re-asses my advice if we were to talk about some other situation. In this particular case we have very limited amount of classes which are used outside the lang package (in fact Vector only). If I were to chose, I'd go with explicit import. I have a habit to look at import statements once I open a class. But as you pointed out, opinions may differ.

 
Stephan van Hulst
Saloon Keeper
Posts: 7993
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liutauras Vilda wrote:Such project looks like a potential candidate for OP's proprietary API, hence such advice.

Advice becomes necessity once the classes actually get used outside of their package. Make them public at that point, and no sooner.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Actually, I would have to use a little fiddle to avoid − 1 in the peek() method, creating a topElement field.rather thanI would probably (if awake) make all methods final, too.
 
Stephan van Hulst
Saloon Keeper
Posts: 7993
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't understand why you would even want to use an array when you have classes that handle all these trickeries for you. You may not like ArrayDeque, Campbell, but it's a fully functional tool that you can use to implement the Stack class.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephan van Hulst wrote:I don't understand why you would even want to use an array . . .
Good idea, which I sh‍ould have got with a bit more thought. I don't not like ArrayDeque; I just think it is the wrong implementation for a stack. I would change line 1 to
[??]public[??] final class ArrayStack<E> implements Stack<E>
By Stack<E> I mean an ordinary stack without all the advanced features, but there is nothing to stop people from writing classes implementing AdvancedStack<E>.
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!