Getting in a bit late here, but yeah, your teacher is a bit whacked here...
I agree with Scott in that all your if/else code is ridiculous. A lookup table is one option, but a simple switch statement also would've done the trick and still been cleaner than all those ifs (and probably still run nicely as most compilers tend to be really good at optimizing switch statements -- they probably internally implement them as lookup tables, though you can't rely on this): switch (cardnum) { case 0: case 1: case 2: case 3: return 11; break; // Not really necessary in this case, but I consider it good form
case 4: case 5: case 6: case 7: return 2; break;
// so on...
default: return 10; break; }
TestHW2 is also a useless class that serves no purpose. If you really want it to serve a purpose, you can put everything in main() in TestHW2::main() and have the only code in main() be this: int main() { TestHW2 test; return test.main(); }
Doesn't do anything useful unless you plan to replace TestHW2 with another class down the road, but at least it fulfills the ridiculous requirements the teacher gave you (maybe he plans to build on this assignment later where this matters? I don't know -- I can't read his mind). I've seen some C++ GUI libraries (Qt for example) that use a similar tactic -- where main() only sets up the GUI library and the "real" main is a method from a class in the GUI library, something like: int main() { GuiApp app; app.initWithSomething(/* whatever you need to init it with */); return app.startEventLoop(); }
I'm also surprised that your compiler isn't issuing a warning because you're not returning a value from main(), since it should be returning an int.
I'm assuming I'm looking at the latest version of your code at http://pastebin.com/qAtAQG8L , but you don't actually have a return statement in main() to actually return the int. You're declaring that it returns an int without actually returning one.
Generally, the rule of thumb is that you return 0 if there are no errors, and basically anything else if there were errors. Technically, you should return EXIT_SUCCESS on success and EXIT_FAILURE for failure (since 0 may be a failure code on some operating systems), but most programmers are lazy and don't do so. These constants are defined in cstdlib.
I'm also noticing a few other examples of things that could be considered "bad form" as I look over the code. The global variables cardsDealt and cardInDeck are two examples of this bad form. These should be instance variables in your Deck class.
BTW, where is your latest code posted? I think the version I'm looking at is pretty ancient.
I'm also noticing a few other examples of things that could be considered "bad form" as I look over the code. The global variables cardsDealt and cardInDeck are two examples of this bad form. These should be instance variables in your Deck class.
This is actually pretty serious, because it means that, for example, you can't actually have two decks at once. Also, the appropriate scope for cardInDeck is actually solely within the method; it shouldn't be an instance variable, let alone a global.
Another thing to note - you should never do comparisons like (x == true) when x is a boolean variable, because that's redundant. In natural language terms, your line of code which says while(cardInDeck == true) translates to something like "While the statement 'the card is in the deck' is true" - the natural thing to do is while(cardInDeck) instead, which is more like "while the card is in the deck".
Your code also seems to be incorrect in a couple of ways, but that's up to you to find and fix.
I'm also noticing a few other examples of things that could be considered "bad form" as I look over the code. The global variables cardsDealt and cardInDeck are two examples of this bad form. These should be instance variables in your Deck class.
This is actually pretty serious, because it means that, for example, you can't actually have two decks at once. Also, the appropriate scope for cardInDeck is actually solely within the method; it shouldn't be an instance variable, let alone a global.
Good point about it being solely within the method. I didn't look at that code closely enough to see that.
Another thing to note - you should never do comparisons like (x == true) when x is a boolean variable, because that's redundant. In natural language terms, your line of code which says while(cardInDeck == true) translates to something like "While the statement 'the card is in the deck' is true" - the natural thing to do is while(cardInDeck) instead, which is more like "while the card is in the deck".
I'll disagree with you there. I like the redundancy as it makes the code easier to read IMHO. This is mostly a case of style preferences/syntactic sugar though -- your argument is also valid, but it has nothing to do with how the code runs and more with making it look "right." Functionally the two are identical.
Another thing to note - you should never do comparisons like (x == true) when x is a boolean variable, because that's redundant. In natural language terms, your line of code which says while(cardInDeck == true) translates to something like "While the statement 'the card is in the deck' is true" - the natural thing to do is while(cardInDeck) instead, which is more like "while the card is in the deck".
I'll disagree with you there. I like the redundancy as it makes the code easier to read IMHO. This is mostly a case of style preferences/syntactic sugar though -- your argument is also valid, but it has nothing to do with how the code runs and more with making it look "right." Functionally the two are identical.
Yes, the two are functionally identical (though the assembly code may be different). However, it's definitely redundant and I don't think it makes the code any more readable. To me, seeing a programmer do it makes me think they don't fully understand the concept of a boolean variable. Also, there are situations in other languages where this can cause actual issues. For example, in C with #define TRUE 1 one would find that if(strcmp(s1, s2)) is quite different to if(strcmp(s1, s2) == TRUE). Here's a real example of where this kind of code could cause trouble.
He might get in trouble for posting assignment code on GitHub, though. Granted, if the deadline is over, I don't see why it would be an issue, but there might be some BS fine print in the course/university policy.
EDIT: I guess Pastebin is almost as likely to cause trouble if that's the case, though.
He might get in trouble for posting assignment code on GitHub, though. Granted, if the deadline is over, I don't see why it would be an issue, but there might be some BS fine print in the course/university policy.
Yes, the two are functionally identical (though the assembly code may be different). However, it's definitely redundant and I don't think it makes the code any more readable. To me, seeing a programmer do it makes me think they don't fully understand the concept of a boolean variable. Also, there are situations in other languages where this can cause actual issues. For example, in C with #define TRUE 1 one would find that if(strcmp(s1, s2)) is quite different to if(strcmp(s1, s2) == TRUE). Here's a real example of where this kind of code could cause trouble.
Well, the C example is a poor one since C doesn't have true boolean variables (at least prior to C99, I believe). I also think using strcmp() itself is a bad example as it does things a little differently than most functions, with 0 being returned for equality and all that.
One case where using a boolean context without comparing to true or false truly does matter is if you're checking for NULL vs. not NULL in the case of smart pointers, such as boost::shared_ptr (and I think std::auto_ptr as well, but I have to look it up). Those classes are specifically written such that they return true or false based on if they are NULL or not when used in a boolean context without having to compare to true or false.
Well, the C example is a poor one since C doesn't have true boolean variables
In general, good programming habits should carry over across languages. The fact that comparisons like (x == true) can sometimes fail in some languages is an indicator that it is something you should generally avoid.
I also think using strcmp() itself is a bad example as it does things a little differently than most functions, with 0 being returned for equality and all that.
That's standard behaviour for comparison functions: negative for less than, zero for equal, positive for greater than - see, for example, the Comparator interface in Java. However, for a better example, there's isalpha, which was mentioned in the link I gave.
Well, the C example is a poor one since C doesn't have true boolean variables
In general, good programming habits should carry over across languages. The fact that comparisons like (x == true) can sometimes fail in some languages is an indicator that it is something you should generally avoid.
For that matter if(x) can fail in some languages as well, so this isn't the best example, though I agree with you in principle. Some languages have if constructs that are required to take a comparison operator and would fail if you don't have some sort of comparison -- for example, Pascal requires the equivalent of if(x == true) (at least it did when I last used it ages ago -- it may have changed since then).
I also think using strcmp() itself is a bad example as it does things a little differently than most functions, with 0 being returned for equality and all that.
That's standard behaviour for comparison functions: negative for less than, zero for equal, positive for greater than - see, for example, the Comparator interface in Java. However, for a better example, there's isalpha, which was mentioned in the link I gave.
Yeah, it's standard behavior for this sort of comparison, true, but if you're a newbie you don't know that.
And I'm old... I used to program in C89... though I never used pre-ANSI C.
For that matter if(x) can fail in some languages as well, so this isn't the best example, though I agree with you in principle. Some languages have if constructs that are required to take a comparison operator and would fail if you don't have some sort of comparison -- for example, Pascal requires the equivalent of if(x == true)
Are you sure? A quick Google suggests that's not the case. Nonetheless, even if that were the case, why would anyone use Pascal!?
He might get in trouble for posting assignment code on GitHub, though. Granted, if the deadline is over, I don't see why it would be an issue, but there might be some BS fine print in the course/university policy.
Schools gotta get with the times.
If he can post it on Pastebin, he can post it as a GitHub Gist.
For that matter if(x) can fail in some languages as well, so this isn't the best example, though I agree with you in principle. Some languages have if constructs that are required to take a comparison operator and would fail if you don't have some sort of comparison -- for example, Pascal requires the equivalent of if(x == true)
Are you sure? A quick Google suggests that's not the case. Nonetheless, even if that were the case, why would anyone use Pascal!?
People used to use Pascal back in the day, even if they don't use it now. Hell, the Windows API was originally written with Pascal calling conventions instead of C calling conventions (which is why you'll find some old Windows code with function declarations like int PASCAL WinMain(...). Even if Pascal isn't the best example, there are other languages out there that fit the mold. MATLAB, which doesn't have the concept of a boolean variable (like C before C99), requires if statements to take a comparison expression. And yes, people do use MATLAB all the time.
Then we get into Smalltalk's funky syntax for if statements, which while it does accept a boolean instead of a conditional expression, still looks way weird: result := some_boolean ifTrue: [ 'greater' ] ifFalse: [ 'lesser' ]
That said, looking over my own code, I'm not really all that consistent when it comes to using if(x) vs. if(x == true). I usually just follow the lead of the existing code if I'm working on an existing code base.
Yeah, MATLAB is good for its libraries, even though as a language it's somewhat dodgy. In any case, while I don't if(x == true) to be a serious concern, it does annoy me a little.
I've been having a "fun" time writing a wrapper so you can use any C++ class you like within the MATLAB environment. I'll just say that MATLAB has some very interesting garbage collection...
Yeah, MATLAB is good for its libraries, even though as a language it's somewhat dodgy. In any case, while I don't if(x == true) to be a serious concern, it does annoy me a little.
Yeah, I don't find if(x) to be a serious concern either, though it doesn't annoy me. It's funny how I think one way but code another way... I think back when I first started coding, I used the if(x == true) construct all the time for "readability," and when talking about coding, I still talk about it... but it turns out in practice I use if(x) far more often these days. I guess my brain and my fingers aren't quite on the same page...
I've been having a "fun" time writing a wrapper so you can use any C++ class you like within the MATLAB environment. I'll just say that MATLAB has some very interesting garbage collection...
So none of this works? I haven't had to use MATLAB in a while myself, since Python with NumPy, SciPy, and matplotlib tends to cover my needs for that kind of thing, but MATLAB definitely has enviable libraries.
We are using mex, however we wrote a program which automatically parses a script file and generates mex files which are then compiled for our entire library. The problem is that MATLAB doesn't garbage collect dynamically allocated C++ objects, so you need to clean them up manually during a clear/clear all scenario.
What about this? Also, it seems that you can wrap it up in a MATLAB handle class which lets you make your own destructor method function delete(this) which I guess would then call the destructor inside the C++ code? It doesn't sound too bad, but MATLAB is definitely quite dodgy in some regards.
No, because we want the C++ objects to persist outside of a single mex file. That way we can pass their pointers around as MATLAB objects, call their functions, etc.
Right, but if you allocate the memory properly and wrap the C++ class with a MATLAB handle class which has a constructor and destructor that just call their MEX counterparts that should work, no?
Yes, that's what I did. However, when you call clear all, it calls the handle deconstructor multiple times. Thus I try to delete unallocated memeory causing a segv. However, if I call clear and then clear all it works just fine.
Comments
I agree with Scott in that all your if/else code is ridiculous. A lookup table is one option, but a simple switch statement also would've done the trick and still been cleaner than all those ifs (and probably still run nicely as most compilers tend to be really good at optimizing switch statements -- they probably internally implement them as lookup tables, though you can't rely on this):
switch (cardnum)
{
case 0:
case 1:
case 2:
case 3:
return 11;
break; // Not really necessary in this case, but I consider it good form
case 4:
case 5:
case 6:
case 7:
return 2;
break;
// so on...
default:
return 10;
break;
}
TestHW2 is also a useless class that serves no purpose. If you really want it to serve a purpose, you can put everything in main() in TestHW2::main() and have the only code in main() be this:
int main()
{
TestHW2 test;
return test.main();
}
Doesn't do anything useful unless you plan to replace TestHW2 with another class down the road, but at least it fulfills the ridiculous requirements the teacher gave you (maybe he plans to build on this assignment later where this matters? I don't know -- I can't read his mind). I've seen some C++ GUI libraries (Qt for example) that use a similar tactic -- where main() only sets up the GUI library and the "real" main is a method from a class in the GUI library, something like:
int main()
{
GuiApp app;
app.initWithSomething(/* whatever you need to init it with */);
return app.startEventLoop();
}
I'm also surprised that your compiler isn't issuing a warning because you're not returning a value from main(), since it should be returning an int.
Generally, the rule of thumb is that you return 0 if there are no errors, and basically anything else if there were errors. Technically, you should return EXIT_SUCCESS on success and EXIT_FAILURE for failure (since 0 may be a failure code on some operating systems), but most programmers are lazy and don't do so. These constants are defined in cstdlib.
BTW, where is your latest code posted? I think the version I'm looking at is pretty ancient.
Another thing to note - you should never do comparisons like
(x == true)when x is a boolean variable, because that's redundant. In natural language terms, your line of code which sayswhile(cardInDeck == true)translates to something like "While the statement 'the card is in the deck' is true" - the natural thing to do iswhile(cardInDeck)instead, which is more like "while the card is in the deck".Your code also seems to be incorrect in a couple of ways, but that's up to you to find and fix.
#define TRUE 1one would find thatif(strcmp(s1, s2))is quite different toif(strcmp(s1, s2) == TRUE). Here's a real example of where this kind of code could cause trouble.EDIT: I guess Pastebin is almost as likely to cause trouble if that's the case, though.
One case where using a boolean context without comparing to true or false truly does matter is if you're checking for NULL vs. not NULL in the case of smart pointers, such as boost::shared_ptr (and I think std::auto_ptr as well, but I have to look it up). Those classes are specifically written such that they return true or false based on if they are NULL or not when used in a boolean context without having to compare to true or false.
if(x == true)(at least it did when I last used it ages ago -- it may have changed since then). Yeah, it's standard behavior for this sort of comparison, true, but if you're a newbie you don't know that.And I'm old... I used to program in C89... though I never used pre-ANSI C.
Then we get into Smalltalk's funky syntax for if statements, which while it does accept a boolean instead of a conditional expression, still looks way weird:
result := some_boolean
ifTrue: [ 'greater' ]
ifFalse: [ 'lesser' ]
That said, looking over my own code, I'm not really all that consistent when it comes to using
if(x)vs.if(x == true). I usually just follow the lead of the existing code if I'm working on an existing code base.In any case, while I don't
if(x == true)to be a serious concern, it does annoy me a little.if(x)to be a serious concern either, though it doesn't annoy me. It's funny how I think one way but code another way... I think back when I first started coding, I used theif(x == true)construct all the time for "readability," and when talking about coding, I still talk about it... but it turns out in practice I useif(x)far more often these days. I guess my brain and my fingers aren't quite on the same page...I haven't had to use MATLAB in a while myself, since Python with NumPy, SciPy, and matplotlib tends to cover my needs for that kind of thing, but MATLAB definitely has enviable libraries.
function delete(this)which I guess would then call the destructor inside the C++ code?It doesn't sound too bad, but MATLAB is definitely quite dodgy in some regards.