HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

U.S. Postal Service zip code and bar code class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
barandzipservicecodeclasspostal

Problem

This program accepts either a 27 digit binary bar code of type string, or a 5 digit integer zip code of type int. The program can return a binary bar code of type string from a 5 digit zip code of type int or vise versa. I would like to know if this is a good implementation of a class. What would you suggest? I am also looking for any suggestions on simplifying the loops.

PostNet.h

```
/*
POSTNET:

Prior to 2009 the bar code on an envelope used by the U.S.Postal Service represented
a five(or more) digit zip code using a format called POSTNET. The bar code consists of
long and short bars. For this program, we will represent the bar code as a string of
digits. The digit 1 represents a long bar, and the digit 0 represents a short bar.
Therefore, a bar code would be represented in our program as follows:

110100101000101011000010011

The first and last digits of the bar code are always 1. Removing these leaves 25
digits. If these 25 digits are split into groups of five digits each then we have the
following:

10100 10100 01010 11000 01001

Next, consider each group of five digits. There always will be exactly two 1’s in each
group of digits. Each digit stands for a number. From left to right, the digits encode
the values 7, 4, 2, 1, and 0. Multiply the corresponding value with the digit and
compute the sum to get the final encoded digit for the zip code. The following table
shows the encoding for 10100.

Bar Code Digits: 1 0 1 0 0
Value: 7 4 2 1 0
Product of
Digit * Value: 7 0 2 0 0

Zip Code Digit: 7+0+2+0+0 = 9

Repeat this for each group of five digits and concatenate to get the complete zip code.
There is one special value. If the sum of a group of five digits is 11, then this
represents the digit 0 (this is necessary because with tw

Solution

Bad comments

//validate bar code format
   bool validateBar(std::string) const;
   //convert integer zip code to string bar code
   std::string zipToBar(int) const;
   //convert string bar code to five digit zip code
   int barToZip(std::string) const;


Don't think any of those comments are actually required. Bad comments are worse than no comments. Your comments should explain WHY the code should explain how and be self documenting.

The problem with too many comments is that the code and comments will diverge over time. If you them come across code that does not behave like the comments describe which is correct? Do you fix the comments to match the code or do you fix the code to match the comments. If you describe WHY you need the information and leave the code to describe how then you do not fall into this maintenance trap.
Prefer throw to exit

void PostNet::setBarCode(std::string bar)
{
   if (validateBar(bar))
      barcode = bar; 
   else
      exit(1);
}
void PostNet::setBarCode(int zip)
{
   std::string bar = zipToBar(zip);
   if (validateBar(bar))
      barcode = bar;
   else
      exit(1);
}


It's a problem. Throw an exception. If the code does not explicitly try and handle the exception then the program will exit (so same result). But if you want to explicitly handle the error, you can now do so.
No need to test when you get

std::string PostNet::getBarCode() const
{
   if (validateBar(barcode))
      return barcode;
   else
      exit(1);  
}
int PostNet::getZipCode() const
{
   if (validateBar(barcode))
      barToZip(barcode);
   else
      exit(1);
}


You have validated that the bar code is valid when you set it. So there is no need to validate it when you get it. Just return the value.
Getters/Setters

Getters and setters are bad programming practice. They expose internal representation of your code. They also are rarely needed in well designed code. But there are situations when you need them. But you have not provided enough context on how the value will be used so I can not say it is bad (just probably bad).
Mutable

Your class has the perfect place where a mutable object can be used.

A mutable object is not part of the state of the object. But can be used to store a cached value. Here your state is held in the bar code. But you could also store the zip value in a mutable (initially it is zero to mark it as not set). But the first time you call getZip() you calculate it and store the value in a mutable member. Subsequent calls then just retrieve the zip from the stored value rather than re-calculating on each call.

Code Snippets

//validate bar code format
   bool validateBar(std::string) const;
   //convert integer zip code to string bar code
   std::string zipToBar(int) const;
   //convert string bar code to five digit zip code
   int barToZip(std::string) const;
void PostNet::setBarCode(std::string bar)
{
   if (validateBar(bar))
      barcode = bar; 
   else
      exit(1);
}
void PostNet::setBarCode(int zip)
{
   std::string bar = zipToBar(zip);
   if (validateBar(bar))
      barcode = bar;
   else
      exit(1);
}
std::string PostNet::getBarCode() const
{
   if (validateBar(barcode))
      return barcode;
   else
      exit(1);  
}
int PostNet::getZipCode() const
{
   if (validateBar(barcode))
      barToZip(barcode);
   else
      exit(1);
}

Context

StackExchange Code Review Q#92298, answer score: 12

Revisions (0)

No revisions yet.