Pytanie Zezwalaj na proste instrukcje if, ale nie musisz mieć żadnych nawiasów klamrowych


używam checkstyle aby sprawdzić, czy mój kod Java nie spełnia wytycznych naszego projektu.

Mamy jednak jedną wskazówkę, że nie mogę wymyślić, jak sprawdzić za pomocą tego narzędzia. Chcemy pozwolić na proste, jeśli (rozumiemy, że nie ma w nim żadnej innej struktury warunkowej), aby nie mieć nawiasu klamrowego, jak w tym przykładzie:

// valid
if(condition) callFunction();

// invalid
if(condition) for(int i = 0; i < someValue; i++) callFunction(i);

// valid
if(condition) {
    for(int i = 0; i < someValue; i++) {
        callFunction(i);
    }
}

// invalid
if(condition) callFunction();
else callOtherFunction();

Tę konwencję można omówić, ale to ta, którą wybraliśmy. Pozwala na zredukowanie składni w przypadku bardzo trywialnych przypadków, ale zapewnia dobre wcięcie i blokowanie rozgraniczenia dla bardziej złożonych struktur.

Każda pomoc z tym będzie naprawdę doceniona.

Jestem również gotowy do wykonania kodu, aby wykonać tę kontrolę, jeśli nic nie jest dostępne, ale naprawdę nie wiem od czego zacząć. W ostatnim ressorcie docenione zostaną również wskazówki na ten temat.


12
2018-05-26 16:26


pochodzenie


Moja poważna sugestia to zmienić twoje wytyczne. Poświęcasz czas na szukanie rozwiązania, które wytyczy Twoje wskazówki dotyczące stylu gorszy. Inline, jeśli takie instrukcje (które nie używają bloków) prowadzą do błędów. Naprawdę, naprawdę paskudne błędy. - Mark Peters
@ Mark Nie wiem o używaniu wbudowanych ifs do prostych wywołań funkcji, ale myślę, że rozsądnie jest użyć ich do sprawdzenia pewnych warunków i wyrzucić wyjątek, jeśli warunki nie są spełnione. - Michael McGowan
To pytanie nie dotyczy tego, czy wytyczne są rozsądne. To pytanie dotyczy modyfikowania schematu w celu zapewnienia zgodności z tymi wytycznymi. Myślę, że powinniśmy unikać omawiania samych wytycznych w tym pytaniu. - Erick Robertson
@Erick: Można powiedzieć, że na dużą część pytań na tej stronie. Moim końcowym celem jest pomaganie ludziom. Jeśli oznacza to konieczność zadawania pytań, a tym samym zanegowanie potrzeby udzielenia odpowiedzi technicznej, niech tak będzie. Wiele rzeczy jest trudnych do zrobienia, ponieważ oni powinien być trudnym do zrobienia. Zauważ, że nie powiedziałem, że to był odpowiedź. - Mark Peters
@Bengt: Nie sądzę, że to tylko kwestia opinii, gdy widziałem złe błędy z powodu tego wyboru stylu. Przy okazji, wytyczne, które cytujesz, pozwalają na to tylko wtedy, gdy konsekwencja nastąpi bez podziału na wiersze. Osobiście uważam, że to jest sposób na lekcję szkodliwych skutków dopuszczenia braku instrukcji, jeśli nie. PO nie wspomniał o tym samym ograniczeniu. - Mark Peters


Odpowiedzi:


Na koniec zaimplementowałem niestandardowy czek na checkstyle. Oto kod źródłowy, jeśli ktoś go interesuje:

import com.puppycrawl.tools.checkstyle.api.Check;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

public class IfBracesCheck extends Check {

    @Override
    public int[] getDefaultTokens() {
        return new int[] {
            TokenTypes.LITERAL_ELSE,
            TokenTypes.LITERAL_IF,
        };
    }

    @Override
    public void visitToken(DetailAST aAST) {
        final DetailAST slistAST = aAST.findFirstToken(TokenTypes.SLIST);

        if(aAST.getType() == TokenTypes.LITERAL_ELSE) {
            // If we have an else, it must have braces, except it is an "else if" (then the if must have braces).
            DetailAST ifToken = aAST.findFirstToken(TokenTypes.LITERAL_IF);

            if(ifToken == null) {
                // This is an simple else, it must have brace.
                if(slistAST == null) {
                    log(aAST.getLineNo(), "ifBracesElse", aAST.getText());
                }
            } else {
                // This is an "else if", the if must have braces.
                if(ifToken.findFirstToken(TokenTypes.SLIST) == null) {
                    log(aAST.getLineNo(), "ifBracesConditional", ifToken.getText(), aAST.getText() + " " + ifToken.getText());
                }
            }
        } else if(aAST.getType() == TokenTypes.LITERAL_IF) {
            // If the if uses braces, nothing as to be checked.
            if (slistAST != null) {
                return;
            }

            // We have an if, we need to check if it has no conditionnal structure as direct child.
            final int[] conditionals = {
                TokenTypes.LITERAL_DO,
                TokenTypes.LITERAL_ELSE,
                TokenTypes.LITERAL_FOR,
                TokenTypes.LITERAL_IF,
                TokenTypes.LITERAL_WHILE,
                TokenTypes.LITERAL_SWITCH,
            };

            for(int conditional : conditionals) {
                DetailAST conditionalAST = aAST.findFirstToken(conditional);

                if (conditionalAST != null) {
                    log(aAST.getLineNo(), "ifBracesConditional", aAST.getText(), conditionalAST.getText());

                    // Let's trigger this only once.
                    return;
                }
            }
        }
    }
}

5
2018-05-27 17:04





Po prostu chcę dodać, że teraz checkstyle obsługuje właściwość "allowSingleLineIf", która obejmuje niektóre przypadki.

    <module name="NeedBraces">
        <property name="allowSingleLineIf" value="true"/>
    </module>

1
2018-01-21 15:52





Chociaż zgadzam się z komentarzami, że jest to zły pomysł, może nie być możliwości zmiany wytycznych. Więc możesz spróbować tego:

  1. W module checkstyle Bloki -> Potrzebujesz szelek, wyłącz gdyby słowo kluczowe
  2. Utwórz nową instancję modułu Regexp -> RegexpSingleLineJava i spróbuj znaleźć regexp, który pasuje do twoich nieważnych przypadków, ale nie do poprawnych

(Nazwy modułów pochodzą z Eclipse Checkstyle Plugin 5.3.0)


0
2018-05-27 07:13



Myślałem o czymś takim, ale regex nie jest wystarczająco mocny, by poradzić sobie z tą zasadą, nawet jeśli jest to prawdopodobnie część rozwiązania. - deadalnix
Regexp jest dość potężny;) Z twoich przykładów drugi przykład jest już nieważny, o ile instrukcja for jest wymagana, aby mieć nawiasy klamrowe. Czwarty przykład jest nadal ważny, ale mam regexp, który działa - po prostu nie w stylu kontrolnym. Może możesz to zrozumieć: (? S) if. *;. * Else - Stephan
Nie, to nie daje rady: upewnij się, że żadna struktura warunkowa nie istnieje, jeśli jest prawie niemożliwe przy użyciu wyrażenia regularnego, a różnorodność przypadków wymaga pewnej logiki. Twoja propozycja będzie pasować do bardzo wielu różnych przypadków, w tym do legalnego użycia znaku if. W każdym razie, sądzę, że z pewnym regexem i odrobiną logiki, można tu coś zarządzać. - deadalnix


CheckStyle 6.14 NeedBracesCheck wsparcie roola allowSingleLineStatement opcja

allowSingleLineStatement, która pozwala na jednoelementowe instrukcje bez nawiasów klamrowych, np .:

if (obj.isValid ()) zwraca true;

while (obj.isValid ()) zwraca true;

zrób to.notify (); while (o! = null);

dla (int i = 0;;) this.notify ();

dokumentacja


0
2018-01-12 14:28