Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

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

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

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

deleted 165 characters in body
Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155
import javax.swing.JOptionPane;
// Source: http://codereview.stackexchange.com/questions/102634/e-mail-testing-code
// Author: http://codereview.stackexchange.com/users/82013/pablo-gomez
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;
 }
 }
}
import javax.swing.JOptionPane;
// Source: http://codereview.stackexchange.com/questions/102634/e-mail-testing-code
// Author: http://codereview.stackexchange.com/users/82013/pablo-gomez
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;
 }
 }
}
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;
 }
 }
}
added 460 characters in body
Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155
 String email;
 int atCount = 0;
 int atPosition = 0;
 int dotPosition = 0;
 boolean hasAt = false, hasDot = false, hasDomain = false, hasTLD = false;
 String errors;errors = "Invalid email address: ";
 int errorCount;errorCount = 0;

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

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.

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 trackstrack.

 // Checking the domain is not empty
 if ((atPosition + 1) < email.length()) {
 if (email.charAt(atPosition + 1) == '.') {
 // hasDomain = false; // no need; already false
 }errors else+= {"Email does not have a domain or domain is empty. ";
 errorCount++;
 hasDomain = true; } 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;
 }
import javax.swing.JOptionPane;
// Source: http://codereview.stackexchange.com/questions/102634/e-mail-testing-code
// Author: http://codereview.stackexchange.com/users/82013/pablo-gomez
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) == '.') {
 hasDomainerrors =+= false;"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;
 }
 }
}
 String email;
 int atCount = 0;
 int atPosition = 0;
 int dotPosition = 0;
 boolean hasAt = false, hasDot = false, hasDomain = false, hasTLD = false;
 String errors;
 int errorCount;

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

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.

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 tracks.

 // Checking the domain is not empty
 if ((atPosition + 1) < email.length()) {
 if (email.charAt(atPosition + 1) == '.') {
 hasDomain = false;
 } 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;
 }
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) == '.') {
 hasDomain = false;
 } 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;
 }
 }
}
 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.

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.

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;
 }
import javax.swing.JOptionPane;
// Source: http://codereview.stackexchange.com/questions/102634/e-mail-testing-code
// Author: http://codereview.stackexchange.com/users/82013/pablo-gomez
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;
 }
 }
}
Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155
Loading
lang-java

AltStyle によって変換されたページ (->オリジナル) /