Search/Delete/Validate in strings

[meltorizor]

Good day dear sirs and ladies,

for my university homework i have to write a code which checks certain input if it has the right size and the right digits/characters used. To be more specific, it should check the validity of a ISB number. For that, we should write subfunctions in which

1.) the entered ISB number is recorded as string and cleared of all hyphens (e.g.: 1234-5678-91 -> 1234567891) and the user informed if the ISB number has the correct amount of digits (which is 10) or not.

2.) the ISB number is checked if only digits and not letters were used except for the last digit which may be also a x or X in addition to a normal number

3.) the previous two functions are used to validate the ISB number and the user is informed whether the validation was succesful or not.

My idea was

for 1.) to create a string function (if something like this can be created) and get the length of the entered string. The length of the string shall be my "running" variable for a loop (from 0 to length of the string) in which i check each digit if it is a hyphen or not, and if yes, delete it via if else. After that, it should check for the right length.

Is it possible to write that in a function? If yes, how? Maybe something like this?

string(ISBN) {
	for (i = ISBN.length) {
        ...
	}

for 2.) to let the function from 1.) return the "cleansed" string into the function from 2.) where the new length of the string defines the new running variable to check every digit if it is a number or a char. (except for the last digit which may also be an x or X). After the check, it shall provide a return value for

3.) if the whole validation process was succesful or not. The user shall be informed about the result via cout.

In my head, the idea of how i want to do it is clear, question is if it's possible the way i described it. Therefore, my C++ skills do not suffice. I think boolean functions may also work but i know even less about them. I'm a bloody noob and help would be appreciated. I don't want the code, just hints how to get started or where i can read how it's done.
The solution with subfunctions is demanded by the professor.
Thanks in advance for reading and helping. Please accept my imaginary potato for this long post. :>

[meltorizor]

For the first function, i wrote this.

string length(string ISBN) {
	int i;
	for (i = ISBN.length, i > 0) {
		if (ISBN[i] == "-") {
			ISBN.erase(i);
		}
		else {
			i--;
		}

	}
	if (ISBN.length == 10) {
		return (ISBN);
	}
	else {
		return false;
	}
	
}

But it's not working as it is...

[SamuelAdams]

I'm thinking something more like this. Sorry for poor formatting.

string(ISBN) {
	for (i = 0; i < ISBN.size(); i++) 
{
// check each char
 if (isdigit(ISBN[i]))
{
// number is good check next one until end
}
else
{
// not a digit, exit and continue.
}
}

[Troaat]

Hi @meltorizor,
Since you wrote "string()", I will assume you want to use std::string only, no char arrays.
If so, then here is the answer:
string is a class, and when defining functions it's always the same:
(data type/class name) functionName(data type 1 parameter1, data type 2 parameter2).
You can do like this:

string functionCountComponents(string ISBN) //the function has "string" as data type
//it's going to return a string if asked to.  

The name of the function is determined by you. Writing the function's name is programmer's call. You can call it "saft" or anything else, but "string" is something that exists: a name for a kind of variable.
string() is something else (a constructor, and it's already defined to do something).
Looking here you can see multiple things.

http://www.cplusplus.com/reference/string/string/

Such as:

for (int i=ISBN.length()) //length() is a function that returns the number of components from ISBN
{
   ...
}

But based of what I understand from 1.), it should look more like this:

int functionCountComponents(string ISBN)
{
    for (int i=0; i<ISBN.length();) //ISBN.length() returns the number of characters at each execution
    {
        if () //check character using "ISBN[i]"
        {
            ISBN.erase(i, 1); //this eliminates the hyphen on position i.
        }
        else if () //write the alternative
        {
            i++;
        }
    }
    return ISBN.length(); //if you want to return the number of digits that there are in the string
}

Or, if you want to return the whole string:

string functionCleanString(string ISBN)
{
    for (int i=0; i<ISBN.length();)
    {
        if ()
        {
            ISBN.erase(i, 1);
        }
        else if ()
        {
            i++;
        }
    }
    if (ISBN.length()==10)
    {
        return ISBN;
    }
    else
    {
        cout<<"Invalid string.";
    }
}

[meltorizor]

Ok thanks, is it always better to run from zero to variable instead from variable to zero?
I wrote a "complete" but non working program, i think it is clear how i wanted to structure it but it has more bugs than lines...

#include <iostream>
#include "stdafx.h"
#include "conio.h"
#include "string"
using namespace std;


string length(string ISBN) {
	int i;
	for (i = ISBN.length, i > 0) {
		if (ISBN[i] == "-") {
			ISBN.erase(i);
		}
		else {
			i--;
		}

	}
	if (ISBN.length == 10) {
		return (ISBN);
	}
	else {
		return false;
	}
	
}

string check(string ISBN) {
	int j;
	for (j = ISBN.length, j > 0) {
		if (isdigit(ISBN[j])) {
			j--;
		}
		else {
			return false;
		}
	}
}

string validation(string ISBN) {
	if (length && check) {
		cout << "The ISB number is valid!\n";
	}
	else {
		cout << "The ISB number is not valid!\n";
	}
	}



int main() {
	string isbn;
	cout << "Please enter the ISB number for validation: \n";
	cin >> isbn;
	validation(isbn);
	_getch;
	return 0;	
}

[jlb]

First when you tell the compiler you're going to return some type of value you must return that type value. Ie. you can't tell the compiler that your going to return a string then return a bool. Next be careful when using return statements inside if/else clauses, be sure that both clauses return the correct values. It is usually better to avoid placing more than one return statement in a function if possible.

Also since you're using std::string you may want to look up some documentation for some of the string helper functions like find_first_of(), find_first_not_of(), and erase.


If you have compile errors you should post the complete compiler error messages, all of them, exactly as they appear in your development environment.

[AbstractionAnon]

For the first function, i wrote this.

Line 16: You're trying to return a bool for a function that returns a std::string.

Here's a cleaned up version of your length function.

//  Pass ISBN by reference since we may change it.
//  Return a bool indicating if length is correct.
bool length (string & ISBN) 
{   //  length is a function call.  Needs ()
    //  Need to subtract 1 from length to get last char
    //  Need to test the zeroth character
    //  Need ; after second term in for
	for (int i = ISBN.length()-1; i >= 0; ) 
	{   if (ISBN[i] == '-')     //  Need to test against char literal
	    {   ISBN.erase(i);
		}
		else 
		{   i--;
		}
	}
	return (ISBN.length() == 10);	//  Need () for function call
}

[gunnerfunner]

#include <iostream>
#include <string>
#include <algorithm>

int main()
{
    std::string input;
    bool match = false;
    while (!match)
    {
        std::cout << "Enter ISBN number: \n";
        getline(std::cin, input);
        input.erase(std::remove(input.begin(), input.end(), '-'), input.end());
        //step 1: remove all hyphens
        if(input.size() == 10)//step 2: then check we've got 10 characters
        {
            if(std::all_of(input.begin(), input.end()-1, ::isdigit))
            {
                if(isalnum(input.back()))//step 3: finally, check the last character
                {
                    std::cout << "Output successful \n";
                    match = true;
                }
                else
                {
                    std::cout << "Last character is not alpha-numeric, try again \n";
                }
            }
            else
            {
                std::cout << "Non digits not allowed in any of first nine characters, try again \n";
            }
        }
        else
        {
            std::cout << "ISBN length is inaccurate, try again \n";
        }
    }
    std::cout << input << '\n';
}

Note: this program will accept input of the format 123-456-789-f i.e with 3 hyphens as valid. If number of hyphens cannot be more than 2 or has to be exactly at the 5th and 10th characters of the input string (which should then be a size 12) then you can have an extra layer of validation. Use std::count to count number of hyphen's and std::distance to return the position of the hyphens

[dhayden]

This follows what the prof is asking for, but I don't really like it:

#include <iostream>
#include <string>

using std::cin;
using std::cout;
using std::string;

// remove dashes from an isbn and convert letters to upper case
string normalize(const string &isbn)
{
    string result;
    for (char ch : isbn) {
	if (ch == '-') {
	    continue;		// skip it
	}
	if (isalpha(ch)) {
	    ch = toupper(ch);	// make sure it's uppercase
	}
	result += ch;
    }
    return result;
}


bool checkNumDigits(string &isbn)
{
    isbn = normalize(isbn);

    // Count digits
    size_t numDigits = 0;
    for (char ch : isbn) {
	if (isdigit(ch)) {
	    ++numDigits;
	}
    }
    return numDigits == 10;
}


bool checkOnlyDigits(string &normalized_isbn)
{
    for (size_t i = 0; i< normalized_isbn.size(); ++i) {
	char ch = normalized_isbn[i];
	if (!isdigit(ch)) {
	    if (i != normalized_isbn.size()-1) {
		return false;	// not the last char
	    }
	    if (ch != 'X') {
		return false;	// last char isn't an 'X'
	    }
	}
    }
    return true;
}

bool validate(string &isbn)
{
    if (!checkNumDigits(isbn)) {
	cout << isbn << " doesn't have 10 digits\n";
	return false;
    }
    if (!checkOnlyDigits(isbn)) {
	cout << isbn << " has extra characters\n";
	return false;
    }
    return true;
}


main()
{
    string str;
    while (cin >> str) {
	if (validate(str)) {
	    cout << str << " is a valid isbn\n";
	}
    }
}

The problem is that you're being asked to write poorly structured code:
- Functions should do just one thing. Function 1 does 2 things instead of 1. It (1) normalizes the isbn by removing hyphens, and (2) checks the number of digits. Actually the prof is asking it to do a third thing: report to the user.
- The normalized string from function 1 should be fed into function 2, but that isn't made clear.
- The validate function does 2 things: validate and report to the user.

Functions should do one thing.
Don't mix computation and presentation. In this case the validation function should not print out anything. That's because it might be called for a variety of different reasons. Let the caller decide what to do if the string isn't valid.

Here is a version where each function does one thing. The second function is eliminated and its logic, which is pretty trivial, is moved inside validate(). Validate returns an enum indicating the status of the number. The main function reads numbers from the input, calls validate and reports on the result.

#include <iostream>
#include <string>

using std::cin;
using std::cout;
using std::string;

// remove dashes from an isbn and convert letters to upper case
string normalize(const string &isbn)
{
    string result;
    for (char ch : isbn) {
	if (ch == '-') {
	    continue;		// skip it
	}
	if (isalpha(ch)) {
	    ch = toupper(ch);	// make sure it's uppercase
	}
	result += ch;
    }
    return result;
}


// return the number of digits in the string
size_t numDigits(string &str)
{
    size_t numDigits = 0;
    for (char ch : str) {
	if (isdigit(ch)) {
	    ++numDigits;
	}
    }
    return numDigits;
}


enum ValidationCode {
    Ok,				// validation passed
    NumDigits,			// wrong number of digits
    ExtraChars			// extra characters in isbn
};

enum ValidationCode validate(string &isbn)
{
    string normal = normalize(isbn);
    size_t count = numDigits(normal);
    if (count != 10) {
	return NumDigits;
    }
    if (normal.size() == 10 ||
	normal.size() == 11 && normal[10] == 'X') {
	return Ok;
    }
    return ExtraChars;
}


main()
{
    string str;
    while (cin >> str) {
	switch (validate(str)) {
	case Ok:
	    cout << str << " is a valid isbn\n";
	    break;
	case NumDigits:
	    cout << str << " doesn't have 10 digits\n";
	    break;
	case ExtraChars:
	    cout << str << " has extra characters\n";
	    break;
	default:
	    cout << "ERROR: validate(" << str << ") return an unknown status\n";
	    break;
	}
    }
}

[jlb]

I also disliked the idea of the functions doing more than what is necessary and slightly altered the "requirements".

#include <string>
#include <iostream>

std::string remove_dashes(const std::string& isbn);
bool check_format(const std::string& isbn);

int main(int argc, char **argv)
{
    std::string ISBN;
    if(argc == 2)
        ISBN = argv[1];
    else
        ISBN = "1234-5678-9x";

    bool is_valid = false;

    std::string ISBN_no_dashes = remove_dashes(ISBN);
    if(ISBN_no_dashes.length() == 10)
        is_valid = check_format(ISBN_no_dashes);

    if(!is_valid)
        std::cout << "Error ISBN number: " << ISBN << " is not valid.\n";
    else
        std::cout << "The ISBN number: " << ISBN << " is valid!\n";

    return 0;
}

/* Return a new string to preserve the original string contents. */
std::string remove_dashes(const std::string& isbn)
{
    std::string isbn_copy = isbn;
    std::size_t found;

    while ((found = isbn_copy.find_first_of("-")) != std::string::npos)
        isbn_copy.erase(found, 1);
    return isbn_copy;
}

bool check_format(const std::string& isbn)
{
    bool return_value = false;
    size_t found = isbn.find_first_not_of("0123456789");
    if(found == std::string::npos)
        return_value = true;
    else if(found == isbn.length() - 1 && toupper(isbn.back()) == 'X')
    { // Last digit may be X or x  or digit all others must be digits.
        return_value = true;
    }

    return return_value;
}

[meltorizor]

Thanks a lot for all the answers, i'll try to comprehend your solutions as there are many commands i don't know (yet :>). I'll come up with a corrected form i hope.

[meltorizor]

As mentioned i don't copy your code because it's not helping me learning programming and the commands. A plea would be to comment everything except the very basic commands and maybe describe how they work, respectively what input they demand, what output they give, how i can work with the result it's giving etc. Like i said, i'm a bloody noob, and i'm afraid i don't have much time to read about all the new commands because in one month the written exam phase begins with five exams like higher mathematics, technical mechanics and shit like that.^^
Ok thanks dear sirs, here is what i could come up with:

#include <iostream>
#include "stdafx.h"
#include "conio.h"
#include "string"
#include "stdio.h"
using namespace std;

string length(string ISBN) {
	int i;
	string normalized;
	for (i = 0, i < ISBN.length()) {
		if (ISBN[i] == "-") {
			ISBN.erase(i);
		}
		else {
			normalized = normalized += ISBN[i];
			i++;
		}
	}
}
	
string check(string normalized) {
	int j;
	string compared;
	for (j = 0; j < normalized.length()) {
		if (isdigit(normalized[j])) {
			compared = compared += normalized[j];
			j++;
		}
		else
			j++;		
	}
}

bool validation {
	if ((normalized.length == 10) && (compared.length == 10)) {
		return true;
		}
	else {
		return false;
		}
}
	
int main() {
	string isbn;
	cout << "Please enter the ISB number for validation: \n";
	cin >> isbn;
	if (validation == true) {
		cout << "The entered ISBN is valid.\n";
	}
	else
		cout << "The entered ISBN is not valid.\n";
	_getch;
	return 0;	
}

As you can see with the error messages, this program is a pure disgrace as it has nearly as much errors as lines...


Severity Code Description Line Suppression State
Warning C4018 '<': signed/unsigned mismatch 11
Warning C4018 '<': signed/unsigned mismatch 25
Error C2446 '==': no conversion from 'const char *' to 'int' 12
Error C2447 '{': missing function header (old-style formal list?) 36
Error C2447 '{': missing function header (old-style formal list?) 39
Error C2447 '{': missing function header (old-style formal list?) 44
Error (active) expected a ';' 11
Error (active) expected a ';' 25
Error (active) expected a ';' 44
Error (active) expected an expression 36
Error (active) operand types are incompatible ("char" and "const char *") 12
Error C2059 syntax error: 'else' 39
Error C2059 syntax error: 'if' 36
Error C2059 syntax error: '}' 42
Error C2143 syntax error: missing ')' before '{' 11
Error C2143 syntax error: missing ';' before ')' 11
Error C2143 syntax error: missing ';' before ')' 25
Error C2143 syntax error: missing ';' before '{' 11
Error C2143 syntax error: missing ';' before '{' 36
Error C2143 syntax error: missing ';' before '{' 39
Error C2143 syntax error: missing ';' before '{' 44
Error C2143 syntax error: missing ';' before '}' 42


Thanks for your patience.

I don't get these error messages. Especially the missing";" ones. I don't put semicolons into loop requirements, do i?

[dhayden]

I understand your frustration. Learning to program is like learning a foreign language. You need a fairly big vocabulary and understanding of the syntax before you can do much of anything.

A few notes regarding the syntax errors:
- work on them from the top to the bottom. Fix 1 or two and then recompile the program. This is very important because it's common for one syntax error to throw the compiler off track, causing it to complain of dozens more. SO it looks like you have 15 errors when in reality there is just one.

- Pay close attention to syntax. Computer programming requires much more precise syntax than human writing does. Every character matters.

- Pay close attention to your functions and what they are supposed to do. I recommend that you add a comment to the top of each function explaining what it's supposed to do.

- The length of a string str is retrieved by a calling str.length(), not str.length. It's a method call, not a member variable.
Okay, now some specifics:

Revisit your for loops. The syntax is for (initial; test; increment) statement
- Semicolons separate the 3 parts, not commas.
- You must have all three parts, but any of them can be empty. What that really means is there must be exactly two semicolons inside the parentheses
- Functions must return a value. It's not enough to compute sometime, you must explicitly tell the compiler what value to return to the caller.

- In the code that I and others have posted, there are two general methods of removing the hyphens from a string. Method 1 is to erase the dashes from the string until there are no more in it. Method 2 is to copy the old string to a new string, character by character, but skipping the dashes. Your code at lines 12-18 is trying to do both. Decide on one method and then stick to it.

[gunnerfunner]

OP: since you have several options, and nearly all of them well-commented throughout as far as I can see, I suggest you pick one solution and try and understand it thoroughly, ask follow-on questions if need be and get it working rather than trying to pick bits from here and there. Sometimes its not going to work because each program tackles the problem in its own way

Later, when you have more time, you can circle back upon the remaining posts and try and pick up new learning stuff from them

[meltorizor]

Thanks for your advice, i'll be back if i can report something new.