Sind solche boolean Anweisen empfehlenswert?

[Maik.Neumann]

Hallo zusammen !

Ich habe in meinem Projekt den folgenden Sourcecode entdeckt und wollte mal nachfragenm, ob es tatsächlich empfehlenswert ist, diesen so auszu formulieren oder sollte man ihn lieber anders schreiben?:

boolean formatKorrekt =  werte != null && werte.length == 1 && Integer.parseInt(werte[0]) != -1;

Danke und Gruß

 

[knilch]

Hi,
Ich würde dies in eine Methode packen und diese in dem entsprechenden Ort dann aufrufen.

public boolean isFormatCorrect(String [] werte) {
	if(werte == null) {
		return false;
	}
	if(werte.length != 1) {
		return false;
	}
	if(Integer.parseInt(werte[0]) == -1) {
		return false;
	}
	return true;
}

So kannst du übersichtlicher definieren, wann welche Bedingungen true/false sind.

 

[stg]

Alles geschmackssache. Wenn man den Code an keiner weiteren stelle braucht: Wieso nicht? Allerdings würde ich eventuelle noch (redundante) Klammern setzen, um es besser lesbar zu gestalten. Für mich selbst würde ich aber ebenfalls zu der von knilch vorgeschlagenen Variante greifen und eine eigene Methode schreiben. Da kann man auch schöner kommentieren, was wann wie wo warum welchen Wert zurückgibt, für den Fall, das man selbst oder gar ein anderer später noch mal an den Code ran muss...

 

[Ruzmanz]

Es kommt ganz auf die Situation an. Für ein kleine Programm ohne Wartung / Wiederverwendung kann man das so schreiben. Man kann das auch in eine Methode auslagern und so wird

if(isFormatierungKorrekt(werte)) {...}

lesbarer. Auch wenn ich das selbst nicht ganz verfolge bietet sich mitlerweile die @NotNull-Annotation an. So wird gekennzeichnet, dass auf gar keinen Fall ein null-Wert erwartet wird:

public boolean isFormatCorrect(@NotNull String [] werte) {
	if(werte.length != 1) {
		return false;
	}
	if(Integer.parseInt(werte[0]) == -1) {
		 return false;
	}
	return true;
}

In diesem Fall könnte "parseInt" eine Exception werfen. Ist es tatsächlich notwendig erst gefährlich zu parsen und dann den Inhalt zu überprüfen?

public boolean isFormatCorrect(@NotNull String [] werte) {
	if(werte.length != 1) {
		return false;
	}
	if("-1".equals(werte[0])) {
		 return false;
	}
	return true;
}

Magic-Values sollte man eigentlich vermeiden. Somit gilt:

public static final String INCORRECT_FORMAT = "-1";

public boolean isFormatCorrect(@NotNull String [] werte) {
	if(werte.length != 1) {
		return false;
	}
	if(INCORRECT_FORMAT.equals(werte[0])) {
		 return false;
	}
	return true;
}

Wenn ich der Methode eigentlich keine Beachtung mehr schenken möchte, dann schreibe es auch mal gerne in eine Zeile, aber das hängt auch von dem restlichen Codestyle ab:

public static final String INCORRECT_FORMAT = "-1";

public boolean isFormatCorrect(@NotNull String [] werte) {
	return werte.length == 1 
		&& !INCORRECT_FORMAT.equals(werte[0]);
}

... Jetzt kommt auch der Umfang ins Spiel. In deiner Klasse befinden sich nun eine weitere Methode und eine statische Variable. Gehöhren die dort tatsächlich hin oder bietet sich vielleicht eine individuelle Klasse "Werte" eher an?

Value value = new Value(werte);
if(value.isValid()) {
	// Code.
}

public class Value {
	public static final int COLUMN_FORMAT_INDICATOR = 0;
	public static final String INVALID_FORMAT = "-1";

	private String[] values;

	public Value(@NotNull String[] values) {
		this.values = values;
        }

	public boolean isValid() {
		return !INVALID_FORMAT.equals(getFormatIndicator());
	}

	public String getFormatIndicator() {
		return getColumn(COLUMN_FORMAT_INDICATOR);
	}

	private String getColumn(int index) {
		if(index < 0 || values.length < index) {
			return null; // Exception / Default-Wert nach Anwendungsfall
		}
		return values[index];
	}
}

... Je nach Anforderung etc. kann man da beliebig rumspielen. (Habe das einfach vor mich hingetippt. Hoffe der Code stimmt)