Topic: C++ Project is A.c.c.o.m.p.l.i.s.h.e.d Thank you Bokomon, KoM for your tutoring
Blayne Bradley
unregistered
posted
the code posted somewhere above is out of date allow me to update it asap on my laptop and dont have my password for smartftp stored.
IP: Logged |
posted
Ah, I see the problem. Your stack is popping quite correctly. However, cout doesn't know that it's dealing with a stack; it thinks you are passing it a quite ordinary char pointer. It therefore prints it out until it reaches a null character. Since you don't actually delete your items, just make the top pointer point elsewhere, it looks as though they're all there, even though they aren't for stack-operations purposes. You can fix this in at least two ways : Write your own stack-printer, which only prints out those elements that are below the top index; or else set deleted (popped) elements to zero again.
I see you fixed your local-global confusion; good. But for the love of god, can you rewrite that empty function? One line, I tell you; one.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
But it works Fine, you win. How do I write it as one line again?
Next how do I set popped elements back to zero?
IP: Logged |
Blayne Bradley
unregistered
posted
would it be instead of:
opr = oprstk.item[oprstk.top];
make it
oprstk.item[oprstk.top] = ' ';
??
I think it wokrs my output is:
A AB ABC AB A
Not sure if this is working I think it is.
IP: Logged |
posted
Well, tell you what, take it step by step. Here's your code:
code:
bool empty( OPERATOR_STACK& oprstk ) {
bool RESULT;
if (oprstk.top == -1) { RESULT = true; return RESULT; } else { RESULT = false; return RESULT; } }
Just to start with, why have you got two separate return statements in here? Didn't your mother teach you that single exit points are good? Also, let me mention that you have come down on the wrong side of the Brace Wars, and your soul will therefore burn in hell for all eternity. One True Brace Style for the win!
As for setting things to zero... dude, if at this point you need me to tell you how to assign a value to a variable, I suggest you just drop the course.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
posted
Wupsie, cross-post. Yes, what you have works. I was actually thinking
oprstk.item[oprstk.top] = 0;
to take advantage of the behaviour of null-terminated strings, but as long as it prints out the right thing, who cares? However, you should have this line in addition to, as my supervisor is fond of saying, the line
opr = oprstk.item[oprstk.top];
since you still want to return what you just popped, presumably.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
Too stubburn to drop it but I believe I was simply confused by what you meant and I think I did what you suggested am I correct in amking top = to a blank?
posted
Making top equal to a blank works, in that it prints out only un-popped letters. I believe making top equal to zero is better, because it makes cout behave the same as your stack logic; that is, it sees 'string ends' in its own internal logic where you've got 'stack ends' in your internal logic. But such consistency, it's true, is more a matter of taste. You're only using cout for testing, so as long as it prints out non-confusing results, you're fine.
Never mind about the brace styles, I was joking. Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
Well obviously ^-^ I changed it from ' ' to 0 never know what little critters will show up.
IP: Logged |
Blayne Bradley
unregistered
posted
Ack I spent 4-5 hours working in emacs today writing a awk script and look what happens! When im coding in C++ whenever I go to save I press CTRL x, CTRL s rather then just control s, grrrr.
IP: Logged |
Blayne Bradley
unregistered
posted
Okay V4 is to evaluate a Postfix expression past to the function for its numurical value.
I was given a sheet of pascal pseudo code and I got rid of the majority of the bugs me thinks:
posted
Ok, now it would be two lines if not for your horrible brace style. You don't need braces anyway for little one-liners like this, unless they're required by your professor or something. You could get
code:
if (oprstk.top == -1) return true; return false;
which is a considerable improvement, to be sure. At least you got rid of the useless intermediate variable. Next I suggest you get rid of the if statement and just return the bool directly.
Incidentally, indenting by a whole tab like that is going to come back and bite you in the ass. Set the indentation to two spaces.
About the 'ord' function, if ti does what I think it does the code you posted has a bug, so I'll let you think a bit about what it is supposed to be doing.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
quote:Originally posted by King of Men: Also, let me mention that you have come down on the wrong side of the Brace Wars, and your soul will therefore burn in hell for all eternity. One True Brace Style for the win!
Blayne don't let KoM lead you astray! Your brace style is beautiful (and certainly the most easy to logically follow!). Let the compiler do what it does best: strip whitespace!!
-- But yeah, your empty function can be reduced to one line... And when possible, always place your returns in an area of a function that will always be called. Return in conditional blocks are often a warning that things are being over-engineered. In fact, if you follow this one suggestion alone, you'll shrink your empty method by one line!
I had a slight problem where despite if I isolated the Eval function and passed a single Postfix expression and returned the accurate value ("AB+C-") in this exmaple 2, would output 3 for EVERY postfix expression depsite my stopwatch cout showing the correct anwser being return from my Operations function.
I brilliantly determiend that Pfval being in a for loop was being on each loop replaced by the proceeding value and thus all 3's.
Then I put pfval as an array an put in a counter and now it works flawlessly ^-^.
V5 is the same as v3, basically do 3 pushes, then do a while that pops until empty and then out put whats in stack after each pop/push.
Annoyingness: Hery high
A whole lot of cpy and pasty and variable changing. Grr.
IP: Logged |
Blayne Bradley
unregistered
posted
Okay version 5 is done, now onwards! The final version versian 6is all thats left all I needs to do is get a full blown conversation function working that will cnfix to Postfix.
posted
I can't help with the immediate issue, but I recommend you don't pass-by-reference a variable, assign it a value and then also return it. If you want to return a value, I recommend you pass everything by-value. If you want to modify a data structure, pass-by-reference is required, of course, but then I would either make it a void function, or return an int error code (with zero signifying success by default). passing a reference to a variable 3 or 4 functions deep can be an exercise in debugging masochism, IMO.
Remember the KISS principle. The first rule in development.
posted
It's the Prcd( sym, opstk.top ) call in Convert. opstk.top is an int. Prcd wants an OPERATOR_STACK.
Posts: 148 | Registered: Feb 2000
| IP: Logged |
Blayne Bradley
unregistered
posted
Okay I fixed it by modifying Prcd to not accept OPERATOR_STACK as an arguement, now its simply Prcd( sym) which works cuz' OPERATOR_STACK is a global and I can call opstk inside the functions.
Not to figure out why my variables keep becomming undefined mid debug.
Okay, its because Pfx = Pfx + Sym doesn't acually work Time to do strcpy ftw.
IP: Logged |
quote:Originally posted by Blayne Bradley: Okay I fixed it by modifying Prcd to not accept OPERATOR_STACK as an arguement, now its simply Prcd( sym) which works cuz' OPERATOR_STACK is a global and I can call opstk inside the functions.
This is evil. Like "get booted from a job" bad. It may work for a class assignment but DO NOT let this become a habit.
posted
Not only that, it is actually by far the more complicated solution, measured in amount of typing. If you do evil hacks, they should at least save you some work! The obvious thing to do is just to pass the stack instead of its 'top' member. I mean, duh. That's like 4 backspaces, instead of all this changing of interfaces and kludging of globals.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
MY latest problem is ha I didn't quit understand at first the logic behind the given flowchart for detmrining the preedneces of operators so I did it only with brakets in mind so I ended up with postfix stirngs like ABC+- not AB+C-
Now I have to code in this mother of all case statements.
IP: Logged |
When you say, "mother of all case statements", that should immediately make you think if there isn't a Better Way(tm). In fact, I would strongly look into using recursion for some of the processing of the statements... I believe that is a more "classical" way to solve this type of problem (even if it is a bit rough on the stack [system stack, not the one you've implemented]).
posted
I think so, ya I think so, problem is I already have the case statements my teacher typed for aomwonw else and I really don't think there's a better way that is currently within my programming skills and time slot.
code:
bool Prcd( char top, char symbol ) {
switch( top ) {
switch( sym ) { case '+': return true; break; case '*': return false; break; case '-': return true; break; case '/': return false; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '*': switch( sym ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '-': switch( sym ) { case '+': return true; break; case '*': return false; break; case '-': return true; break; case '/': return false; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '/': switch( sym ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '$': switch( sym ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return true; break; case '(': return false; break; case ')': return true; break; } break;
case '(': return false; break; }
}
Gah! Now it says indirection not allowed. Why me of all people getting these errors...
:EDIT: Figrued it out had opstk.item[opstk.item] not a good thing.
posted
Ok, in the first place, if you're going to have returns in every case, you don't need breaks. In the second place, your posted code has a bug in it. In the third place, could you for the love of Papa Janitor use [ code] blocks?
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Incidentally, I think you people are going way overboard in helping Blayne with his homework. And you're not going him any favors either. A huge part of learning to program is figuring out how to debug on your own.
Posts: 10177 | Registered: Apr 2001
| IP: Logged |
posted
Oh, and in the fourth place, about your current thread title, 'stupid syntax errors' : Consider who put the syntax errors in there.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
OKays, Version 6 complted. Thank KoM for your help.
MrSquicky; I'm 90% sure Ive only asked for help in regards to syntax errors, I am pretty sure that that I have indeed learned a great deal and my skill in managing the debugger has increased drastically.
If you look at the entirety of my code and look at my requests you'lll see that I'e only asked for help in only a small minority of my code.
posted
Well, it's a code block, which is good, but it's still got a bug in it, so I hope that's not your final code. Posts: 10645 | Registered: Jul 2004
| IP: Logged |
// asst3v1.cpp fall 2006 ALG starts from straray.cpp ch.7 // array of strings #include <iostream> #include <iomanip> //for setw function to format output #include "string.h" #include <math.h>
//standard namespace using namespace std;
const int LMAX = 50; //maximum number of infix strings in array const int NMAX = 30; //maximum size of each infix string const int LSIZE = 5; //number of actual infix strings in array const int MAXSTACK = 100;
/**************************************************************************** PRINT OUT THE INFIX EXPRESSIONS ****************************************************************************/
/********************************************* Is it an Operand? *********************************************/ bool Oprnd( char symb ) { if (isupper(symb)) return true; return false; }
/********************************************* Pop the top Operator from the stack *********************************************/ void Pop( char& sym, OPERATOR_STACK& oprstk ) {
/********************************************* Pop the top Operand from the stack *********************************************/ float Popopr( float& sym, OPERAND_STACK& oprndstk ) {
//makes the op_1 to the op_2 = val val = pow( operator_1, operator_2 );
return val;
}
/********************************************* Precendence Function *********************************************/ bool Prcd( char top, char symbol ) {
switch( top ) {
switch( symbol ) { case '+': return true; break; case '*': return false; break; case '-': return true; break; case '/': return false; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '*': switch( symbol ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '-': switch( symbol ) { case '+': return true; break; case '*': return false; break; case '-': return true; break; case '/': return false; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '/': switch( symbol ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return false; break; case '(': return false; break; case ')': return true; break; } break;
case '$': switch( symbol ) { case '+': return true; break; case '*': return true; break; case '-': return true; break; case '/': return true; break; case '$': return true; break; case '(': return false; break; case ')': return true; break; } break;
posted
Get rid of the breaks in your precedence function (and why are you calling it Prcd, anyway? Didn't your prof tell you to use descriptive names? Dropping out all the vowels is not descriptive.) Then take a closer look at your outer switch statement.
Posts: 10645 | Registered: Jul 2004
| IP: Logged |
Blayne Bradley
unregistered
posted
well I ran it with the Infix array we were given and it works perfectly but yes I have 24 hours reamining now to fix up my code to get it to look nicer ^-^ so ya breaks wiill now = gone soonish.
IP: Logged |
Blayne Bradley
unregistered
posted
and on another note my prof actually prefers less descriptive names. I dont understand why.
IP: Logged |
Blayne Bradley
unregistered
posted
Actually on closer inspection it DIDNT work perfectly ^-^ I had a '+' inbetween E and an F on one and an extra '/' on the end.
Debuggging told me it was always returning true so I checked the case.
Turns out I was missing a '+' case.
Thanks KoM.
Got rid of all the breaks and now I'm commenting my code.
IP: Logged |
Blayne Bradley
unregistered
posted
My teacher after I anncounced my completion of v5, congrauated me for my focus and dedication to the assignment. I feel so proud ^-^
IP: Logged |
Printer-friendly view of this topic
![[Frown]](frown.gif)
Fine, you win. How do I write it as one line again?![[Smile]](smile.gif)
![[Mad]](mad.gif)
![[Big Grin]](biggrin.gif)
![[Confused]](confused.gif)
I dont understand why.
