E-Mail Validator (ohne Regex!)

[NicoRyl]

Hallo zusammen,

was ich hier versuche, ist ein E-Mail Validator der nach ein paar Kriterien die eingegebene Email überprüft.
Leider funktioniert das nicht wirklich. Es wird immer "Email ist nicht gestattet" ausgegeben.
Ich bin noch ein Anfänger, also nehmt bitte Rücksicht. Ich bin mir sicher, dass man das Ganze auch besser machen kann oder mein Code völliger Schwachsinn ist.
Trotzdem bitte ich um Hilfe.
Vielen Dank schonmal im Voraus!

LG, Nico.

package com.company;
import java.util.Scanner;

public class EmailValidator {

     public static void main (String[] args) {
         EmailValidation validation = new EmailValidation();
         validation.getInfo();
     }
}

class EmailValidation{
    private static String email;
    public static String[] topLevel;

    private static boolean test1 = true;
    private static boolean test2 = true;
    private static boolean test3 = true;
    private static boolean test4 = true;


    public static void getInfo() {
        Scanner x = new Scanner (System.in);

        System.out.println("Gebe die Email-Adresse ein: ");
        email = x.nextLine();
        System.out.println(CheckEmail(email));
    }

    public static String CheckEmail(String email1) {
        topLevel = new String[]{".com", ".de", ".net", ".at", ".schwarz", ".mail", ".eu", ".us"};

            if (email1.charAt(0) != '@') {
                test1 = true;
            }
           
            else if (email1.contains("_")) {
                test2 = false;
            }
            else if (email1.contains("@")) {
                test3 = true;
            }
           
            else if (email1.endsWith(String.valueOf(topLevel))); {
                test4 = true;
            }
           
          if (test1 && test2 && test3 && test4) {
              return "Email ist gestattet.";
          } else {
              return "Email ist nicht gestattet.";
          }
    }
}

 

[LimDul]

Du verwndest else if. Das heißt, wenn test1 auf true gesetzt wird, kann er in die anderen Blöcke gar nicht mehr reingehen, weil er ja schon im ersten Block ist.

Das heißt du musst für deinen Code aus dem else if jeweils ein if machen. Du willst ja jede Bedingung prüfen.

 

[kneitzel]

Deine Prüfung ist so nicht korrekt und auch nicht lesbar und ich habe Probleme, das alles sauber für Dich zu sortieren... Aber versuchen wir es mal:

a) Du initialisierst alles auf true - eine Prüfung + auf true setzen ist also unsinnig. Denn da änderst Du nichts.
b) Wenn eine Prüfung wahr ist, werden alle anderen Prüfungen nicht mehr gemacht. Das ist soweit ok, aber nur, wenn dann fest steht: es ist ungültig, d.h. Du hast eine Entscheidung.

Dann ist der Code schwer zu lesen. Ich bevorzuge da etwas in der Art (Pseudocode):

bool checkSomething(whatever) {
    if (something) return false;
    if (somethingElse) return false;
    if (anotherItem) return false;
    
    return true;
}

Dann ist also der Ansatz: Du prüfst diverse Ausschlüsse um dann ein Ergebnis zu geben, wenn es möglich ist.

 

[NicoRyl]

Deine Prüfung ist so nicht korrekt und auch nicht lesbar und ich habe Probleme, das alles sauber für Dich zu sortieren... Aber versuchen wir es mal:

a) Du initialisierst alles auf true - eine Prüfung + auf true setzen ist also unsinnig. Denn da änderst Du nichts.
b) Wenn eine Prüfung wahr ist, werden alle anderen Prüfungen nicht mehr gemacht. Das ist soweit ok, aber nur, wenn dann fest steht: es ist ungültig, d.h. Du hast eine Entscheidung.

Dann ist der Code schwer zu lesen. Ich bevorzuge da etwas in der Art (Pseudocode):

bool checkSomething(whatever) {
    if (something) return false;
    if (somethingElse) return false;
    if (anotherItem) return false;
   
    return true;
}

Dann ist also der Ansatz: Du prüfst diverse Ausschlüsse um dann ein Ergebnis zu geben, wenn es möglich ist.

Danke für die Antwort. Ich verstehe was du meinst, weiß aber leider nicht wie ich es umsetzten kann. 🙁

 

[kneitzel]

Ach ja: Wenn Du schon so viele Variablen hast: benenne sie richtig! Und dann keine if (whatever) boolean-Zuweisung. Das sind direkte Zuweisungen. Und keine Klassenvariablen nutzen - die Variablen werden doch nur in der Methode verwendet und können daher dann lokal sein.

Also etwas in der Art:

public static final String[] TOP_LEVEL_DOMAINS = new String[]{".com", ".de", ".net", ".at", ".schwarz", ".mail", ".eu", ".us"};

public static String CheckEmail(String email1) {
   
    boolean firstCharIsAtSign = email1.charAt(0) == '@';
    boolean containsUnderscore = email1.contains("_");
    boolean containsAtSign = email1.contains("@");
    // ...
}

Und dann ist das String.valueOf(topLevel) natürlich auch Quatsch. Mach mal einfach ein System.out.println(String.valueOf(topLevel)) um Dir das anzuzeigen....

Edit: bool -> boolean - wenn man Pseudocode kopiert kommt Quatsch raus 🙂

 

[NicoRyl]

Du verwndest else if. Das heißt, wenn test1 auf true gesetzt wird, kann er in die anderen Blöcke gar nicht mehr reingehen, weil er ja schon im ersten Block ist.

Das heißt du musst für deinen Code aus dem else if jeweils ein if machen. Du willst ja jede Bedingung prüfen.

Danke! Jetzt funktioniert es schonmal besser 🙂 Allerdings funktioniert die TopLevelDomain-Prüfung noch nicht. Weißt du woran das liegen könnte?

 

[NicoRyl]

Ach ja: Wenn Du schon so viele Variablen hast: benenne sie richtig! Und dann keine if (whatever) boolean-Zuweisung. Das sind direkte Zuweisungen. Und keine Klassenvariablen nutzen - die Variablen werden doch nur in der Methode verwendet und können daher dann lokal sein.

Also etwas in der Art:

public static final String[] TOP_LEVEL_DOMAINS = new String[]{".com", ".de", ".net", ".at", ".schwarz", ".mail", ".eu", ".us"};

public static String CheckEmail(String email1) {
  
    boolean firstCharIsAtSign = email1.charAt(0) == '@';
    boolean containsUnderscore = email1.contains("_");
    boolean containsAtSign = email1.contains("@");
    // ...
}

Und dann ist das String.valueOf(topLevel) natürlich auch Quatsch. Mach mal einfach ein System.out.println(String.valueOf(topLevel)) um Dir das anzuzeigen....

Edit: bool -> boolean - wenn man Pseudocode kopiert kommt Quatsch raus 🙂

Vielen Dank für deine Hilfe. - Okay, das "String.valueOf" macht keinen Sinn. Wie kann ich denn andernfalls diese TopLevelDomain Prüfen?

 

[kneitzel]

In einer Schleife alle Elemente des Array prüfen.

´Sowas würde ich in einer separaten Methode schreiben, um es übersichtlich zu halten, aber Du kannst es auch innerhalb schreiben. Dazu eine lokale Variable auf false setzen. Dann alle Elemente prüfen. Wenn eine passt, dann die Variable auf true setzen.
Das wäre das 08/15 Schema für sowas