c#, ειναι readable αυτο;

παπι · 3 Νοεμβρίου 2017



   class TestController : SecureConnection

    {

        public Data.User User { get; set; }



        public TestController(IWebSocketConnection conn) : base(conn)

        {

            

        }

        protected override void OnSecureMessage(string msg)

        {

            var packet = FromJson<Packet>(msg);

            switch(packet.Type)

            {

                case "auth": OnAuth(FromJson<Packet<Auth>>(msg));

                    break;

            }

            

            base.OnSecureMessage(msg);

        }



        private void OnAuth(Packet<Auth> auth)

        {

            User = Data.User.Auth(auth.Payload.User, auth.Payload.Pass);

            var res = new Packet<AuthRespones>

            {

                Id = auth.Id,

                Type = "auth",

                Payload = new AuthRespones

                {

                    Status = (User == null) ? "fail" : "ok"

                }

            };

            Send(res);

        }



    }



    



    class Packet

    {

        public int Id { get; set; }

        public string Type { get; set; }

    }

    class Packet<T> : Packet

    {

        public T Payload { get; set; }

    }

    class Auth

    {

        public string User { get; set; }

        public string Pass { get; set; }

    }

    class AuthRespones

    {

        public string Status { get; set; }

        public string Token { get; set; }

    }

Papakaliati · 3 Νοεμβρίου 2017

Μια χαρα readable ειναι, αλλα δεν υπαρχει λογος υπαρξεις θαρρω του base class Packet, θα επρεπε ειτε να ειναι interface (ipacket), ειτε το packet<T> να interface. Γενικως θα μπορουσες να δουλευεις λιγο περισσοτερο με interfaces, θα επιτρεψουνε να ειναι testable στο μελλον.

Το packet type θα πρεπει να ητανε enum.

Επιπλεον στο switch παντα να βαζεις και default παντα, ενα

default:

new ArgumentOutOfRangeException();

κανει θαυματα στο να εντοπιζεις bugs.

albNik · 3 Νοεμβρίου 2017

Επιπλεον στο switch παντα να βαζεις και default παντα, ενα

default:

new ArgumentOutOfRangeException();

κανει θαυματα στο να εντοπιζεις bugs.

Αν το κάνει αυτό θα σκάει πάντα όταν Type != "auth"

Papakaliati · 3 Νοεμβρίου 2017

Αν το κάνει αυτό θα σκάει πάντα όταν Type != "auth"

Ε καλα δεν χρειαζεται να throw exception δε και καλα, μπορεις απλα να το κανεις log. Φανταζομαι οτι ο κωδικας του δεν σταματαει μονο στο auth, αλλα πιανεις και αλλες περιπτωσεις.

Alithinos · 3 Νοεμβρίου 2017

Να ρωτήσω κάτι ?

Γιατί έφτιαξες switch με μόνο 1 case ? Θα προσθέσεις στο μέλλον περισσότερα ?

Αν είναι μόνο 1 η περίπτωση ελέγχου που θα μας απασχολήσει, προσωπικά θα προτιμούσα κάτι τέτοιο:

if(packet.Type == "auth")
    OnAuth(FromJson<Packet<Auth>>(msg);

Για να λιγοστέψω τις γραμμές που καταλαμβάνει ο κώδικας.

Επίσης γενικά προτιμώ τους constructors, για να μη χρειάζεται να γράφω και το όνομα του κάθε arg κάθε φορά που δημιουργώ αντικείμενο, και να κάνω αρχικοποίηση σε 1 σειρά.

Αυτά είναι προσωπικές μου γνώμες / προτιμήσεις δεν είναι τίποτα κανόνες, και μπορώ να πω ότι ναι είναι readable ο κώδικας γιατί μπόρεσα και τον δίαβασα χωρίς κόπο, απλά άμα θες σκέψου λίγο αυτά που σου πα.

παπι · 3 Νοεμβρίου 2017

Αυτο που θελω ειναι οσο το καλυτερο encapsulation και abstraction ωστε να μην λεω σε κανα μηνα "wtf wtf wtf!!!!"

Η κλαση TestController ουσιαστικα ειναι ενας client. Θελω να φυγουν databse, connection κλπ. Θελω καθαρο rpc αναμεσα server-client

Τωρα εβαλα και αλλα πραματα, αλλα αρχισαν τα wtfs...

class TestController : SecureConnection
    {

        public TestController(IWebSocketConnection conn) : base(conn)
        {
          
        }
        protected override void OnSecureMessage(string msg)
        {
            var packet = FromJson<Packet>(msg);
            switch(packet.Type)
            {
                case "createcustomer":
                    CreateCustomer(FromJson<Packet<NewCustomerRequest>>(msg));
                    break;
                case "editcustomer":
                    EditCustomer(FromJson<Packet<EditCustomerForm>>(msg));
                    break;
                case "customerinfo":
                    GetCustomerInfo(FromJson<Packet<CustomerInfo>>(msg));
                    break;
            }
            
            base.OnSecureMessage(msg);
        }

        private void CreateCustomer(Packet<NewCustomerRequest> customer)
        {
            var cust = Data.Customer.FindByPhone(customer.Payload.Phone);
            if (cust == null)
            {
                cust = new Data.Customer
                {
                    Name = customer.Payload.Name,
                    UserId = ConnectedUser.Id
                };
                cust.Create();
                cust.AddNewPhone(customer.Payload.Phone);
            }

            Send(
                new Packet<NewCustomerRespones>
                {
                    Type = customer.Type,
                    Id = customer.Id,
                    Payload = new NewCustomerRespones
                    {
                        Id = cust.Id,
                        Status = "ok"
                    }
                }
                );
            
        }

        private void EditCustomer(Packet<EditCustomerForm> form)
        {
            var cust = Data.Customer.Find(form.Payload.Id);
            if(cust == null)
            {
                Send(new Packet<EditCustomerForm>
                {
                    Id = form.Id,
                    Type = form.Type,
                    Payload = new EditCustomerForm
                    {
                        Status = "fail"
                    }
                });
                return;
            }
            foreach (var edit in form.Payload.PhoneEdits)
            {
                
            }
        }
      
        private void GetCustomerInfo(Packet<CustomerInfo> info)
        {
            var cust = Data.Customer.Find(info.Payload.Id);
            if (cust == null)
            {
                throw new NotImplementedException();
            }
            info.Payload.Name = cust.Name;
            info.Payload.SelectedAddress = new Address
            {
                Id = cust.LastAddressId,
                FullRute = cust.GetLastUsedAddress().QueryAddress
            };
            info.Payload.SelectedPhone = new Phone
            {
                Id = cust.LastPhoneId,
                Number = cust.GetLastUsedPhone().Number
            };
            info.Payload.Adresses = cust.GetAddress()
                .Select(x => new Address { Id = x.Id, FullRute = x.QueryAddress })
                .ToList();
            info.Payload.Phones = cust.GetPhones()
                .Select(x => new Phone { Id = x.Id, Number = x.Number })
                .ToList();

            Send(info);
        }
    }
    
    public class NewCustomerRequest
    {
        public string Name { get; set; }
        public string Phone { get; set; }

    }
    public class NewCustomerRespones
    {
        public string Status { get; set; }
        public long Id { get; set; }
    }
    public class EditCustomerForm
    {
        public class Edit
        {
            public string Do { get; set; }
            public string Status { get; set; }
            public long Id { get; set; }
        }

        public long Id { get; set; }
        public string Status { get; set; }
        public List<Edit> PhoneEdits { get; set; }
        public List<Edit> AddressEdits { get; set; }
        public Edit NameEdit { get; set; }

    }

    public class Address
    {
        public long Id { get; set; }
        public string FullRute { get; set; }
    }
    public class Phone
    {
        public long Id { get; set; }
        public string Number { get; set; }
    }
    public class CustomerInfo
    {
        public long Id { get; set; }
        public string Name { get; set; }
        public Address SelectedAddress { get; set; }
        public Phone SelectedPhone { get; set; }
        public List<Address> Adresses { get; set; }
        public List<Phone> Phones { get; set; }
    }

Οι κλασεις κατω απο τον TestController θα μπουν σε dll εφοσον θα τις εχουν και ο σερβερ και ο client.

Το Auth το εβαλα εδω εφοσον ειναι μερος του πυρηνα.

    class SecureConnection : BaseConnection
    {

        public Data.User ConnectedUser { get; private set; }

        public SecureConnection(IWebSocketConnection conn) : base(conn)
        {
            ConnectedUser = null;
        }
        protected override void OnMessage(string msg)
        {
            //todo imp crypto

           
            if (FromJson<Packet>(msg).Type == "auth")
            {
                var auth = FromJson<Packet<Auth>>(msg);
                ConnectedUser = Data.User.Auth(auth.Payload.User, auth.Payload.Pass);
                Send(new Packet<AuthRespones>
                {
                    Id = -1,
                    Type = "auth",
                    Payload = new AuthRespones
                    {
                        Status = (ConnectedUser == null) ? "fail" : "ok"
                    }
                });
            }
            else if (ConnectedUser == null)
            {
                Send(new Packet<AuthRespones>
                {
                    Id = -1,
                    Type = "auth",
                    Payload = new AuthRespones
                    {
                        Status = "fail"
                    }
                });
            }
            else
            {
                OnSecureMessage(msg);
            }
            base.OnMessage(msg);
        }
        protected virtual void OnSecureMessage(string msg)
        {

        }
        protected override void Send(object obj)
        {
            //todo imp crypto
            base.Send(obj);
        }

      
    }
    class Packet
    {
        public int Id { get; set; }
        public string Type { get; set; }
    }

    class Packet<T>  : Packet 
    {
        public T Payload { get; set; }
    }
    class Auth 
    {
        public string User { get; set; }
        public string Pass { get; set; }
    }
    class AuthRespones
    {
        public string Status { get; set; }
        public string Token { get; set; }
    }

Papakaliati · 4 Νοεμβρίου 2017

Ενα factory pattern χρειαζεσαι, δηλαδη μια κλαση που θα παιρνει το packet, και θα επιστρεφει το packet<T> που πρεπει να στειλεις.

Με αυτον τον τροπο, ο TestController θα ειναι εξαιρετικα συμμαζεμενος, και το σημαντικοτερο οταν θελεις να προσθεσεις περισσοτερα packet cases, απλα επεκτεινεις το factory χωρις να αγγιζεις τoν Controller. Την factory class θα την περνας στον TestController με injection.

Αυτην την στιγμη, ολα τα classes του τυπου create customer, ειναι στην πραγματικοτητα CreateAndSendXXXPacket.

Θα ητανε προτιμοτερο απλα να επιστρεφουνε το επιθυμητο packet, και μετα να τα στελνεις παντα απο ενα σημειο.

Alithinos · 4 Νοεμβρίου 2017

Κοίτα εγώ ακολουθώ αρκετές συμβουλές που είχα διαβάσει σε ένα βιβλίο, το "clean code" του bob martin. Και άλλες από αλλού, πχ

Να μη γράφω μεθόδους μεγαλύτερες από 5 σειρές κώδικα.

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

Έτσι οι μέθοδοι χωρίζονται σε layers και έχεις μια πχ που καλεί 4, 4 σειρές κώδικα, και η κάθε μια απ αυτές έχει πχ άλλες 5 γραμμές κώδικα.

Έτσι αντί για 20 γραμμές σε 1 μέθοδο, έχεις 20 σε 4 μεθόδους.

Έτσι πάντα βρίσκεις αμέσως τι κάνει η κάθε μέθοδος, και το κομμάτι της που σε ενδιαφέρει, και ο κώδικας είναι πιο οργανωμένος και απομονωμένος απ τον υπόλοιπο ώστε και να τον αλλάζεις πιο γρήγορα.

Το σκεπτικό είναι ότι το αρχείο κώδικα θα το γράψεις σαν να γράφεις κάτι με σκοπό να διαβαστεί, οπως μια εφημερίδα γιαη παράδειγμα. Στο πάνω μέρος του αρχείου κώδικα σου δίνεται διαβάζοντας μια γενική ιδέα υψηλού επιπέδου του τι κανει, όπως η εφημερίδα έχει τους τίτλους ειδήσεων που σου δίνουν μια γενική ιδέα, και στην συνέχεια αν θες να μάθεις τις λεπτομέρειες σκρολαρεις προς τα κάτω όπως θα άλλαζες σελίδα, εντοπίζοντας ακριβώς τη μέθοδο που σε ενδιαφέρει να δεις σε χαμηλότερου επιπέδου εργασίες. Η οποία επιβάρυνση από επιπρόσθετες κλήσεις είναι αμελητέα στις περισσότερες των περιπτώσεων αν πάντα μπορείς εύκολα να κατευθυνθεις στο κομμάτι που θες να αλλάξεις / ελέγξεις, και πάντα αυτό αποτελείται από 5 σειρές κώδικα max. Ριξτου μια ματιά κάποια στιγμή του βιβλίου, είναι απ τα καλύτερα στο χώρο της πληροφορικής που έχω διαβάσει.

Συνδυάζει και χρήσιμες πρακτικές συμβουλές, και διασκεδαστικό / καλό τρόπο γραψίματος.

Papakaliati · 5 Νοεμβρίου 2017

Το μέχρι 5 γραμμές η κλάση είναι κάτι ενδεικτικό και επιπλέον λάθος στις περισσότερες περιπτώσεις. Κάθε κλάση θα πρέπει να κάνει ένα μόνο πράγμα, ανεξάρτητα σειρών.

Το να σπας 20 σειρές που κάνουν έναν μόνο πράγμα σε 4 κλασεις απλά για να ακολουθήσεις έναν κανονα που λέει μέχρι 5 σειρές είναι το ίδιο αστοχο με το να έχεις μια κλάση να κάνει 4 διαφορετικά πράγματα.

Άσε που άμα λες 5 σειρές ανά κλάση αυτοματος αποκλείεις χρήση σου switch με πάνω από μια περίπτωση και default...

Alithinos · 5 Νοεμβρίου 2017

Δεν είπα ανά κλάση, αλλά ανά μέθοδο. Ναι ο αριθμός 5 είναι ενδεικτικός, η ουσία είναι άμα μια μέθοδος κάνει πάνω από 1 πράγματα, αντί να τα βάλεις όλα χύμα μέσα, να χωρίσεις τα 'πράγματα' σε μεθόδους, ακόμα και αν αυτές είναι να κληθούν μόνο 1 φορά.

Ασε με να σου δείξω ένα παράδειγμα να δεις στη πράξη τι εννοώ.

Έχω μια κλάση με την εξής μέθοδο: (και άλλες, αλλά για το παρόν παράδειγμα ας μείνουμε σε αυτή)

    private void FixedUpdate()
    {
        if (State == QuestState.completed | State == QuestState.failed)
        {
            if (State == QuestState.completed)
                RewardPlayer();
            else
                Punishment();

            enabled = false;
            return;
        }
        else
            UpdateStages();
    }

Ναι είναι πάνω από 5 σειρές. Οι RewardPlayer(), και UpdateStages() είναι private στη κλάση που καλούνται μόνο απ, αυτή, και η Punishment() delegate που περνάται στην αρχικοποίηση.

Αν τη κοιτάξεις, μπορείς με μια γρήγορη ματιά να καταλάβεις τι κάνει, με τις πληροφορίες του πως ακριβώς πετυχαίνει ό,τι κάνει κρυμμένες.

Και αν κάποια στιγμή ενδιαφερθείς κατά το debugging να δεις πως ακριβώς κάνει κάτι απ' αυτά που κάνει, απλά κάνεις scroll και διαβάζεις την αντίστοιχη μέθοδο. (τις κρύβω και με #region / #endregion) για να περιορίζω το scrolling.

Οι 2 private μέθοδοι έχουν αυτό το κώδικα:

    #region REWARD PLAYER
    private void RewardPlayer()
    {
        if (sheet.curMoney + money > sheet.maxMoney)
            sheet.curMoney = sheet.maxMoney;
        else
            sheet.curMoney += money;
        
        if(item != null)
            inventory.Add(item);

        if (level)
            GameObject.FindWithTag("LevelUp").GetComponent<LevelUp>().ShowScreen();
    }
    #endregion

και

    #region UPDATE STAGES
    private void UpdateStages()
    {
        for (int i = 0; i < Stages.Count; i++)
        {
            Debugger(Stages[i]);
            if (Stages[i].State != StageState.completed)
            {
                Stages[i].UpdateStage();
                DetermineQuestState(Stages[i]);
            }
            else
                Stages.Remove(Stages[i]);
        }
    }
    #endregion

Αν δεν ακολουθούσα αυτές τις αρχές / συνήθειες, η FixedUpdate() η οποία καλείται κάθε 0.2s θα ήταν έτσι:

    private void FixedUpdate()
    {
        if (State == QuestState.completed | State == QuestState.failed)
        {
            if (State == QuestState.completed)
                {
                    if (sheet.curMoney + money > sheet.maxMoney)
                        sheet.curMoney = sheet.maxMoney;
                    else
                        sheet.curMoney += money;
        
                    if(item != null)
                        inventory.Add(item);

                    if (level)
                       GameObject.FindWithTag("LevelUp").GetComponent<LevelUp>().ShowScreen();
                }
            else
                Punishment();

            enabled = false;
            return;
        }
        else
        {
            for (int i = 0; i < Stages.Count; i++)
            {
                Debugger(Stages[i]);
                if (Stages[i].State != StageState.completed)
                {
                    Stages[i].UpdateStage();
                    DetermineQuestState(Stages[i]);
                }
                else
                    Stages.Remove(Stages[i]);
           }
        }
    }

Και μόνο που βλέπεις τόσα if/else μαζεμένα σε πιάνει δέος.

Οι πληροφορίες του τι κάνει η FixedUpdate θα είναι αναμεμιγμένες με το πως το κάνει ακριβώς.

Το αποτέλεσμα θα είναι:

# Κάθε φορά που θέλω να καταλάβω τι κάνει η FixedUpdate θα πρέπει να σπαταλώ περισσότερο χρόνο διαβάζοντας για να διαχωρίζω το τι από το πως.

# Επειδή άνθρωποι ήμαστε, και θα έχω μπροστά μου κώδικα του πως, θα τον διαβάσω χωρίς απαραίτητα να χρειάζεται να γνωρίζω το πως εκείνη τη στιγμή.

# Υπάρχει η πιθανότητα επειδή όλα είναι σε ένα σημείο λόγο ενός λάθους στο πως να χαλάσω το τι, ή το αντίστροφο.

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

Γιατί πολλές φορές θέλεις να κρύψεις τις λεπτομέρειες της υλοποίησης, για τους γνωστούς λόγους.

Από προσωπική εμπειρία έχω βρει ότι αυτό (και άλλες συμβουλές του βιβλίου που ανέφερα) με έχει βοηθήσει να είναι πιο εύκολα και γρήγορα αναγνώσιμο το πρόγραμμα μου, και να βρίσκω και τα bugs πιο εύκολα και γρήγορα ως αποτέλεσμα αυτού.

Το βιβλίο αυτό έφτασα να το διαβάσω αφού πρώτα είχα δει βίντεο του συγγραφέα στο youtube και μου έκανε καλή εντύπωση. Το πρόβλημα που αντιμετώπιζα το περιέγραφε πολύ καλά, και ήξερε τη λύση. Ήταν αυτό το πρόβλημα όπου φτάνεις σε ένα σημείο και αλλάζεις κάτι εδώ (πχ την ένταση της λάμπας στο δωμάτιο) και χαλάει κάτι τελείως άσχετο κάπου αλλού (πχ η κατεύθυνση της βαρύτητας στην εξωμείωση, και τα αντικείμενα με εξωμείωση φυσικής εκσφεντονίζονται στον ουρανό), όπως δείχνει η γελοιογραφία περίπου:

Είχα φτάσει σε σημείο να φοβάμαι να κάνω αλλαγές ή να προσθέσω κάτι καινούριο, για να μη χαλάσουν τα πάντα όλα. Απ' όταν διάβασα το βιβλίο, το πρόβλημα με τα αρκουδάκια εξαφανίστηκε, οι αιτίες των bugs εντοπίζονται και λύνονται πολύ πιο γρήγορα, και γενικά είναι πιο άνετο και γρήγορο το να διαβάζω το κώδικά μου και να του κάνω αλλαγές, ακόμα και σε αρχεία που είχα γράψει πριν 3 και 4 μήνες, για τα οποία υπάρχει 0 documentation.

Papakaliati · 5 Νοεμβρίου 2017

Δεν ακολουθησες την αρχη 5 γραμμες ανα μεθοδο, αλλα καθε μεθοδος να κανει ενα πραγμα.

Παρεπιπτοντως το

 if (State == QuestState.completed | State == QuestState.failed)
        {
            if (State == QuestState.completed)
                RewardPlayer();
            else
                Punishment();

            enabled = false;
            return;
        }
        else
            UpdateStages();

δεν εχει κανενα νοημα. Με μια swich η if εχεις πολυ πιο κομψο και ευαναγνωστο αποτελεσμα :

 
switch(State)
{
   case QuestState.completed:
   ...
   break;

   case State == QuestState.failed:
   ...
   break;

   default:
   ...
   break;
}

Αποφευγεις ενα αχαρο nesting που ειναι τερμα μπερδευτικο σε αυτον που το διαβαζει.

Το να αποφυγεις το code duplication δεν ειναι αυτοσκοπος, ειδικα αμα μιλαμε για μια γραμμη κωδικα, και εφοσων οδηγει σε αρκετα πιο δυσαναγνωστο και πολυπλοκο κωδικα.

Επιπλεον, το region - end region για μια μονο μεθοδο δεν εχει κανενα απολυτως νοημα. Γιατι δεν προσθετεις απλα comments / xml comment? Αφου single methods μπορεις να κανεις anyway collapse, ποτε δεν κερδιζεις απολυτως τιποτα.

Alithinos · 5 Νοεμβρίου 2017

Έστω και με switch. Αν οι RewardPlayer() και UpdateStages() είναι ξεχωριστές μέθοδοι και όχι ο κώδικας τους χύμα στα cases, είναι πολύ καλύτερα.

Τα region τα βάζω για να βρίσκω τη μέθοδο που θέλω στη κλάση με λιγότερο scrolling. Ναι γίνεται και collapse η μέθοδος χωρίς region, αλλά έχω παρατηρήσει ότι το εφέ του region στο VS με βοηθά να ξεχωρίσω αυτό που θέλω πιο γρήγορα.

Επεξ/σία 5 Νοεμβρίου 2017 από Alithinos

Ilias95 · 5 Νοεμβρίου 2017

Btw, επειδή έχω διαβάσει κι εγώ αυτό το βιβλίο, καλά όλα αυτά σε higher-level languages που υποστηρίζουν και OOP, αλλά σε lower level γλώσσες (πχ. C) δεν νομίζω ότι εφαρμόζονται με τίποτα.

7 Νοεμβρίου 2017

Δε με έπαιρνε ο ύπνος, οπότε είπα να το δοκιμάσω και εγώ.

Δες αυτό: https://sourcemaking.com/design_patterns/bridge

κα αυτό : https://sourcemaking.com/design_patterns/factory_method

ΥΓ1. Τα ονόματα τα άλλαξα λίγο, αλλά δε νομίζω να μπερδευτείς.

ΥΓ2. Έχω βάλει και code map.

ΥΓ3. Έβαλα το Newtonsoft για να μετατρέπει τα json σε αντικείμενα.

ΥΓ4. Επιδέχεται μεγαλύτερη διόρθωση, αλλά βαριόμουν να το προχωρήσω

Insomnia.zip

Σύνδεση

c#, ειναι readable αυτο;

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

παπι

Papakaliati

albNik

Papakaliati

Alithinos

παπι

Papakaliati

Alithinos

Papakaliati

Alithinos

Papakaliati

Alithinos

Ilias95

Επισκέπτης

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

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

Σύνδεση

Σύνδεση