Granny's Programming Pearls
"inside of every large program is a small program struggling to get out"
JavaRanch.com/granny.jsp
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Matching Grouping Symbols  RSS feed

 
Charles Cole
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I need to write a program that checks whether a Java source-code file has correct pairs of grouping symbols and the program must implement the Stack<E> class to solve this problem.

Here is the file:


I tried something like this, but I'm not exactly sure what I am doing wrong:

 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
hi Charles,

what exactly is the problem you are facing? The code seems okay,
I tried it and it works. I only got an ArrayOutOfBounds-error, at line 40.

Greetz,
Piet
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Well, you do get an 'java.util.EmptyStackException' when you peek from an empty stack.
I'd say: at line 29 and a half, check for an empty stack first.

Greetz,
Piet

 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Charles Cole wrote:I tried something like this, but I'm not exactly sure what I am doing wrong:

Well, one problem I see is that all your code is in one place (ie, in your main() method), which is generally not a good thing. And I suspect that the reason is that you've just sat down and started to code without writing out what needs to be done in English.

For example:
1. Do you understand why you've been asked to use a Stack?
2. One of your major steps is plainly "convert file contents to a String". So why not write a method to do it? Perhaps:
  public static String convertFileContents(String fileName) { ...
or maybe even:
  public static char[] convertFileContents(String fileName) { ...
since that's what you end up using.

That would take the first dozen lines or so out of your main() method, and replace it with a method call that describes what you're doing for any poor soul who has to try and decipher your code.
Part of the art of programming is breaking problems down into manageable pieces or steps; but it's hard to do if you don't spend some time working out what those "steps" are before you start coding.

Also - a few Java tips for you:
1. Lines 16-20 can be replaced wtih:
  char[] charArray = fileString.toCharArray();

2. A char[] can be processed as Characters with a "for-each" loop, viz:
  for(Character temp : charArray) { ...
Although in your case it means that you'll then have to define i outside and increment it yourself.
Probably worth remembering for the future though.

3. Your code contains a LOT of hard-coded stuff, especially in the loop starting at line 24. This makes it "brittle" because if you ever need to change it, or add new stuff (for example: quotes), you'll have your work cut out for you. Supposing instead you had two extra fields in your class:
  private static final String OPENING_CHARS = "({[";
  private static final String CLOSING_CHARS = ")}]";
What do you think your code might look like then? (Hint: the indexOf() method in String will help you out)

An even more Object-Oriented solution might be to make your "group pairs" an enum ... but that's probably for when you have a bit more experience.

Basically: Try and remove as much "logic" from your main() method as you possibly can, and put it in other methods. That will allow it to look very much like the actual steps involved in solving the problem, eg, maybe something like this:Do you see how that helps? Quite apart from being easier to read, your problem is now broken up into logical pieces that are independent, and which you can tackle (and test) one at a time.

HIH

Winston
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston,

with all respect: why not wait for Charles to tell what his problem is?

Greetz,
Piet
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Piet Souris wrote:with all respect: why not wait for Charles to tell what his problem is?

Point taken. This was my "first coffee" effort, so I was probably a bit over-zealous.

Winston
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
But your comments are, as always, spot on.
 
Charles Cole
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry for the late response. I redid the code and I can't figure out what to write to finish it off with checking whether or not the symbols match. I can't really think right now, can someone give me some help?

 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Charles Cole wrote:Sorry for the late response. I redid the code and I can't figure out what to write to finish it off with checking whether or not the symbols match. I can't really think right now, can someone give me some help?

I don't understand. It looks very similar to what you had before, except without the final print statement, which looked just fine. If the Stack is empty, then the symbols match, no?

A couple of minor points:
  Character temp = new Character(charArray[i]);
is unnecessary; just use:
  Character temp = charArray[i];

Also: your
  } else if (temp.equals('}') || temp.equals(')') || temp.equals(']')) {
check is redundant since you go on to check for the same characters individually.

But neither is wrong; just unnecessary.

Perhaps you could explain exactly what it is you don't follow.

Winston
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!