Γνώμη για κομμάτι κώδικα.

[Fat4Life]

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

 

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

 

Σε ViewBag το εβαζε το αποτελεσμα, θα γινόταν αυτόματα int. αλλα και πάλι καλύτερα να έκανε typecasting όταν χρσηιμοποιούσε το return value.

 

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

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

 

To else-if ακόμα και με return βελτιώνει αρκετά το readability. To Or που είχε δεν είχε νόημα εφόσων έλεγχε το ίδιο πράγμα στην παραπάνω συνθήκη.

 

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

 

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

 

^^^^ this, πολύ σωστός. Προφανώς ο NickSym δεν έχει δεί άλλο κώδικα πέρα απο εργασίες σχολής σε C που δεν έχει exception handling  . 

[paparovic]

Άμα θες να του την πεις, κάνε ένα reduce. 

 

Σε JS (from the top of my head):

function CheckAccessLevel(id) {
let hm = [
  [DocumentAccessType.FundraisingLeadOnly, isFundraisingLeadOrAbove(id)],
  [DocumentAccessType.FundraisingLeadCp, isCpLeadOrAbove(id)],
  [DocumentAccessType.ProjectTeam, isInProjectTeam(id)],
  [DocumentAccessType.Everyone, true]
];

return hm.reduce((acc, pair)=>{
  if (pair[1] && !acc) {
    return pair[0];
  }
}, false);
}

Επεξ/σία 23 Νοεμβρίου 2016 από paparovic

[Fat4Life]

 

Άμα θες να του την πεις, κάνε ένα reduce. 

 

Σε JS (from the top of my head):

function CheckAccessLevel(id) {
let hm = [
  [DocumentAccessType.FundraisingLeadOnly, isFundraisingLeadOrAbove(id)],
  [DocumentAccessType.FundraisingLeadCp, isCpLeadOrAbove(id)],
  [DocumentAccessType.ProjectTeam, isInProjectTeam(id)],
  [DocumentAccessType.Everyone, true]
];

return hm.reduce((acc, pair)=>{
  if (pair[1] && !acc) {
    return pair[0];
  }
}, false);
}

 

 

C# version  :

public DocumentAccessType CheckAccessLevel(int id)
{
    var hm = new List<Tuple<DocumentAccessType, Func<int, bool>>>()
    {
        new Tuple<DocumentAccessType, Func<int, bool>>
            (DocumentAccessType.FundraisingLeadOnly, IsFundraisingLeadOrAbove),

        new Tuple<DocumentAccessType, Func<int, bool>>
            (DocumentAccessType.FundraisingLeadCp, IsCpLeadOrAbove),

        new Tuple<DocumentAccessType, Func<int, bool>>
            (DocumentAccessType.ProjectTeam, IsInProjectTeamOrAbove)
    };

    foreach(var pair in hm)
    {
        if(pair.Item2(id))
        {
            return pair.Item1;
        }
    }

    return DocumentAccessType.Everyone;
}

Το redability στο θεό χαχαχαχαχαχαχαχαχαχαχαχαχα

[παπι]

Σε ViewBag το εβαζε το αποτελεσμα, θα γινόταν αυτόματα int. αλλα και πάλι καλύτερα να έκανε typecasting όταν χρσηιμοποιούσε το return value.

 

 

 

To else-if ακόμα και με return βελτιώνει αρκετά το readability. To Or που είχε δεν είχε νόημα εφόσων έλεγχε το ίδιο πράγμα στην παραπάνω συνθήκη.

 

 

^^^^ this, πολύ σωστός. Προφανώς ο NickSym δεν έχει δεί άλλο κώδικα πέρα απο εργασίες σχολής σε C που δεν έχει exception handling  .

 

Γενικα συμφωνω με τον papakaliati, αλλα δεν βλεπω προβλημα με το σκετο if. Για το or δεν ξερω τι του ζητησαν. Αν του ζητησαν να γινουν map αυτες οι δυο funcs, τοτε καλα εκανε και το εβαλε και το προβλνμα πλεον ειναι αλλου.

Για το καστ παλι δεν ξερω τι παιζει. Η διορθωση που εκανες παντως, σου εβγαλε compile error. Το βλεπω απο τα refs count.

 

Τελος να πω πως προσωπικα δεν βλεπω κατι το τραγικο στο σνιπ.

[masteripper]

Κάτι τέτοιες περιπτώσεις είναι που το Table driven programming shines.... ...1 πίνακας ,ζευγάρια input-output........thats all

[tsofras]

Χωρίς να ξέρω τι κάνουν οι μέθοδοι, θα μπορούσες να υλοποιήσεις την λογική μέσα στο enum και να γλίτωνες απο όλες τις if

[anon667]

Ρε συ Fat, το συζήτησες με τον senior καθόλου; Εγώ θα ξενέρωνα αν τυχόν έμπαινα σε φόρουμ και έβλεπα πως συνεργάτης έχει γράψει τέτοια πράγματα.

 

Γενικά, καλό είναι πριν ξεκινήσετε το θάψιμο για κάτι που βλέπετε σε κώδικα άλλου, να προσπαθήσετε να καταλάβετε τον λόγο για τον οποίο μπορεί να το έχει γράψει έτσι. Οκ, αν είναι fail level developer να το καταλάβω, αλλά άμα γενικά είναι καλός κάποιος, πάντα υπάρχουν κάποιες παράμετροι που τον έχουν ωθήσει να μην γράψει τον σούπερ ντούπερ κώδικα -- π.χ. πίεση χρόνου.

[Fat4Life]

Γενικα συμφωνω με τον papakaliati, αλλα δεν βλεπω προβλημα με το σκετο if. Για το or δεν ξερω τι του ζητησαν. Αν του ζητησαν να γινουν map αυτες οι δυο funcs, τοτε καλα εκανε και το εβαλε και το προβλνμα πλεον ειναι αλλου.

Για το καστ παλι δεν ξερω τι παιζει. Η διορθωση που εκανες παντως, σου εβγαλε compile error. Το βλεπω απο τα refs count.

 

Τελος να πω πως προσωπικα δεν βλεπω κατι το τραγικο στο σνιπ.

 

Το τραγικό είναι πως σε 8 γραμμές κώδικα έκανε τόσα λάθη.

 

Να εξηγήσω λίγο παραπάνω την κατάσταση.

 

Έχουμε μια class την οποία έχω φτιάξει και είναι υπεύθυνη για τα user permissions στο application με όνομα UsersLogicProcessor. Σε αυτό το class υπάρχουν διάφοροι μέθοδοι όπως IsFundraisingLeadOrAbove, IsCpLeadOrAbove, IsInProjectTeamOrAbove. Αυτές οι μέθοδοι πέρνουν ένα Id και επιστρέφουν ένα boolean δεν έχουν side effects.

 

Αυτός λοιπόν πέταξε τη μέθοδο για το document access level (που είναι κάτι συγκεκριμένο και χρησιμοποιεί μόνο αυτός στο page που φτιάχνει), μέσα στο γενικο class για τα user permissions  . Η διόρθωση μου δεν βγάζει compile error απλώς έπρεπε να αλλαχτεί το return type απο int σε enum στο interface IUsersLogicProcessor, το οποίο δεν έκανα γιατί η μέθοδος έφυγε απο το class.

 

Proof:

Για το ViewData δε το σχολιάζω καν.......

 

Σχετικά με το Or: To IsFundraisingLeadOrAbove Στο πρώτο if αν είναι true θα κάνεις return ενώ αν είναι false δεν θα παίξει κανένα ρόλο στην αποτίμηση του δεύτερου condition είναι πασιφανές.

 

Typecasting του enum σε int is this real tea??

 

Τα if return δεν είναι τοσο τραγικά άλλα κάνουν τον κώδικα πιο δυσανάγνωστο, ενώ το if-else φαίνεται αμέσως τι θα γίνει.

Χωρίς να ξέρω τι κάνουν οι μέθοδοι, θα μπορούσες να υλοποιήσεις την λογική μέσα στο enum και να γλίτωνες απο όλες τις if

 

Δεν γίνεται αυτό στη C# δεν μπορείς να βάλεις function στα enum. Είναι σαν enum της C (integers)

Ρε συ Fat, το συζήτησες με τον senior καθόλου; Εγώ θα ξενέρωνα αν τυχόν έμπαινα σε φόρουμ και έβλεπα πως συνεργάτης έχει γράψει τέτοια πράγματα.

 

Γενικά, καλό είναι πριν ξεκινήσετε το θάψιμο για κάτι που βλέπετε σε κώδικα άλλου, να προσπαθήσετε να καταλάβετε τον λόγο για τον οποίο μπορεί να το έχει γράψει έτσι. Οκ, αν είναι fail level developer να το καταλάβω, αλλά άμα γενικά είναι καλός κάποιος, πάντα υπάρχουν κάποιες παράμετροι που τον έχουν ωθήσει να μην γράψει τον σούπερ ντούπερ κώδικα -- π.χ. πίεση χρόνου.

 

Δε πρόκειται να μπεί στο insomnia δε μιλλάει ελληνικά. Γενικά δε θα έλεγα πως γράφει ωραίο κώδικα πολύ τσαπατσουλιές απ ότι έχω δει απλά με τρέλανε που είδα τόσες βλακείες σε ένα τόσο απλό κομμάτι κώδικα. Πίεση χρόνου υπάρχει μεγάλη αλλα αυτό δεν είναι δικαιολογία για να γράφεται άσχημος κώδικας. Τι θα γίνει μετά απο μερικούς μήνες όταν σου ζητήσουν να αλλάξεις κάτι? Η όταν κάποιος άλλος developer κληρωνομήσει τον κώδικα σου?

[paparovic]

Η όταν κάποιος άλλος developer κληρωνομήσει τον κώδικα σου?

 

Αυτό είναι πρόβλημα του αποδέκτη της κληρονομιάς. 

 

[tsofras]

 

 

Δεν γίνεται αυτό στη C# δεν μπορείς να βάλεις function στα enum. Είναι σαν enum της C (integers)

 

οκ το block που έβαλες είναι ολόιδιο με java γι αυτό έκανα την αναφορά με το enum (στην java γίνεται)

[EnglishSpeaking]

Χωρίς να είμαι σχετικός θα γράψω τη χαζομάρα μου..  μπορεί να κάνει ένα μεγάλο if-else που να μοιάζει με switch.. 

if(condition1 && contition2 bla bla)) )
{
DoSomething();
}
else
{
DefaultAction();
}

[kstavrid]

Χωρίς να θέλω να σε κρίνω για τα κομμάτια κώδικα που μας έδειξες, ήθελα να ρωτήσω. 'Εχετε δοκιμάσει να γράψετε στυλ:

if (condition){
   return var;
}else if(condition2){
   return var2;
}else{
   return var3;
}

και όχι στυλ:

if (condition)
{
   return var;
}
else if(condition2)
{
   return var2;
}
else
{
   return var3;
}

Προσπαθώ να καταλάβω γιατί προτιμάται ο 2ος τρόπος από πολλούς, μήπως τον υιοθετήσω και εγώ  

 

Επίσης class names αρχίζουν με κεφαλαίο και μεθόδοι/μεταβλητές με μικρό, αυτά είναι τα standard απ'όσο ξέρω τουλάχιστον.. Πείτε μου την γνώμη σας για αναγνωσιμότητα γενικότερα.

[Kercyn]

Εγώ ακολουθώ (όσο μπορώ) αυτό. Το coding style εξαρτάται από εσένα ή από την εταιρεία στην οποία εργάζεσαι, αλλά ακόμα κι αν γράφεις μόνος σου καλό είναι να ακολουθείς κάποιος κανόνες σε όλο τον κώδικα. Άμα θες να γράφεις τις μεταβλητές με μικρό, γράφτες με μικρό σε όλο τον κώδικα και όχι τις μισές με μικρό και τις άλλες μισές με κεφαλαίο.

[Fat4Life]

Χωρίς να θέλω να σε κρίνω για τα κομμάτια κώδικα που μας έδειξες, ήθελα να ρωτήσω. 'Εχετε δοκιμάσει να γράψετε στυλ:

if (condition){
   return var;
}else if(condition2){
   return var2;
}else{
   return var3;
}

και όχι στυλ:

if (condition)
{
   return var;
}
else if(condition2)
{
   return var2;
}
else
{
   return var3;
}

Προσπαθώ να καταλάβω γιατί προτιμάται ο 2ος τρόπος από πολλούς, μήπως τον υιοθετήσω και εγώ

 

Επίσης class names αρχίζουν με κεφαλαίο και μεθόδοι/μεταβλητές με μικρό, αυτά είναι τα standard απ'όσο ξέρω τουλάχιστον.. Πείτε μου την γνώμη σας για αναγνωσιμότητα γενικότερα.

 

Αυτά είναι τα naming και styling convention, που ακολουθεί κανείς ανάλογα με τη γλώσσα την οποία χρησιμοποιεί ώστε να είναι ο κώδικας του οπτικά ίδιος με των άλλων.

 

Το πρώτο που προτείνεις π.χ. χρησιμοποιείται απ' αυτούς που γράφουν javascript, ενώ το δεύτερο το δικό μου στη C#.

 

Και εγώ όταν γράφω javascript ξεκινάω με μικρό γράμμα μεθόδους και μεταβλητές.

[kstavrid]

Εγώ ακολουθώ (όσο μπορώ) αυτό. Το coding style εξαρτάται από εσένα ή από την εταιρεία στην οποία εργάζεσαι, αλλά ακόμα κι αν γράφεις μόνος σου καλό είναι να ακολουθείς κάποιος κανόνες σε όλο τον κώδικα. Άμα θες να γράφεις τις μεταβλητές με μικρό, γράφτες με μικρό σε όλο τον κώδικα και όχι τις μισές με μικρό και τις άλλες μισές με κεφαλαίο.

 

Πολύ ωραίο το site, θα το κοιτάξω αρκετά σίγουρα. Για τα υπόλοιπα προφανώς συμφωνώ, δεν έχω δουλέψει ακόμα για corp οπότε ελπίζω να μην πετύχω κάποια όπου τα standards τους είναι κακασχημα...