a better way to write this?

[DonRamon069]

I've been learning for a while and was asked to do this program which I did but I feel like it could've been written in a more efficient way. Could someone at least tell me if what I wrote is decent/understandable? The program works as intended so I guess that's something. Thanks in advance.

Given a highway number, indicate whether it is a primary or auxiliary highway. If auxiliary, indicate what primary highway it serves. Also indicate if the (primary) highway runs north/south or east/west.

#include <iostream>
using namespace std;

int main() {
   int highwayNumber;

   cin >> highwayNumber;

   //check if highwayNumber's length is 1, and its digit is 5
   if ( (to_string(highwayNumber).length() == 1) && (to_string(highwayNumber).at(0) == '5') ) {
      cout << "I-" << highwayNumber << " is primary, going north/south." << endl;
   }
   //check if highwayNumber's length is 3, and its second digit is 0
   else if( (to_string(highwayNumber).length() == 3) && (to_string(highwayNumber).at(1) == '0') ) {
     cout << "I-" << highwayNumber << " is auxiliary, serving I-" << to_string(highwayNumber).at(2) << ", going north/south." << endl;
   }
   //check if highwayNumber's length is 2, and its second digit is 5
  else if ( (to_string(highwayNumber).length() == 2) && (to_string(highwayNumber).at(1) == '5') ) {
      cout << "I-" << highwayNumber << " is primary, going north/south." << endl;
   }
   //check if highwayNumber's length is 2, and its second digit is 0
   else if ( (to_string(highwayNumber).length() == 2) && (to_string(highwayNumber).at(1) == '0') ) {
      cout << "I-" << highwayNumber << " is primary, going east/west." << endl;
   }
   //check if highwayNumber's length is 3, and its third digit is 5
   else if( (to_string(highwayNumber).length() == 3) && (to_string(highwayNumber).at(2) == '5') ) {
     cout << "I-" << highwayNumber << " is auxiliary, serving I-" << to_string(highwayNumber).at(1) << to_string(highwayNumber).at(2) << ", going north/south." << endl;
   }
   //check if highwayNumber's length is 3, and its third digit is 0
   else if( (to_string(highwayNumber).length() == 3) && (to_string(highwayNumber).at(2) == '0') ) {
     cout << "I-" << highwayNumber << " is auxiliary, serving I-" << to_string(highwayNumber).at(1) << to_string(highwayNumber).at(2) << ", going east/west." << endl;
   }

   else {
     cout << highwayNumber << " is not a valid interstate highway number." << endl;
   }

   return 0;
}

[Handy Andy]

Hello DonRamon069,

It is possible that your code may work. I have not tested it yet.

The first thing you need to understand is that all odd numbered interstate highways start in the South and run North. And all even numbered interstate highways start on the West coast and run West to East.

The lower odd numbered highways start at the West coast and go East to the higher numbers. E.G., I-5 is on the West side of California and I-95 is on the East coast. One exception I can think of is I-99 which is in about the middle of Pennsylvania and mostly in the northern half of the state.

The same is true for the even numbered highways with I-10 just above New Orleans and I-94 in North Dakota.

In your if/else if statements you are going to a lot of work changing the number into a string for no real reason. Replacing the first if statement with if (highwayNumber % 2 == 1) all you would need with this is an else. Since your only 2 choices are an odd number or an even number.

This is mostly your original code with some blank lines, comments and changing the {} style. You should find that it is much easier to read with the different style of {}s and proper indentation. For different styles of {} see https://en.wikipedia.org/wiki/Indentation_style#Brace_placement_in_compound_statements
Of all the styles I like the "Allman" style.

I will load up the program and make the changes then if I find anything else I will let you know.

Andy

[Ganado]

Replacing the first if statement with if (highwayNumber % 2 == 1) all you would need with this is an else. Since your only 2 choices are an odd number or an even number.

That changes the logic of the program, since entering 7 would now be valid when it wasn't before.

One thing is you don't need to repeat "to_string(highwayNumber)" a dozen times. Save it in a variable.

   cin >> highwayNumber;
   string highway = to_string(highwayNumber); 

You also don't necessarily need to even save it as a string variable to begin with, if you manipulate the digits using / and %, but from a code clarity standpoint, I'd say using strings is clearer.

Btw: Since you're already checking the string's length, you can just do string[i] instead of string.at(i), because bounds checking is not needed.

If you really want to condense the code, there's probably something really clever you could do with mapping the possible inputs to the limited set of outputs, but I couldn't think of an easy way off the top of my head.

[DonRamon069]

Thank you both for your answers!

I will try and edit it with the tips you gave me.

[Handy Andy]

Hello DonRamon069,

Apparently I am wrong. A number of years ago when I was driving a truck I was taught that the roads run South/North and West/East. I always thought this was backwards, but that is what I was being told at the time. Now I see that it was wrong. https://en.wikipedia.org/wiki/Interstate_Highway_System#Primary_(one-_and_two-digit)_Interstates

You may want to make highway number a string because of "I-35W" and "I-35E". and some day there may even be an "I-69W", "I-69E" and "I-69C" to deal with. The 2 "I-35"s may never change because they have been in Texas to long. Since the number ends with a letter it is easy to change the string into a number because it would stop at the letter. This would also be useful for 3 digit interstate numbers or you could use Ganado's suggestion of using (/ and %) to check the first digit.

Another problem I see, and as Ganado pointed out, "I-5" is the lowest number and there is nothing between "I-5" and "I-15", so 7 - 13 would be invalid numbers. Also there is nothing between "I-15" and "I-25".

A "std::map","std::list" or even a "std::vector" could be used to hold all valid interstate numbers. This way you could check if the entered number is valid before you determine if it is odd or even.

Andy

[jonnin]

what is code without good puns? use the std::map, of course.

[againtry]

@OP

From that same https://en.wikipedia.org/wiki/Interstate_Highway_System the numbering system, for primary, major primary and auxiliary highways, is as follows:

East/West even number
North/South odd number

Primary highways are a 1 or 2 digit number (P - PP), a major primary is divisible by 5

Auxiliary highways are a 3 digit number made up of a single digit followed by the 1 or 2 digit number (A0P - APP)of its associated primary

Integers along with judicious use of % and / operators are much better than strings

[againtry]

#include <iostream>
#include <string>

//using namespace std;

int main()
{
    std::string direction;
    std::string type;
    
    int highwayNumber;
    
    std::cout << "Please enter a highway number: ";
    std::cin >> highwayNumber;
    
    if (highwayNumber > 999 or highwayNumber < 1)
    {
        std::cout << "Invalid highway number\n";
        return - 1;
    }
    
    // A) DIRECTION
    if (highwayNumber % 2 == 0)
        direction = "E/W";
    else
        direction = "N/S";
    
    // B) TYPE
    if(highwayNumber < 100)
    {
        type = "a primary";
        {
            if (highwayNumber % 5 == 0)
                type += " major";
        }
    }
    else
        type = "an auxiliary";
    
    // OUTPUT
    std::cout
    << "I-" << highwayNumber
    << " is a " << type
    << " highway running " << direction;
    
    // PRIMARY SERVED BY AUXILIARY
    if(highwayNumber > 99)// EDIT: could be 100 depends if there is no highway 0
    {
        std::cout << " serving I-" << highwayNumber % 100;
    }
    
    std::cout << '\n';
    
    return 0;
}

[mbozzi]

@DonRamon, I think your code is essentially fine. Very straightforward - which is excellent.

While the code you've written is awful from an efficiency point-of-view, that cost is mostly insignificant - certainly dwarfed by the cost of starting the process, certainly on Windows, and certainly when caches are cold.

Still, I'd suggest some changes.

0. Check whether std::cin succeeded when reading;
1. Get rid of the repeated calls to to_string;
2. Add #include <string>, which is required for portability's sake;
3. Remove comments which offer no additional information, e.g.
//check if highwayNumber's length is 1, and its digit is 5
4. Potentially, sort the if-clauses logically; and
5. Use '\n', not std::endl, to terminate lines, unless you need to follow every newline written with an explicit flush.

[dhayden]

Since you always treat highway as a string, why not enter it as a string? Or at the very least, convert it to a string after reading it.

This is an excellent example of when data is better than code. Just put the logic into a table and have your code run through the table until it finds a match.

#include <iostream>
using namespace std;

struct HighwayTable {
    // If the length matches, and the character at position is value...
    unsigned length;
    unsigned position;
    char value;

    // .. then
    bool isPrimary;		// primary or auxilliary?
    bool north_south;		// runs north/south or east/west?
    unsigned auxPos, auxLen;	// if aux, then pos and len of substring for main highway
};


int main(int argc, char **argv) {
    HighwayTable table[] = {
	// len	pos	value	primary? N/S	auxPos	auxLen
	{1,	0,	'5',	true,	true,	0,	0 },
	{3,	1,	'0',	false,	true,	2,	1 },
	{2,	1,	'5',	true,	true,	0,	0 },
	{2,	1,	'0',	true,	false,	0,	0 },
	{3,	2,	'5',	false,	true,	1,	2 },
	{3,	2,	'0',	false,	false,	1,	2 }
    };

    string highway;

    while (cin >> highway) {
	bool found = false;
	for (HighwayTable &t : table) {
	    if (highway.length() == t.length && highway[t.position] == t.value) {
		cout << "I-" << highway
		     << " is " <<  (t.isPrimary ? "primary" : "auxiliary")
		     << ", ";
		if (!t.isPrimary) {
		    cout << "serving I-" << highway.substr(t.auxPos, t.auxLen)
			 << ", ";
		}
		cout << "going " << (t.north_south ? "north/south" : "east/west") << ".\n";
		found = true;
		break;
	    }
	}
	if (!found) {
	    cout << highway << " is not a valid interstate highway number.\n";
	}
    }
   return 0;
}