Προς το περιεχόμενο

Προτεινόμενες αναρτήσεις

Δημοσ.

Senior dev στην ομάδα μου είχε πετάξει μέσα σε ένα class μου το παρακάτω function.

 

post-402124-0-81081000-1479906989_thumb.png

 

Τι του κάνεις?

 

 

Το άλλαξα σε αυτο:

 

post-402124-0-20700400-1479907444_thumb.png

  • Απαντ. 31
  • Δημ.
  • Τελ. απάντηση

Συχνή συμμετοχή στο θέμα

Δημοφιλείς Ημέρες

Δημοσ.

Το switch θα ήταν πιο καθαρό ναι αλλά δε γίνεται με switch γιατι καλείς διαφορετικά functions σε κάθε condition. Το θέμα είναι γιατί έκανε typecasting to enum σε Int????????? και γιατι δε χρησιμοποιεί if-else. Επισης το || στο δευτερο if δεν θα εκτελεστεί ποτέ.

Δημοσ.

Το switch θα ήταν πιο καθαρό ναι αλλά δε γίνεται με switch γιατι καλείς διαφορετικά functions σε κάθε condition. Το θέμα είναι γιατί έκανε typecasting to enum σε Int????????? και γιατι δε χρησιμοποιεί if-else.

 

Έχεις δίκιο, δεν είδα ότι έκανε invoke διαφορετικές methods σε κάθε if.

 

Το typecasting νομίζω το κάνει για να πάρει το underlying value του enum.

Δημοσ.

Το switch θα ήταν πιο καθαρό ναι αλλά δε γίνεται με switch γιατι καλείς διαφορετικά functions σε κάθε condition. Το θέμα είναι γιατί έκανε typecasting to enum σε Int????????? και γιατι δε χρησιμοποιεί if-else.

 

Πολλα return και στην δικια σου περιπτωση. 

public DocumentAccessType CheckAccessLevel(int id)
{
    var documentAcceptType = DocumentAccessType.Everyone;
    if (IsFundraisingLeadOrAbove(id))
        documentAcceptType = DocumentAccessType.FundraisingLeadOnly;
    ....
    else if ()
    ....
    else

    return documentAcceptType;
}

Ωστε αμα θελεις στο μελλον να το κανεις debug να μπορεις απλα με ενα breakpoint να εχεις το αποτελεσμα, και γενικως ειναι πολυ πιο καθαρη λυση.

  • Like 4
Δημοσ.

Το switch θα ήταν πιο καθαρό ναι αλλά δε γίνεται με switch γιατι καλείς διαφορετικά functions σε κάθε condition. Το θέμα είναι γιατί έκανε typecasting to enum σε Int????????? και γιατι δε χρησιμοποιεί if-else. Επισης το || στο δευτερο if δεν θα εκτελεστεί ποτέ.

else με return/break/continue, δεν υπαρχει λογος να το βαλεις. 

Το καστ και το or, δεν ξερω.

Δημοσ.

 

Δεν είμαι περήφανος γιαυτό
 return  IsA() ? Doc.Type1 : IsB() ? Doc.Type2 : IsC() ? Doc.Type3 : Doc.Type4;

 

μαυτο, το πιο πιθανο ειναι να σου ζητησει τον κωδικο του obfuscator

  • Like 1
Δημοσ.

μαυτο, το πιο πιθανο ειναι να σου ζητησει τον κωδικο του obfuscator

Αν το δεις για αρκετή ώρα το συνηθίζεις και δεν φαίνεται τόσο άσχημο

Δημοσ.

Εγώ πάλι νομίζω ότι χωρίς το elif με σκέτο if είναι πιό καθαρό

Αν η συνθήκη ελέγχου η συνάρτηση δηλαδή στα if, elif ήταν η ίδια ή παίζει ρόλο η σειρά εκτέλεσης των ελέγχων είναι οκ το elif 

Δημοσ.

Ο κωδικας ειναι αθλιος και στις δυο περιπτωσεις.

Τοσο το format οσο και τα ονοματα-ουρες του enum και των μεθοδων.

 

Επισης αυτες οι μεθοδοι τι ελεγχο κανουν;

Υπαρχει ελεγχος οριων της παραμετρου; Αν ναι, απλα διεγραψε αυτες τις μεθοδους και αλλαξε αυτην με ελεγχους σε ακρα και switch.

 

Δεν είμαι περήφανος γιαυτό

 

 return  IsA() ? Doc.Type1 : IsB() ? Doc.Type2 : IsC() ? Doc.Type3 : Doc.Type4;
Νομιζω πως θα ηταν πιο "καθαρο" ως

 

return  IsA() ? Doc.Type1 : 
        IsB() ? Doc.Type2 : 
        IsC() ? Doc.Type3 : 
                Doc.Type4;
Δημοσ.

Ο κωδικας ειναι αθλιος και στις δυο περιπτωσεις.

Τοσο το format οσο και τα ονοματα-ουρες του enum και των μεθοδων.

 

Επισης αυτες οι μεθοδοι τι ελεγχο κανουν;

Υπαρχει ελεγχος οριων της παραμετρου; Αν ναι, απλα διεγραψε αυτες τις μεθοδους και αλλαξε αυτην με ελεγχους σε ακρα και switch.

 

 

Η νομεκλατουρα της μεθολου ειναι υποδειγματικη βασικα.

 

Και επιπλεον σοβαρα προτεινεις να παρει ενα κομματι κωδικα που τηρει το Single responsibility principle και και να το καταστρεψει; Επιπλεον θεωρω οτι ειναι σχεδον σιγουρο οτι οι μεθοδοι αυτοι χρησιμοποιουνται και σε αλλους ελεγχους, μηπως θελεις να κανεις copy paste τον ελεγχο σε ακρα παντου; Και τριτον ( η μαλλον πρωτο σε σειρα πρωτεραιοτητας ) δεν ξερεις ποσο πολυπλοκος ειναι ο ελεγχος της καθε μεθοδου. Μπορουμε να υποθεσουμε οτι δεν περικλυει απλα 1=διευθυντης...

 

Bεβαια θα ειχε νοημα να ειναι η μεθοδος στο στυλ  

...
{
var userType= GetUserType(id);
var accessType = GetAccessTypeForUser(userType)
return accessType;

}

Δημιουργήστε ένα λογαριασμό ή συνδεθείτε για να σχολιάσετε

Πρέπει να είστε μέλος για να αφήσετε σχόλιο

Δημιουργία λογαριασμού

Εγγραφείτε με νέο λογαριασμό στην κοινότητα μας. Είναι πανεύκολο!

Δημιουργία νέου λογαριασμού

Σύνδεση

Έχετε ήδη λογαριασμό; Συνδεθείτε εδώ.

Συνδεθείτε τώρα

  • Δημιουργία νέου...