Essentially, the code is meant to validate, find the manufacture origin, and manufacture date of a vehicle using the VIN number. I had written the "valid" function first and after it worked I moved onto the "origin" and "year" functions. Once they worked, I tested everything together and suddenly the "valid" function would return a "true" value even when it wasn't supposed to. I've tried to rewrite it, but as far as I can tell, only the if statement regarding length and the for loop actually work. What I don't understand is why it's not working correctly.

To go into more detail, the VIN number must be 17 characters long, must have only digits and uppercase letters, and must exclude the letters "I," "O," "Q," "U," and "Z." Again, the arrangement I had worked, but after writing the subsequent two functions it ceased to return a false value when the above criteria was not met.

Can anyone point out what I'm doing wrong? I've been over this numerous times and I can't seem to figure out where I'm missing something.

Edit: Added a missing digit to the VIN array.

using namespace std;

#include <cstring>
#include<cctype>
#include <iostream>
#pragma warning(disable : 4996)

bool valid(char[]);
void origin(char[], char[]);
int year(char[]);

int main() {

bool validity;

char vin[] = "1FTRW14W84KC76110";
char country[20];

cout << "Testing the VIN " << vin << endl;

validity = valid(vin);

if (validity == true) {
    cout << "\nVIN is valid" << endl;
}
else {
    cout << "\nVIN is not vaild." << endl;
}

origin(vin, country);

cout << "Year: " << year(vin) << endl;

return 0;
}

bool valid(char vin[]) {

bool result = false;

long long length = strlen(vin);

if (length == 17) {
    for (int i = 0; i < 17; i  ) {
        if (isalnum(vin[i])) {
            if (isupper(vin[i])) {
                if (vin[i] != 'I' && vin[i] != 'O' && vin[i] != 'Q' && vin[i] != 'U' && vin[i] != 'Z') {
                    return true;
                }
                else {
                    return false;
                }
            }
            else if (isdigit(vin[i])) {
                return true;
            }
            else {
                return false;
            }
        }
        else {
            return false;
        }
        
    }
}
else {
    result = false;
}

return result;
}

void origin(char vin[], char country[]) {

if (vin[0] >= 'A' && vin[0] <= 'H') {
    strcpy(country, "Africa");
    cout << "Origin: " << country << endl;
}
else if (vin[0] >= 'J' && vin[0] <= 'R') {
    strcpy(country, "Asia");
    cout << "Origin: " << country << endl;
}
else if (vin[0] >= 'S' && vin[0] <= 'Y') {
    strcpy(country, "Europe");
    cout << "Origin: " << country << endl;
}
else if (vin[0] >= '1' && vin[0] <= '5') {
    strcpy(country, "North America");
    cout << "Origin: " << country << endl;
}
else if (vin[0] >= '6' && vin[0] <= '7') {
    strcpy(country, "Oceania");
    cout << "Origin: " << country << endl;
}
else {
    strcpy(country, "South America");
    cout << "Origin: " << country << endl;
    }
}

int year(char vin[]) {

int y;

if (isdigit(vin[9])){
    y = 2000   (vin[9] - '0');
}
if (vin[9] <= 'M') {
    y = 2010   (vin[9] - 'A');
}
else{
    y = 1993   (vin[9] - 'P');
}

return y;
}

CodePudding user response：

As people have commented, you are mixing styles of fail checks with pass checks. It's good to pick one style, and then you can simplify the logic. Here is an example of all fail checks.

bool valid(char vin[])
{
    bool result = false;

    long long length = strlen(vin);

    if (length != 17)
    {
        return false;
    }
    for (int i = 0; i < 17; i  )
    {
        if (!isalnum(vin[i]))
        {
            return false;
        }
        if (islower(vin[i]))
        {
            return false;
        }
        if (vin[i] == 'I' || vin[i] == 'O' || vin[i] == 'Q' || vin[i] == 'U' || vin[i] == 'Z')
        {
            return false;
        }
    }
    return true;
}

CodePudding user response：

As Nathan points out this has never worked.

All the

return true;

Statements should read

result = true; // Continue checking...

You are getting bad positives by breaking too early on return true.

You tagged this c so in c one would rather be inclined to write it as something like.

bool is_valid(std::string vin){
    std::string forbidden = "IOQUZ";

    auto has_forbidden_it = std::find_first_of(vin.begin(), vin.end(), 
                                     forbidden.begin(), forbidden.end());
    if(has_forbidden_it != vin.end()){
       return false;
    }
    if(vin.size() != 17){
        return false;
    }
    for(char ch: vin){
        auto valid_ch = std::isdigit(ch) || std::isupper(ch);
        if(!valid_ch){
            return false;
        }
    }        
    return true;
}