switch (ch)
{
case '\n':
flags |= SEPARATOR;
break;
case 'a':
...
}
switch (ch)
{
case '\n':
flags |= SEPARATOR;
break; // same indentation as 'case' above.
case 'a':
if (flags > SEPARATOR)
return 0;
...
break; // or maybe [[fallthrough]]; if there's another
// switch entry next.
}
#include <stdexcept>
namespace
{
bool validAsBit(char ch)
{
return ch =='0' || ch == '1';
}
char const eMsgInvalidBit[] =
"Invalid input: bits can only have values '1' or '0'.";
}
// Convert e.g. '1101' to 13.
unsigned int base2ToUint(char const *bits)
{
int retval = 0;
while (*bits != '\0')
{
if (not validAsBit(*bits))
throw(std::runtime_error(eMsgInvalidBit));
retval <<= 1;
retval |= *bits++ == '1' ? 1u : 0u;
}
return retval;
}a + b' rather than: 'a+b'. A less dense layout is a bit easier on the eye. Exceptions are comma (','), scope resolution ('::) and member access ('.' and '->') operators. Threat the comma as you would in plain text: put a space after, but not before. Treat scope resolution and member access operators as you would a hyphen in plain text: with no spaces on either side.
for (....)' instead of: 'for(....)'.
iostream' (which defines streams) when including 'iosfwd' (which merely declares streams) would do.
algorithm' header happens to include the 'vector' header, but there is no guarantee it will continue to do so in the future. So if your program includes 'algorithm' and manipulates std::vectors, it should include 'vector' too.
int main(int argc, char **argv)
{
// Quick check for '-h' option.
if (argv[1][1] == 'h') // JB: BOUNDS if argc == 1
usage();
}
for (size_t ix = 1; ix <= argc; ++ix) // JB: CANON, CW
cout << argv[ix - 1] << ' ';
for (size_t ix = 1, end = argc; ix != end; ++ix) // no argc cast
cout << argv[ix] << ' ';
const int const *ip; // invalid
const int *const *ip; // inconsistent
const int nr; // therefore also inconsistent
int const *const ip; // "ip is a const pointer to a const int"
int const nr; // consistent
cout << *args; //JB CTR
++args;
cout << *args++;
for (size_t ix = 1; ix <= argc; ++ix) // JB: CW
cstdlib), not C headers (stdlib.h).
class Proxy {
string d_str; // When merely using the str parameter: string const &
...
public:
Proxy(string const &str);
};
int main()
{
unsigned int age = 0;
string name = ""; //JB: DORD: default constructor suffices.
cin >> name >> age;
if (name.empty())
{
cerr << "Can't work with empty name.\n";
return ERR_NAME_EMPTY;
}
else if (not name.empty() && age < 18)
// JB: DORD: first part will never match.
{
cerr << "You are a minor.\n";
return ERR_MINOR;
}
cout << "Welcome, " << name << '\n';
return 0; // JB: DORD, C
}
int main(int argc, char **argv)
{
extern char **environ;
char **end = environ;
while (*end != 0)
++end;
cout << (end - environ) << " envvars\n";
end = argv;
while (*end != 0) //JB: FACTOR
++end;
//JB: FACTOR
cout << (end - argv) << " args\n";
}
size_t ntbs_count(char const * const *start);
int main(int argc, char **argv)
{
extern char **environ;
cout << ntbs_count(environ) << " envvars\n";
cout << ntbs_count(argv) << " args\n";
}
size_t ntbs_count(char const * const *start)
{
char const * const *end = start;
while (*end != nullptr)
++end;
return end - start;
}
int main(int argc, char **argv)
{
bool has_args = false;
if (argc > 1)
has_args = true; // JB: CTR/FLOW: use ternary
//JB: FLOW: ternary used as if-stmt. (return value ignored)
has_args ? cout << "args given" : cout << "no args";
//JB: FLOW: Possibly return E_NO_ARGS here, or if not,
//JB: continue without needing extra indentation.
if (has_args)
{
Op operator;
if (argv[1][0] == 'a')
operator = ADD;
// JB: FLOW: use switch, not if-else ladder.
else if (argv[1][0] == 'm')
operator = MULTIPLY;
else if (argv[1][0] == 's')
operator = SUBTRACT;
else if (argv[1][0] == 'd')
operator = DIVIDE;
size_t ix = 2;
// JB: FLOW: while-loop on known number of iterations.
while (ix != argc)
process(argv[ix]);
return 0;
}
else //JB: SF else after return.
return E_NO_ARGS;
}
class MyClass{
int d_nr;
public:
number(int nr)
{ //JB: ICI
d_nr = nr;
}
...
};class MyClass{
int d_nr;
public:
number(int nr);
...
};
MyClass::number(int nr)
{
d_nr = nr;
}
// This is the file myclass.ih
#include <iosfwd>
#include "myclass.h" //JB: IF.// This is the file myclass.ih
#include "myclass.h" //JB: .h missing iosfwd? Detected here!
#include <iosfwd>#include statements you have to write, and when using precompiled headers, it also reduces build time.if (cond) var = value;', or 'int value = 3, count = 0;' Each statement should be written on a line of its own, and nested statements should be indented. Nested compound statements use the same indentation as their main statement, but the statements they contain are indented.
char const *a = s; // JB: NAMING (2x)
while (*a != '\0') // JB: NSC
++a;
size_t TheValueThatWeComputeAsLength = a - s; // JB: NAMING
return TheValueThatWeComputeAsLength; // JB: CTRchar const *end = begin;
while (*end != '\0') //JB: Better, but still NSC.
++end;
return end - begin;EIEIO = 4, // was 3 is much more convenient.
enum
{
ESUCCESS = 0,
EUNKNOWN = 1,
EIO = 2, // simple IO error
EIEIO = 3, // extended IO error
};
bool all_divisible_by_3(std::istream &ins)
{
int number;
while (true)
{
in >> number;
if (ins.eof())
return true;
if (not ins.good())
exit(ins.bad() ? EIEIO : EIO);
if (number % 3 != 0)
return false;
}
return true;
}cin >> word; //JB: PASCAL
while (cin && word != "quit")
{
cout << '"' << word << "\" ";
cin >> word;
}while (true)
{
cin >> word;
if (!cin || word == "quit")
break;
cout << '"' << word << "\" ";
}if (valid(ch))
{ // JB: Perl
++count;
}if (valid(ch))
++count;++count) over postfix (count++). The same goes for the decrement operators (--). If their return value is ignored, don't use postfix operators at all.
for (int i = 0; i < end; i++) is not a problem: the compiler is free to optimize i++ into ++i here.
* of a pointer or the & of a reference binds tighter to the variable name than to the type. We don't like multiple definitions with the same type, but in int* ipt, num; num would be an int, not a pointer. So make it int *ipt;elses after returns, entirely unused variables. Please make your code shorter by removing the superfluous part.
computeTheLengthOfAStringThatIsPassedAsArgument. Try to capture the essence of the identifier's meaning in a short name. E.g., use 'length' instead. The function's argument already suggests that its length will be computed.