Darf das while ((line = r.readLine ())! = Null) -Konstrukt verwendet werden?

73725
mkalkov

Ich möchte den folgenden Code umgestalten, da ich mich bei der Verwendung der Zuweisung im Vergleichsoperator nicht wohl fühle. Es sieht aus wie ein ziemlich idiomatisches C, aber denken Sie, dass dies eine gute Praxis in Java ist?

private void demoA(BufferedReader reader) throws IOException {
    String line = null;
    while ((line = reader.readLine()) != null) {
        doSomething(line);          
    }
}

Hier ist eine Alternative.

private void demoB(BufferedReader reader) throws IOException {
    String line = reader.readLine();
    while (line != null) {
        doSomething(line);
        line = reader.readLine();
    }
}

UPDATE: Ich bin über eine ähnliche Frage gestolpert , die vor einigen Jahren gestellt wurde. Es scheint, dass die Meinungen darüber, ob es in Ordnung ist oder nicht, geteilt sind. Sowohl Guava als auch Commons IO bieten jedoch alternative Lösungen für dieses Problem. Wenn ich eine dieser Bibliotheken im aktuellen Projekt hätte, würde ich sie wahrscheinlich stattdessen verwenden.

Antworten
50
Sie fragen vor allem nach einer Antwort auf Basis einer Meinungsäußerung. In meinem Fall hat sich der erste Ansatz als klarer herausgestellt, wenn ich zwischen beiden wählen muss, aber es ist nur mein Fall. morgano vor 5 Jahren 0
Ich gehe davon aus, dass das letzte `reader.readLine ();` `line = reader.readLine ();`? Sein sollte. tobias_k vor 5 Jahren 0
tobias_k: yes, thanks. I've fixed it. morgano: well, it is a question about coding guidelines, and most such questions are opinion-based. Still, I would like to know if majority prefers one of these options or if there is no strong preference. Thanks for you reply! mkalkov vor 5 Jahren 0
Ich halte "demoA" für idiomatisch für Java. Allerdings gibt es eine alte SO-Frage zum selben Thema: http://stackoverflow.com/questions/4677411/iterating-over-the-content-of-a-text-file-line-by-line-is-therethere -a-best-practice Besonders interessant ist die am besten bewertete (nicht akzeptierte) Antwort. Sebastian Redl vor 5 Jahren 3
Ich schreibe nicht `String line = null;` sondern `String line;`. johnchen902 vor 5 Jahren 2
Ich denke, eines der wichtigsten Dinge, die hier zu berücksichtigen sind, ist, dass Sie es mit einem "BufferedReader" zu tun haben, der diese Redewendung seit Beginn von Java verwendet. Wenn Sie es mit etwas anderem als einem gepufferten Leser getan haben, wissen die Leute möglicherweise nicht, was Sie tun. Da Sie jedoch gerade eine Zeile lesen, haben die meisten Java-Entwickler dies erkannt und werden dies sofort erkennen. corsiKa vor 5 Jahren 0
Was ist mit 'while (null! = (Line = ...)) `? Niklas B. vor 5 Jahren 0

6 Antworten auf die Frage

67
amon

Assignment inside a condition is ok in this case, as the assignment is surrounded by an extra pair of parentheses – the comparison is obviously != null, there is no chance that we wanted to type line == reader.readLine().

However, a for loop might actually be more elegant here:

for (String line = reader.readLine(); line != null; line = reader.readLine()) {
    doSomething(line);
}

Alternatively, we could do this which also restricts the scope of line as with the for-loop, and additionally eliminates unnecessary repetition:

while (true) {
    final String line = reader.readLine();
    if (line == null) break;

    doSomething(line);
}

I like this solution most because it doesn't mutate any variables.

+1 für `for`-Schleife. Ich hatte vorher nicht darüber nachgedacht, aber das ist klar und vermeidet "Linien" außerhalb des Loop-Bereichs. Emily L. vor 5 Jahren 8
Ich mag auch die for-Loop-Lösung. Es ist dicht, lesbar und lenkt nicht die Aufmerksamkeit von `doSomething (line) 'ab. mkalkov vor 5 Jahren 2
In der zweiten Schleife: Warum immer einen neuen abschließenden String erstellen und die Variable nicht vor dem while setzen? Fabio F. vor 5 Jahren 0
Es gibt ein spezielles Konstrukt in C ++, um den äußeren Gültigkeitsbereich von Müllvariablen freizuhalten: `while (int a = f ()) {...}`. Ich denke, dass es gut ist, solchen Code zu schreiben. polkovnikov.ph vor 5 Jahren 0
@FabioF. Ich erstelle nicht jedes Mal einen "neuen String", das passiert ohnehin wegen "readLine". Es wird jedoch als bewährte Methode angesehen, die Sichtbarkeit von Variablen auf den kleinstmöglichen Bereich zu beschränken. Hier bedeutet dies, dass die Zeile nicht außerhalb der Schleife deklariert werden sollte. Variablen und andere Aspekte der Unveränderlichkeit nicht neu zuzuweisen, ist eine gute Angewohnheit, die ich bei der funktionalen Programmierung herausgefunden habe: Dies macht Code verständlicher, da der Variablenname und sein Wert austauschbar verwendet werden können („referenzielle Transparenz“) Mutationszustand muss ich im Auge behalten. amon vor 5 Jahren 5
@amon what about `{String line; while(...){...}}` Fabio F. vor 5 Jahren 0
@FabioF. Dies würde zwar die Sichtbarkeit einschränken, fügt jedoch eine unnötige Einrückung hinzu. Und wir könnten genauso gut ein gleichwertiges "for (String line; ...;) {...}" verwenden, stattdessen alle Vorteile, keine der Nachteile. amon vor 5 Jahren 2
Ich habe eine positive Antwort erhalten ... gute Antwort, mit Ausnahme des 1-Liner-Vorschlags: `if (line == null) break;` was im Widerspruch zu den [Java-Code-Style-Richtlinien (7.4)] (http: // www. oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449) rolfl vor 5 Jahren 0
Die letzte Lösung gefällt mir am besten. Es ist oft eine gute Idee, zu vermeiden, dass Schleifenausstiegspunkte durch Code getrennt werden, der sinnvolle Nebeneffekte hat. Wenn sich der natürliche Ort für den Schleifenausstiegspunkt jedoch in der Mitte befindet, ist es besser, ein while / break-Konstrukt zu verwenden, als den Dupliziervorgang Code, der vor dem Endpunkt der Schleife stehen soll. supercat vor 5 Jahren 2
Der while-Vorschlag (true) funktioniert besonders gut, wenn der Code für das nächste zu bearbeitende Element weniger trivial ist als hier. In trivialen Fällen spielt es keine Rolle, welches Muster Sie verwenden, da es ohnehin trivial ist, aber dieses Muster funktioniert auch in komplexen Fällen. Dadurch wird vermieden, dass dieser Code am Ende der Schleife dupliziert wird. Dies ist fehleranfällig, wenn der Code in der Schleife länger wird und möglicherweise eine continue-Anweisung enthält. Das ist durchaus üblich, wenn Sie Ihren Code wie "Get line; Exit, wenn keine weiteren Zeilen; Check Line, ob Zeile ignoriert werden soll; Prozesszeile;" gnasher729 vor 5 Jahren 0
@ EmilyL., Ich stimme der `for`-Schleife nicht zu. Während der Code _functional_ ist, ist eine "for" -Schleife semantisch für eine bestimmte Anzahl von Iterationen (einschließlich Iteration über eine Collection / ein Array), während eine "while" -Schleife semantisch für eine unbekannte Anzahl von Iterationen ist (z. B. Lesen aus einer Datei). . Beide Schleifenvarianten können natürlich mit der anderen aufgebaut werden, aber das ist nicht der Punkt. Brian S vor 5 Jahren 0
@BrianS Obwohl dies im naiven Fall der Fall ist, hat Javas Einführung des "for-each" -Konstrukts in 1.5 zu einer gewissen Färbung der Bedeutung einer "for" -Schleife geführt. Um eine solche Schleife verwenden zu können, muss ein Objekt "Iterable" implementieren, aber nichts über diese Schnittstelle impliziert, dass die Anzahl der Objekte, die vom Iterator erzeugt werden, der von "Iterable.iterator ()" zurückgegeben wird, spezifisch sein muss. Ein vernünftiges Beispiel wäre die "Iterable" -Lösung aus Arians Antwort; Sie wissen nicht, wie viele Zeilen die "Iterable" erzeugen wird, aber jede Zeile ist ihr eigenes Element, an dem sie arbeiten kann, und so hat "Each" immer noch eine Bedeutung. JAB vor 5 Jahren 1
20
palacsint

You could increase the abstraction level of the code a little bit with an iterator-like pattern and the same time you could reuse an existing library (with the experience of the authors) for that: Apache Commons IO LineIterator. It would replace the null check to a little bit readable hasNext()/nextLine().

Using an iterator hides an unnecessary detail: the reader returns null when there is no more data. The hasNext() method is closer to the (English) language, the code is easier to read. You still can check the details inside LineIterator if you need that but usually readers/maintainers happier with a higher level overview of the method which is easier to understand. (This answer and question contain an expressive example.)

A sample method:

private void demoC(BufferedReader reader) throws IOException {
    final LineIterator it = new LineIterator(reader);
    try {
        while (it.hasNext()) {
            String line = it.nextLine();
            // do something with line
        }
    } finally {
        it.close();
    }
}

See also: Effective Java, 2nd edition, Item 47: Know and use the libraries (The author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.)

Wir verwenden Apache Commons IO nicht für dieses Projekt, aber danke für einen Hinweis! mkalkov vor 5 Jahren 0
@mkalkov: Es ist einfach eine Klasse mit ein paar Zeilen. Sie können also Ihre eigene Version erstellen oder die Implementierung in Ihr Projekt kopieren (überprüfen Sie jedoch vorher die Lizenz). palacsint vor 5 Jahren 0
Natürlich können Sie den Abstraktionsgrad erhöhen. Aber warum solltest du? Was sind die Vorteile? gnasher729 vor 5 Jahren 0
@ gnasher729: Gute Frage, danke! Ich habe die Antwort aktualisiert, bitte überprüfen. palacsint vor 5 Jahren 0
Um line = null zu testen, müssen Sie nun etwas über eine nicht standardmäßige LineIterator-Klasse lernen, die tief in einer Bibliothek vorhanden ist. Sie müssen wissen, dass der Iterator eine "hasNext" -Methode hat, die Sie kennen müssen Wenn Sie in der nächsten Zeile "nextLine" aufrufen, müssen Sie wissen, dass Sie auf dem Iterator "close" aufrufen müssen. Sie haben die Ausnahmebehandlung hinzugefügt. Es gibt 8 Zeilen Boilerplate-Code anstelle von 3. Der eigentliche Code ist jetzt drei tief verschachtelte Ebenen beginnt statt zwei, entschuldigen Sie mich, wenn ich überhaupt nicht beeindruckt bin. gnasher729 vor 5 Jahren 0
@ gnasher729: Zwei Dinge zu beachten: 1. 'close' schließt nur den Leser, der im Originalcode fehlt. 2. Code wird mehr gelesen als geschrieben. Es empfiehlt sich daher, das Lesen zu optimieren. palacsint vor 5 Jahren 0
Warum verwenden Sie diesen Iterator manuell anstatt in einer foreach-Schleife? Phoshi vor 5 Jahren 2
@Phoshi: leider kann foreach nicht "Iterator" verwenden, es braucht und "Iterable". palacsint vor 5 Jahren 3
@palacsint: Ah natürlich. Ich denke, meine Frage ist dann, warum LineIterator Iterable nicht implementiert, aber ich denke, das ist nicht etwas, das Sie beantworten können. Phoshi vor 5 Jahren 2
@Phoshi: Sie können jeden Leser (mit einem beliebigen Stream) an den LineIterator übergeben. Einige von ihnen unterstützen möglicherweise nicht das Zurücksetzen (und können nur einmal gelesen werden, wie ein Netzwerkanschluss). Bei mehrmaligem Lesen müssen mehrere Iteratoren mit `Iterable.iterator ()` erstellt werden. palacsint vor 5 Jahren 1
@palacsint: Ist das nicht genau das gleiche Szenario wie hier? Eine foreach-Schleife kompiliert sich effektiv in den Code, den Sie hier haben, und Sie können beide Codes immer noch nicht sicher durchlaufen. Phoshi vor 5 Jahren 0
@palacsint Keiner von `Iterator`,` Iterable` und `LineIterator` unterstützt` reset`. Ich bin mir also nicht sicher, was Sie wollen. JAB vor 5 Jahren 0
@JAB: Reset gilt für Streams. Sie könnten eine `LineIterable` erstellen, deren` iterator () `-Methode` reset` für den zugrunde liegenden Stream aufrufen könnte. (Es hätte Nachteile, da es nicht zu einfach ist, gleichzeitige Iteratoren zu unterstützen.) palacsint vor 5 Jahren 0
@Phoshi: Ja, aber foreach unterstützt das nicht. palacsint vor 5 Jahren 0
@palacsint: foreach ruft einfach .next auf, bis .hasNext auf false gesetzt ist. Das einzige Problem würde auftreten, wenn Sie versuchen, / zweimal zweimal aufzuzählen. Dies ist in diesem Fall genau dasselbe. Ich verstehe immer noch nicht, warum es keine Iteration sein konnte. Phoshi vor 5 Jahren 0
@palacsint Klar, Sie könnten `LineIterator` in` ResettingLineIterator` unterteilen und den Konstruktor den Reader zurücksetzen lassen. Das Argument gegen "Iterable" unter https://issues.apache.org/jira/browse/IO-181 ist für die Implementierung beider Schnittstellen für dasselbe Objekt und ist durchaus vernünftig, alle Argumente gegen eine `LineIterable`-Klasse sind jedoch von` getrennt LineIterator, der einen neuen Iterator für jeden Aufruf von "iterator ()" zurückgibt, gilt aufgrund der Art des zugrunde liegenden "Readers" gleichermaßen für "LineIterator". JAB vor 5 Jahren 0
(Not to mention that using multiple `LineIterator`s for concurrency would possibly be worse than a `LineIterable` class as each of those `LineIterator`s could close the `Reader`, which would invalidate any other `LineIterator`s on that `Reader` even if it were one supporting `mark`/`reset`/`skip` and the iterators were subclassed to keep track of their positions independently [though you could also override `LineIterator.close()` to not actually call `Reader.close()` until all iterators for that `Reader` have closed/been consumed - perhaps a concurrent `Map` for the counts?]). ,> JAB vor 5 Jahren 1
Of course, in Java 8 you'd do something like `BufferedReader rf; if (reader instanceof BufferedReader) rf = (BufferedReader) reader; else rf = new BufferedReader(reader); rf.lines().forEach(line -> do_something(line));` and all this arguing goes away (to be replaced by arguing over whether a functional programming style is truly appropriate for Java and whether it improves readability or not and so on... though the ability of streams to be internally concurrent is pretty nice). JAB vor 5 Jahren 0
@Phoshi: Oh, ich verstehe jetzt deinen Punkt. Es könnte "Iterable" sein, aber die Leute bei Apache haben das nicht gewählt. Ich denke, es wäre leicht zu missverstehen und könnte Kopfschmerzen verursachen, wenn man es in zwei foreach-Schleifen verwenden möchte. So könnte es sein, aber ich bin mir nicht sicher, ob ich das möchte oder nicht. palacsint vor 5 Jahren 0
13
Cephalopod

Instead of wrapping the reader in an iterator, you could also wrap it in an Iterable that then returns the iterator.

It would allow you to write the following

for (String line: linesOf(reader)) {
     // ...
}

which makes very clean code.

5
200_success

In my opinion, demoA() is fine just as it is.

Assignment as a side effect within a test is normally frowned upon, but this usage is an excellent example of why the language feature exists. It's compact, non-repetitive, idiomatic, and efficient. Use it, and don't feel guilty about it!

2
David

Please realize that

if (cond(var = expr))

can usually be rewritten as

var = expr;
if (cond(var)) ...

and

while (cond(var = expr))

can always be rewritten as

for (var = expr; cond(var); var = expr)

without even affecting the meaning of break; or continue; in the loop.

So there is quite rarely a definite need for cramming assignments into conditionals' conditions.

1
Joydeep Bhattacharya

Sie können wahrscheinlich auch so etwas tun:

Files.lines(path)

Es holt alle Zeilen aus der Datei als Stream, dann können Sie die Zeichenfolge nach Ihrer Logik sortieren, diese dann in einer Liste sammeln und in die Ausgabedatei schreiben.

Diese Antwort gibt Ihnen einen funktionalen Programmieransatz. Der größte Teil der Methode ist selbsterklärend.

Files.lines(Paths.get(path)).map(--Your business logic--);

In der steam API stehen verschiedene Funktionen zur Verfügung, die Ihre Verarbeitung erleichtern.