12
\$\begingroup\$

I made some code to check that e-mails given apply to the right format, and I'm sure there must be a better way to do this.

import javax.swing.JOptionPane;

public class MailTest {
    public static void main (String [] args){
        String email="";
        int at1=0,at2=0;
        int dot1=0;
        boolean at = false, dot=false, domain=false, tld=false;

        do {
            email=JOptionPane.showInputDialog("Insert your email");

                //Checking that we only have one @
            for (int i=0; i<email.length();i++)
                if(email.charAt(i)=='@')        
                    at1++;
            if(at1!=1)
                at=false;
            else at=true;
            at1=0;
                //Getting @'s position on the screen
            for (int i=0;i<email.length();i++)
                if (email.charAt(i)=='@')
                    at2=i;

                //Checking the domain is not empty
            if ((at2+1)<email.length())
                if(email.charAt(at2+1)=='.')
                    domain=false;
                else domain=true;

                //Cheking if there is at least one dot
            for (int i=at2; i<email.length();i++)
                if(email.charAt(i)=='.')
                    dot1++;
            if(dot1==0)
                dot=false;
            else dot=true;

                //Getting last dot's position
            for (int i=0;i<email.length();i++)
                if(email.charAt(i)=='.')
                    dot1=i;

                //Checking there's some Top-Level Domain
            if (email.length()==dot1+1)
                tld=false;
            else tld=true;
        }while(at==false||domain==false||dot==false||tld==false);

        System.out.println("You made it!");
    }
}
\$\endgroup\$
  • 22
    \$\begingroup\$ I can be very brief here: don't. Of all the assumptions you've made here, only one is correct: there has to be an @ in the email address. Otherwise: it can have multiple dots, it can have no dots, it can have no TLD, it can have no domain, it can have IP addresses as domain, it can have quotes, it can have spaces, etc. When validating emails the correct approach is to check for the presence of an @ and hope for the best. You can always try to send an email and see if it bounces back. \$\endgroup\$ – Jeroen Vannevel Sep 3 '15 at 0:18
  • 6
    \$\begingroup\$ RFC5321, RFC5322 and RFC6531, have fun! :) \$\endgroup\$ – h.j.k. Sep 3 '15 at 0:52
  • \$\begingroup\$ I use C# so I can use EmailAddressAttribute directly, but Microsoft has a very nice Regular Expression you can look at here: referencesource.microsoft.com/… \$\endgroup\$ – ta.speot.is Sep 3 '15 at 4:03
  • \$\begingroup\$ Was this a normal question or is it off-topic here? It's my first and I don't wanna mess it up :) \$\endgroup\$ – Pablito Sep 3 '15 at 9:40
  • 1
    \$\begingroup\$ And just to show how bad the situation is: There are working email addresses in use out there, that do not comply to any RFC. Not every email provider implemented this check correctly before issuing addresses to their customers ... \$\endgroup\$ – linac Sep 3 '15 at 11:31
17
\$\begingroup\$

First, heed what Jeroen Vannevel said; it will save you much grief.

Of all the assumptions you've made here, only one is correct: there has to be an @ in the email address. Otherwise: it can have multiple dots, it can have no dots, it can have no TLD, it can have no domain, it can have IP addresses as domain, it can have quotes, it can have spaces, etc. When validating emails the correct approach is to check for the presence of an @ and hope for the best. You can always try to send an email and see if it bounces back.

I'll address the code as-is, even if the purpose of it is not necessarily a workable idea. It's a good exercise in working with Strings.


Variable naming

Let's start here...

    String email="";
    int at1=0,at2=0;
    int dot1=0;
    boolean at = false, dot=false, domain=false, tld=false;

Your variables say the name of the character(s) it is checking, but not what it is checking for. Most IDEs have a Refactor/Rename tool so let's use that for your code. This will make their intent more clear.

Also, in the case of your script, most if not all the variables get overwritten at some point or another. So either overwrite only if needed, or don't assign a value until you know what value it should hold. I will use the former.

    String email;
    int atCount = 0;
    int atPosition = 0;
    int dotPosition = 0;
    boolean hasAt = false, hasDot = false, hasDomain = false, hasTLD = false;
    String errors = "Invalid email address: ";
    int errorCount = 0;

I added a few more variables, as you can see; we'll use those for aggregating errors.


Checking for @

You're looping twice over the same email address looking for the same character. Why not just save yourself one loop and do all your assignments in one pass? Also, please use curly braces even for one-liners, it will make your code easier to follow. It's also good to use white space around operators.

        // Checking for '@' char
        for (int i = 0; i < email.length(); i++) {
            if (email.charAt(i) == '@') {
                atCount++;
                if (atCount == 1) {
                    atPosition = i;
                }
            }
        }
        if (atCount != 1) {
            //hasAt = false; // no need for this; already false
            errors += "Email must have exactly 1 '@' character. ";
            errorCount++;
        } else {
            hasAt = true;
        }
        // atCount =0; // no need for this; we're not using it anywhere else

Checking for other things

We can use some similar techniques to clean-up the rest of the code. Note the use of errors to add any applicable errors, and errorCount to keep track.

        // Checking the domain is not empty
    if ((atPosition + 1) < email.length()) {
        if (email.charAt(atPosition + 1) == '.') {
            // hasDomain = false; // no need; already false
            errors += "Email does not have a domain or domain is empty. ";
            errorCount++;
        } else {
            hasDomain = true;
        }
    }
    //Checking if there is at least one dot after the '@' char
    for (int i = atPosition; i < email.length(); i++) {
        if (email.charAt(i) == '.') {
            hasDot = true;
        }
    }
    if (hasDot != true) {
        errors += "Must have at least one dot after '@' character. ";
        errorCount++;
    }
    // no else; hasDot would already be false

    //Getting last dot's position
    for (int i = 0; i < email.length(); i++) {
        if (email.charAt(i) == '.') {
            dotPosition = i;
        }
    }
    //Checking there's some Top-Level Domain
    if (email.length() == dotPosition + 1) {
        // hasTLD = false; // no need; already false
        errors += "No top-level domain found. ";
        errorCount++;
    } else {
        hasTLD = true;
    }

do {...} while (...)

This construct is not working out too good for this purpose. The primary issue with it is that if any error is found during it, it just starts over from the start. It never does anything useful if there is an error, like print the error or exit the program.

Perhaps instead we can just move the logic into a method of its own, and call that method in main() against the input email address?

public class MailTest {

    public static void main (String [] args){
        String email = JOptionPane.showInputDialog("Insert your email");
        isValidEmail(email);
    }

    static boolean isValidEmail(String inputEmail) {
        /* All your vars and logic in here */
    }

}

And we can use a similar logic at the end in a simple if() ... else statement, and return whether or not the email is valid (according to your criteria).

    if (hasAt && hasDomain && hasDot && hasTLD) {
        System.out.println("Email entered: " + email);
        errors = "No errors found.";
        System.out.println(errors);
        return true;
    } else {
        System.out.println("Email entered: " + email);
        System.out.println("Errors found: " + errorCount);
        System.out.println(errors);
        return false;
    }

So you get a useful output, and a useful return value...

 Email entered: hello.world.com
 Errors found: 1
 Invalid email address: Email must have exactly 1 '@' character.
 Email entered: hello.world@foobar.com
 No errors found.

Everything put together:

import javax.swing.JOptionPane;

public class MailTest {

    public static void main (String [] args){
        String email = JOptionPane.showInputDialog("Insert your email");
        isValidEmail(email);
    }

    static boolean isValidEmail(String inputEmail) {

        String email = inputEmail;
        int atCount = 0;
        int atPosition = 0;
        int dotPosition = 0;
        boolean hasAt = false, hasDot = false, hasDomain = false, hasTLD = false;
        String errors = "Invalid email address: ";
        int errorCount = 0;

        // Checking for '@' char
        for (int i = 0; i < email.length(); i++) {
            if (email.charAt(i) == '@') {
                atCount++;
                if (atCount == 1) {
                    atPosition = i;
                }
            }
        }
        if (atCount != 1) {
            errors += "Email must have exactly 1 '@' character. ";
            errorCount++;
        } else {
            hasAt = true;
        }
        // Checking the domain is not empty
        if ((atPosition + 1) < email.length()) {
            if (email.charAt(atPosition + 1) == '.') {
                errors += "Email does not have a domain or domain is empty. ";
                errorCount++;
            } else {
                hasDomain = true;
            }
        }
        //Checking if there is at least one dot after the '@' char
        for (int i = atPosition; i < email.length(); i++) {
            if (email.charAt(i) == '.') {
                hasDot = true;
            }
        }
        if (hasDot != true) {
            errors += "Must have at least one dot after '@' character. ";
            errorCount++;
        }
        //Getting last dot's position
        for (int i = 0; i < email.length(); i++) {
            if (email.charAt(i) == '.') {
                dotPosition = i;
            }
        }
        //Checking there's some Top-Level Domain
        if (email.length() == dotPosition + 1) {
            errors += "No top-level domain found. ";
            errorCount++;
        } else {
            hasTLD = true;
        }

        // validity returning routine
        if (hasAt && hasDomain && hasDot && hasTLD) {
            System.out.println("Email entered: " + email);
            errors = "No errors found.";
            System.out.println(errors);
            return true;
        } else {
            System.out.println("Email entered: " + email);
            System.out.println("Errors found: " + errorCount);
            System.out.println(errors);
            return false;
        }
    }
}
\$\endgroup\$
14
\$\begingroup\$

Email validation aside...

Switching between GUI input and console output

You take your input via a JOptionPane, but the output of your validation is displayed via System.out. It will be much better UX-wise to show the result as another dialog box too.

Braces

Please use braces, it's not that hard. One extra line here or there and your conditions/loops will look even messier to clean up.

Boolean assignment and evaluation

if(dot1==0)
    dot=false;
else dot=true;

This can be rewritten as dot = dot1 != 0.

while(at==false||domain==false||dot==false||tld==false)

This can be rewritten as while (!(at && domain && dot && tld)).

And speaking about variable names...

Variable names

It's not easy to decipher what do at1, at2, dot1 etc. mean. It's recommended to use more descriptive names here, e.g. hasTLD.

\$\endgroup\$
  • 1
    \$\begingroup\$ Both answers given are good, but yours deals more with the code aspect, which will be more useful in the future for the OP. +1 \$\endgroup\$ – Joseph Farah Sep 3 '15 at 1:35
8
\$\begingroup\$

As mentioned by @JeroenVannevel, there really isn't a good way to do this. The best you can do is something like the below. Essentially, I have one function, isEmailAddress, which returns true if an @ character is found in the email address.

import javax.swing.JOptionPane;

public class MailTest {
    public boolean isEmailAddress(String emailAddress) {
        return emailAddress.contains("@");
    }

    public static void main(String[] args) {
        String emailAddress = JOptionPane.showInputDialog("Enter an email address.")

        if(isEmailAddress(emailAddress)) {
            System.out.println("What a correct email!");
        }
    }
}

As, again mentioned by @JeroenVannevel, there are just too many issues to consider when trying to validate email addresses, here are just a few of those issues:

  • An email address doesn't necessarily, end with a domain name, i.e, foo.bar.
  • An email address can contain an IP address as a domain name, i.e, 3.3.4.2, or others.
  • An email address can also have different formats as well.

In short, the only "good" way to check if an email address is valid is to ensure that it contains an @ character.

\$\endgroup\$
  • \$\begingroup\$ Can an email address contain more than one @? \$\endgroup\$ – jacwah Sep 3 '15 at 10:14
  • 1
    \$\begingroup\$ @jacwah, yes in a quoted local part "B@Man"@example.com \$\endgroup\$ – Jasen Sep 3 '15 at 10:34
1
\$\begingroup\$

Just wanted to make a STRONG comment about creating you own definition of what is or isn't a valid email address. I have seen way to many validation scripts that block real and valid email addresses because of lack of understanding.

Be sure to read and UNDERSTAND this wiki if you don't want to read the RFC's about valid email addressees.

\$\endgroup\$
0
\$\begingroup\$

I took some ideas from the Answers and Comments on people here, and made this code. I'm not sure about doing this by posting an answer, but I didn't want to edit the question.

import javax.swing.JOptionPane;

public class MailTest2 {
    public static void main (String [] args){
        String email="";
        int errorsNum=0;
        boolean exit=true;

        do {
            int atNum=0,atLoc=0,dotNum=0,dotLast=0;
            email=JOptionPane.showInputDialog("Insert your email");

                //Checking that we only have one @ and getting @'s position
            for (int i=0; i<email.length();i++){
                if(email.charAt(i)=='@'){
                    atNum++;
                    if(atNum==1){
                        atLoc=i;
                    }
                }
            }

            if (atNum<1){
                JOptionPane.showMessageDialog(null, "The address must have at least 1 at. \nPlease, try again");
                errorsNum++;
                continue;
            }else if (atNum>1){
                JOptionPane.showMessageDialog(null, "The address must have only 1 at. \nPlease, try again");
                errorsNum++;
                continue;
            }

                //Checking the domain is not empty
            if (atLoc+1==email.length()){
                JOptionPane.showMessageDialog(null, "The domain part name cannot be empty");
                errorsNum++;
                continue;               
            }

                //Checking the domain doesn't start with a dot
            if ((atLoc+1)<email.length()){
                if(email.charAt(atLoc+1)=='.'){
                    JOptionPane.showMessageDialog(null, "The domain's name cannot begin with a '.'");
                    errorsNum++;
                    continue;
                }
            }

                //Cheking if there is at least one dot and getting the last dot's position
            for (int i=atLoc; i<email.length();i++){
                if(email.charAt(i)=='.'){
                    dotNum++;
                    dotLast=i;
                }
            }

            if (dotNum<1){
                JOptionPane.showMessageDialog(null, "The address must have a Top-level Domain (.com/.uk/.org/.etc)\nPlease, try again");
                errorsNum++;
                continue;
            }

                //Checking there's some Top-Level Domain
            if (email.length()==dotLast+1){
                JOptionPane.showMessageDialog(null, "The address must have a Top-level Domain (.com/.uk/.org/.etc)\nPlease, try again");
                errorsNum++;
                continue;
            }

            exit=false;         
        }while(exit&&errorsNum<8);

        if(errorsNum>=7){
            JOptionPane.showMessageDialog(null, "You run out of tries.\nPlease, try again later!");         
        } else {
            JOptionPane.showMessageDialog(null, "You made it!");
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.