Kontynuuję omawianie projektu Pana kmph, który pojawił się już w poprzednim wpisie z serii „Code review”. Tam też znajdziecie link do repozytorium.

Dzisiaj krótko bo szykuję się do poruszenia tematu związanego z zastosowaniem wzorców i SOLIDa. W dodatku tekst powinien być w niedzielę, a mamy wtorek, także trzeba się śpieszyć :)

Co tym razem?

W tym wpisie zajmę się tematem nazw i tekstów w kodzie. Ponieważ większość pracy programisty to czytanie cudzego kodu dlatego dobrze, żeby nasz kod dobrze się czytało innym.

Pierwszą sprawą jest konwencja stosowana przy nazwach wszelakich. Drugi temat dotyczy tekstów znajdujących się w kodzie.

Nazwa własna

Większość, jeśli nie wszystkie powszechnie stosowane języki programowania posiadają coś takiego jak ogólnie przyjęta konwencja zapisu elementów wszelakich. Od stawiania nawiasów czy spacji, aż po styl wpisywania nazw metod czy zmiennych. I tym ostatnim się dzisiaj pokrótce zajmę. Ponieważ język C# również posiada coś takiego jak konwencję nazewnictwa, przygotowaną przez Microsoft.

Jedną z zasad tam podanych jest rozpoczynanie nazw publicznych pól, właściwości czy metod z dużej litery. Jest to powszechnie stosowany zapis i każdy programista C# to zauważa. Również wszystkie biblioteki czy frameworki pisane są w zgodzie z tą regułą. Jednak autor kodu nie zawsze się do tego stosuje w przypadku metod. A w przypadku właściwości w większości przypadków używa małej litery.

Przykładowo w tym fragmencie kodu:

    public static class Effect
    {
        public class EffectAndItsStorage<Eff> where Eff : IEffectComparable<Eff>
        {
            public readonly Eff effect;
            public readonly IEffectStorage affected;

            public EffectAndItsStorage (Eff _effect, IEffectStorage _affected)
            {
                effect = _effect;
                affected = _affected;
            }

            public override bool Equals(object obj)
            {
                if (obj == null) return false;
                if (obj.GetType() != this.GetType()) return false;
                var that = (EffectAndItsStorage<Eff>)obj;
                return effect.Equals(that.effect) && affected.Equals(that.affected);
            }

            public override int GetHashCode()
            {
                return HashCode.Combine(effect, affected);
            }
        }

        public static IEnumerable<EffectAndItsStorage<Eff>> iterateEffects<Eff>(IDictionary<IEnumerable<Eff>, IEffectStorage> src, EffectComparer<Eff> cmp) where Eff : IEffectComparable<Eff>
        {
            var done = new List<EffectAndItsStorage<Eff>> { };
            var todo = src.SelectMany(elt => elt.Key.Select(eff => new EffectAndItsStorage<Eff>(eff, elt.Value)))
                .Except(done).OrderBy(eff => eff.effect, cmp);
            while(todo.Any())
            {
                var ret = todo.First();
                done.Add(ret);
                yield return ret;
            }
        }
    }

Mamy metody, które pochodzą z samego .NETa czyli GetHashCode() i Equals(). I one są pisane dużą literą. Zaś poniżej metoda autora iterateEffects() zaczyna się z małej litery i wręcz ciężko mi było odnaleźć gdzie jest dokładnie jej nazwa.

To samo dotyczy publicznych pól na początku tej klasy.

Z resztą korzystając z narzędzia Resharper od razu widać gdzie te nazwy nie trzymają się konwencji bo dostajemy podkreślenie i po najechaniu stosowny komunikat:

Jak wspominałem w poprzednim wpisie – spójność jest ważna. Można oczywiście powiedzieć, że przyjęło się inną konwencję nazewniczą. Jednak w takim wypadku należałoby się do niej stosować w całym naszym kodzie i odpowiednio skonfigurować narzędzia do analizy tak, żeby wiedziały co robimy. Tylko zmieniając tę konwencję tracimy spójność z wbudowanymi elementami frameworka i powstają mieszanki jak ta na powyższym przykładzie.

Jeśli ktoś się zastanawia jaki jest polecany sposób zapisu różnych fragmentów naszej aplikacji to może się zapoznać z całym przewodnikiem dotyczącym wszystkich konwencji proponowanych przez Microsoft. Praktycznie każdy twórca zewnętrznych bibliotek i narzędzi się do nich stosuje dzięki temu czytany kod zawsze wygląda w podobny, znajomy sposób.

Tekst tu, tekst tam

W kodzie znajduje się dużo tekstu. Głównie opisów i wiadomości z parametrami.

W przypadku czystych opisów jedyna rada jaką mogę dać to zastanowienie się nad wyciągnięciem ich do jakichś zewnętrznych zasobów. Zwłaszcza jeśli chciałoby się w przyszłości mieć aplikację wielojęzyczną. Jednak jest to bardziej luźna sugestia. Same opisy są chyba we wszystkich miejscach przypisane do jakichś zmiennych więc nie ma walających się w kodzie magic-stringów.

Ale w przypadku tekstów z parametrami wyglądają one tak:

            if (typeResistMult == 0)
            {
                defender.addNoticeUpdate("monname is immune to the attack!", "minor");
            }
            else if (typeResistMult < 1)
            {
                defender.addNoticeUpdate("monname enjoys a " + (1 / typeResistMult).ToString() + "x resistance to the attack!", "minor");
            }
            else if (typeResistMult > 1)
            {
                defender.addNoticeUpdate("monname is " + typeResistMult.ToString() + "x vulnerable to the attack!");
            }

            if (shieldDamage + staminaDamage + healthDamage != 0 && kind == "major")
            {
                defender.addNoticeUpdate("monname has taken a hit!");
            }

            if (shieldDamage + staminaDamage + healthDamage != 0)
            {
                if (shieldDamage != 0)
                {
                    defender.shield -= shieldDamage;
                    defender.addNoticeUpdate("monname has lost " + shieldDamage.ToString() + " Shield!", kind);
                }

                if (staminaDamage != 0)
                {
                    defender.stamina -= staminaDamage;
                    defender.addNoticeUpdate("monname has lost " + staminaDamage.ToString() + " Stamina!", kind);
                }

                if (healthDamage != 0)
                {
                    defender.health -= healthDamage;
                    defender.addNoticeUpdate("monname has lost " + healthDamage.ToString() + " Health!", kind);
                }

                foreach (var eff in defender.effects.OfType<IOnDamaged>())
                    eff.onDamaged(this, defender);

                defender.addResourceUpdate();

                defender.addSleepUpdate(kind);
            }

O ile sama konkatenacja stringów może być ok to jednak łączenie ich w ten sposób z parametrami wprowadza nieporządek. Zwłaszcza kiedy parametry są co najmniej dwa. Wtedy linijka robi się oczywiście długa i można przeoczyć jakiś parametr. W takim wypadku wg mnie czytelniejszym rozwiązaniem jest zastosowanie metody String.Format() i zapisanie tego w taki sposób:

defender.addNoticeUpdate(String.Format("monname has lost {0} Health!", healthDamage.ToString()), kind);

Nie mówiąc o tym, że łączenie tekstu w wywołaniu metody to niezbyt korzystny pomysł. Taki zapis zmniejsza czytelność parametrów przekazanych do funkcji i utrudnia debugowanie. Na pierwszy rzut oka nie widziałem, że tam jest drugi parametr gdzieś na końcu. Lepiej by było tekst przypisać do pomocniczej, dobrze nazwanej zmiennej i dopiero ją przekazać do funkcji.

Jeśli chodzi o formatowanie tekstów to C# w wersji 6 wprowadził dodatkowy operator string interpolation, którego używa się podobnie jak String.Format().

Dodatkowo łącząc wartości z tekstem metoda ToString() jest wywoływana automatycznie dlatego nie ma potrzeby jej jawnego używania, co ponownie pokazuje m.in. Resharper:

Dodatkowo zastanawia mnie stosowanie w pewnych miejscach tablic stringów:

Próbowałem znaleźć gdzie mają one sens jednak wszędzie widzę przekazywanie jedynie tablicy jednoelementowej – czyli po prostu pojedynczego tekstu. Więc tutaj pytanie do autora, co za tym stało?

Zakończenie

Tak jak wspomniałem na wstępie dzisiaj tekst krótszy. Ale mimo to mam nadzieję, że wartościowy.

Jeżeli chcecie być na bieżąco z tekstami i newsami, nie tylko dotyczącymi wpisów z serii „Code review” ale ogólnie bloga to zapraszam na fanpage na Facebooku.

Zaś chętnych do podesłania mi kodu do oceny odsyłam do wpisu, w którym podaję więcej informacji na ten temat.