Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_TYPE_H_
#define STACK_TYPE_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_TYPE_H_
#define STACK_TYPE_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_TYPE_H_
#define STACK_TYPE_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_H_STACK_TYPE_H_
#define STACK_H_STACK_TYPE_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_H_
#define STACK_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
Include guards
Rather than relying on #pragma once
, you should use an include guard in your Stack.h
file instead, like this:
#ifndef STACK_TYPE_H_
#define STACK_TYPE_H_
// Original code for Stack.h goes here.
#endif
While #pragma once
is supported across many compilers, there's always the chance that there is one that doesn't support it because it still isn't standard. If you feel the need to use both #pragma once
and include guards, just do something like this:
#ifndef STACK_H_
#define STACK_H_
#pragma once
// Original code for Stack.h goes here.
#endif
Passing by const
reference
If you never modify the value of an argument when you pass it, like in the push
function:
void push(Type data) { ... }
Rather than passing the value normally, you should pass it by const
reference, like this:
void push(const Type& data)
{
...
}
While this is a micro-optimization for "small" types, like int
, or bool
, when you start dealing with "bigger" types, like std::string
, or a user-defined type, doing this becomes a little more important.
Nitpicks
While your test file containing main
isn't such a huge deal, there are still a few bad things I want to point out about it.
First off, this line:
using namespace std;
Is as really bad habit to get into. It can also result in many bad things happening, especially if you're using Boost, which provides alternatives to some of the functions in std
. Some of the bad things that can happen can be found here.
This line here should be removed as well:
system("PAUSE");
Preferably if you need to display output for the user to see, you should use something like this instead:
std::cin.get();
system("PAUSE");
is bad, for the following reasons:
- It's slow and un-optimal.
- It's insecure.
- It's very platform-dependent.
Preferably, as it is the C++ style, you should define you class and it's function/constructor signatures inside Stack.h
, and then create a file named Stack.cpp
where you implement the function/constructors.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
bool isEmpty() { return length == 0; }
Should become this:
bool isEmpty() const {
return length == 0;
}
It's also worth noting that your code is clearer when you place your curly braces like this:
something
{
}
Not like this:
something {
}
This isn't required though, just a tip.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
It's also worth noting that your code is clearer when you place your curly braces like this:
something
{
}
Not like this:
something {
}
This isn't required though, just a tip.
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this:
Finally, your isEmpty
function should be const
, since it isn't modifying anything. That means that this: