Problem with boolean variable in email validation program

Question

I am trying to create an email validation program without using the regex library. In one of my functions, I want to return a Boolean to check if there is an @ sign in the email address and also if it is in a valid position (the @ sign cannot be one of the first three characters of the string). However I am having problems with it because every time I run the program by entering in an email address with the @ sign in an invalid position, it keeps telling me that the email is valid. Please help!

valid = checkEmail(email); //function call 

if(valid == true)
  cout << "Your email is valid!" << endl;

else
  cout << "Your email is invalid!" << endl;


bool checkEmail(string email)
{
  int counter;
  int length;
  bool firstThree; //checks to make sure @ is not in the first three chars

  counter = 0;
  length = email.length();
  firstThree = false;

  for(int i = 0; i < length; i++)
  {
    if(email[i] == '@')
      counter++;
  }

 for(int y = 0; y < length; y++)
 {
   if(email[0] == '@' || email[1] == '@' || email[2] == '@')
      firstThree = true;

   else
       firstThree = false;

 }

 cout << "\n" << counter << endl; //check to see if counter works

 if(counter != 1 && firstThree == true)
   return false;

 else
   return true;
}

It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) and [Debugging Guide](http://idownvotedbecau.se/nodebugging/) — NathanOliver, May 03 '19 at 16:23
`if(counter != 1 && firstThree == true)` will return false only if your email has no @ ind it **AND** if the @ is in the first threee characters. Also notice that the for loop around the firstThree checking is doing nothing usefull. — Yastanub, May 03 '19 at 16:26
https://hackernoon.com/the-100-correct-way-to-validate-email-addresses-7c4818f24643 — Jarod42, May 03 '19 at 16:29
Also have a look at [std:.string::find_first_of](https://en.cppreference.com/w/cpp/string/basic_string/find_first_of) which resturns the first occurence of a string. With that your task would be very simple. Have a look at `std::basic_string` in general as well when you want to do something like that since it provides many useful ustilities — Yastanub, May 03 '19 at 16:34

score 0 · Answer 1 · answered May 03 '19 at 18:19

I guess you need to define the function bool checkEmail(string email) at the begining of the program. Basically flip if else and the function definition.

bool checkEmail(string email)
{
  int counter;
  int length;
  bool firstThree; //checks to make sure @ is not in the first three chars

  counter = 0;
  length = email.length();
  firstThree = false;

  for(int i = 0; i < length; i++)
  {
    if(email[i] == '@')
      counter++;
  }

 for(int y = 0; y < length; y++)
 {
   if(email[0] == '@' || email[1] == '@' || email[2] == '@')
      firstThree = true;

   else
       firstThree = false;

 }

valid = checkEmail(email); //function call 

if(valid == true)
  cout << "Your email is valid!" << endl;

else
  cout << "Your email is invalid!" << endl;

score 0 · Answer 2 · edited Jun 20 '20 at 09:12

There seem to be a lot of beginner mistakes here, so I suggest learning the basics of programming and c++ first and like others suggested, use a debugger. Here are some of the mistakes:

Function definition

c++ is not like other common languages in the sense that functions need to be defined before they can be used. This means that you either need to move your checkEmail function above the function call, or create a separate definition above the function call like:

bool checkemail(string email);

as an exmaple.

Incorrect if-statement logic; Unnecessary for loop:

I'm assuming that based on how emails are formatted, you want the checkEmail function to return false if it doesn't match the correct formatting, and based on what your function currently does, that means that it will return false if the first three characters are @ or if there isn't exactly one @ symbol in the email. However, you used a && operator, which signifies and, meaning it will return true even when you don't want it to (much like how @Yastanub stated in his first comment). Better yet, that entire logic and if statement can be simplified greatly using std::string::find with a while loop and a vector (similar to this method):

vector<size_t> posVec;
size_t pos = email.find('@', 0); //note that this can be an int instead of size_t, but it can cause stack overflow with bigger numbers
while(pos != string::npos){
    posVec.push_back(pos);
    pos = email.find('@', pos+1);
}
switch(posVec.size()){
    //if there is only one @ symbol found
    case 1:
        bool firstThree = false;
        for(int i = 0; i <= 2; i++)
            //if there is only one @ but it's in the first 3 positions, the email isn't valid
            if(posVec[0] == i)
                firstThree = true;
        return !firstThree;
    //otherwise the email doesn't work
    default:
        return false;
}

Remember to include the required libraries in order for this part to work:

#include <string>
#include <vector>

This also eliminates the second for loop in your function that is useless because the variable y is not being used, and the counter that was being used for testing

Another note

Using if (valid == true) is unnecessary as well. You can just use if (valid) because of how booleans work with if-statements.

Problem with boolean variable in email validation program

2 Answers2

Function definition

Incorrect if-statement logic; Unnecessary for loop:

Another note