Searching a string of consecutive characters in a binary bar code

Question

I am writing a program that will take a binary bar code and convert it to a zip-code. I need to validate the bar code. A valid binary bar code contains no more and no less than two 1s in any position per five digits. This is what I have for the validation part so far and would like simplify this code.

#include <iostream>
#include <string>
using std::cout;
using std::string;
int main()
{
 const int SIZE = 5;//number of characters to test in a set
 string bar = "100101010010001";
 string tempstring;
 int count = 0;
 while (count < bar.length())
 {
 int test = 0;
 tempstring = bar.substr(count, SIZE);//split bar string into next five characters
 for (int index = 0; index < SIZE; index++)
 {
 if (tempstring[index] == '1')
 {
 ++test;
 }
 }
 if (test != 2)
 {
 cout << "Invalid bar code!\n";
 exit(1);
 }
 count += 5;
 }
 cout << "Valid bar code!\n";
}

Answer 1

You can do without creating a temp string, simply by using bar[count + index] instead of tempstring[index]. Also, the while loop would be more natural as a for loop. Like this, with some other improvements:

for (int count = 0; count < bar.length(); count += 5)
{
 int ones = 0;
 for (int index = 0; index < SIZE; index++)
 {
 if (bar[count + index] == '1')
 {
 ++ones;
 }
 }
 if (ones != 2)
 {
 cout << "Invalid bar code!\n";
 return 1;
 }
}

Answer 2

• The constant SIZE and its comment don't quite seem consistent. A name such as size would correspond to an actual size of, say, a structure. But with such a comment, it doesn't seem to be for that. Consider renaming the constant to something more accurate.

• bar should be const since it's not modified.

• test seems like an ambiguous name for an integer variable. It sounds more like a Boolean test flag, but that's not the case here. Consider renaming it to something more accurate.

• Since tempstring is only used within the while loop, it can just be initialized there. It doesn't need to be declared before it.

• You shouldn't mismatch integer types when doing comparisons:

  while (count < bar.length())
  

  Since length() returns an std::size_t, which is an unsigned type, have count be of type std::size_t as well. If you had compiler warnings up, then this should've been reported.

• This may be a small nit-pick, but you can just return 1 instead of calling exit() in main().

Answer 3

• Last substring may contain less than SIZE characters. I suppose you want to reject such strings too.

• string.substr() creates a new object. It doesn't matter in a toy program; in general I recommend to avoid creating temporary objects.

• You may want to consider std::accumulate() instead of spelling out the counting loop.