Literówki w kodzie, komentarzu i na głównej

Mam już taki wzrok, że widzę literówki i nie przepuszczam im. Po prostu taki już jestem, tak mój mózg pracuje. Oczywiście nie toczę wojen w zespołach o to. Są już automatyczne narzędzia które to wychwytują (dodatek ReSpeller do ReSharpera), a resztę sobie poprawię sam, to sama przyjemność.

Można więc myśleć, że to aż tak się nie zwraca bo w sumie kod się uruchomi niezależnie od tego czy klasa się będzie nazywać ArticleTpeConwerter czy ArticleTypeConverter. I że ten porządek, pomoże generalnie projektowi, ale przejdzie to na pewno niezauważone.

Ale, ale

Właśnie zdałem sobie sprawę, że te wszystkie niechluje, które się niczym nie przejmują (i walą literówki i nawet nie chce im się poprawiać po sobie) robią to samo z tekstami, które idą do klienta.

Inspiracja (można przeczekać minutę o CV, później jest o pracy):

Reguła DRY – tip #1

DRY – Don’t Repeat Yourself. Łamanie tej reguły ma tyle twarzy, że postanowiłem każdy mały przykład wrzucać.

Gorszy kod:

string GetFilePath()
{
    return IsForDebug
        ? $@"{Folder}\{Name}.{Extension}.{VersionNumber}"
        : $@"{Folder}\{Name}.{Extension}";
}

Lepszy kod:

string GetFilePathBetter()
{
    var filePath = $@"{Folder}\{Name}.{Extension}";

    if (IsForDebug)
    {
        filePath += $".{VersionNumber}";
    }

    return filePath;
}

Wiem że nie powinno się sklejać ścieżki w ten sposób tylko użyć Path.Combine(). Przykład z kodu.

Po komentarzu @marfusios2 zdałem sobie sprawę, że można to ulepszyć do

string GetFilePathMaybeBetter()
{
    var postfix = IsForDebug
        ? $".{VersionNumber}"
        : "";

    return $@"{Folder}\{Name}.{Extension}{postfix}";
}

Ciągle jest jeden zbędny string, ale już zdecydowanie krótszy.

Moje lokalne standardy pracy z gitem

Dorobiłem się już kilku wypracowanych przez lata małych standardów przydatnych w pracy z gitem. Stosowane LOKALNIE, mają więc sens tylko w mojej lokalnej pracy i muszą być wyczyszczone przed wykonaniem PUSHA.

  • Rozpoczynanie od „−− ” – jest to commit, który jest tylko lokalny i powienien być na pewno w całości usunięty przez pushowaniem. Może to być lokalna podmiana bazy danych, jakaś zmiana ułatwiająca szukanie aktualnego buga.
  • „.” (kropka) – oznacza na tyle małą rzecz, że po prostu kropka
  • „…” – powinno być dołączone do poprzedniego commita (f – fixup gdy robimy interactive rebase). Niby można od razu zrobić git commit --amend, ale jednak jest częścią czegoś większego, gdzie chcę jeszcze przez jakiś czas mieć osobne commity.
  • „…prev” – trochę jak poprzednio tylko powinien byc doklejony do jednego z poprzednich commitów.
  • „wip” – skrót od „Work In Progress”, coś na szybko, bo nie miałem czasu wpisać właściwego opisu, trzeba było szybko sprawdzić coś na innym branchu etc.
  • „wip treść commita” – gdy jesteś w trakcie czegoś to lepiej to zaznaczyć na początku commita niż na końcu (wtedy wyglądało by „treść commita wip”)
  • „++ treść commita” – gdy jesteś w trakcie czegoś czegoś większego, a zmiana którą zrobiłeś (nie jest cześcią aktualnego feature’a) powinna pójść prosto na branch develop

MasterCoder – CODING KOKS FIREFOX

mastercoder

W dzisiejszej odsłonie reflektory sławy padają na konkurs MasterCoder. Wreszcie było mi dane sprawdzić się w rozwiązywaniu zadań algorytmicznych na czas. Każdego dnia zmagałem się z nowymi zadaniami programistycznymi (weekendy na szczęście były wolne na regenerację umysłu). O 20 wpadało nowe zadanie i zaczynała się jazda, kto pierwszy ten lepszy, ale przede wszystkim liczyła się jakość. Zazwyczaj wysyłałem odpowiedź o 22. Następnie czekanie do 8 kiedy to spływały wyniki i ewentualnie można było wysyłać ponowne rozwiązania zadań.

Właśnie ok tej 21:30 spadały na mnie najtrudniejsze decyzje. Czy już wysyłać? Czy może jeszcze dopieścić? Moje testy przechodzą, ale czy pokryłem wszystkie przypadki? Czy dobrze zrozumiałem opisany problem? Adrenalina, presja czasu, zmęczenie… Wykonujący się kod jest bezlitosny. Jakże odmienne warunki od tego co ma się na co dzień w pracy. Wszyscy wiedzą jaka jest proza życia.

morcinek-ranking-mastercoder-2016

Ranking finałowy i zaznaczony ja. No cóż, postarałem się, walczyłem jak lew;) i jadę na finał. Odbędzie się w sobotę w Łodzi podczas konferencji Mobilization. Pierwszy etap to „Jeden z dziesięciu”, do kolejnego przechodzą 3 osoby i będzie to już live coding. No cóż, trzymajcie kciuki 😀

DLA PRAWDZIWYCH WYMIATACZY KODU – sprawdź czy podołałbyś zadaniom konkursowym.

Moje rozwiązania zadań umieszczone na GitHubie: https://github.com/kmorcinek/MasterCode2016

Otwórz gita w odpowiednim folderze

Taka mała rzecz a cieszy (dobra, dwie małe rzeczy).

Pod skrótem klawiszowym „Win+2” (czyli druga aplikacja która jest „pinned” do paska w Windows) otwiera mi się konsola gita. Moje podstawowe narzędzie pracy. I teraz trzeba tylko zmienić folder, w którym się uruchomi na ten, w którym projekcie aktualnie pracuję.

Klikamy właściwości tego programu (wprost z paska) i ustawiamy:

start-git-in-folder

Immutable Value Object przesyłany/serializowany (np po SignalR)

Najbardziej popularnym Value Object jest Money. Dzisiejsza implementacja jest okrojona, bo chciałbym pokazać tylko jedną rzecz. W tym przykładzie instancje możemy tworzyć tylko poprzez metode Create() (taka fabryka). Konstruktor jest prywatny.

public class Money
{
    public static Money Create(decimal value, string currency)
    {
        // some additional checks
        return new Money(value, currency);
    }

    Money(decimal value, string currency)
    {
        Value = value;
        Currency = currency;
    }

    public decimal Value { get; }
    public string Currency { get; }
}

Pewnie dla tego przykładu lepiej mieć po prostu publiczny konstruktor z dwoma parametrami, ale mam wiele przykładów innych bardziej skomplikowanych Value Objects, gdzie podejście z prywatnym konstruktorem i kilkoma fabrykami walidującymi różne przypadki ma sens.

Czy można przesyłać immutable Value Object poprzez SignalR?

Taki obiekt nie przejdzie serializacji JsonSerializerem i nie może być przesłany SignalR’em. Uważam, że nie ma nic złego w przesyłaniu takiego immutable Money między clientem a serverem (obydwa w .NET, nie tylko webem żyje człowiek 😉 ). Czy ktoś uważa, że to złe podejście i trzeba tworzyć do tego DTO? Postawiłem na prostotę.

Rozwiązanie

Trzeba dodać attrybut [JsonConstructor] do konstruktora.

public class Money
{
    public static Money Create(decimal value, string currency)
    {
        // some additional checks
        return new Money(value, currency);
    }

    [JsonConstructor]
    Money(decimal value, string currency)
    {
        Value = value;
        Currency = currency;
    }

    public decimal Value { get; }
    public string Currency { get; }
}

Źródło: JSON.net: how to deserialize without using the default constructor?

Jak testować/rozkminiać takie rzeczy

using Newtonsoft.Json;
using Xunit;
[Fact]
public void SerializationTest()
{
var money = Money.Create(1, "USD");
string json = JsonConvert.SerializeObject(money);
Money result = JsonConvert.DeserializeObject<Money>(json);
Assert.Equal(money.Currency, result.Currency);
}

view raw
MoneyTests.cs
hosted with ❤ by GitHub

Szlak by mnie trafił gdybym miał takie moje przemyślenia, przypuszczenia, POC testować na żywym projekcie, który się ileś tam uruchamia i potem muszę jeszcze sie przeklikać, żeby coś wywołać.

Izolujemy co istotne od tego co nieistotne i testujemy 🙂

TypeScript – deklaruj explicite zwłaszcza dane z JavaScriptu

Dzisiaj podczas przeklikiwania jednego ze scenariuszy w przeglądarce, coś poszło nie tak. Sprawdzam Unit Testy – wszystko działa. Trudno, tak bywa w „realu”. Debuguję i okazało się na końcu, że porównujemy:

"8" === 8

Skąd tam ten string???

Okazało się, że dane przychodzą z kontrolki x-editable dla angulara.

<span edit="4" onbeforesave="onbeforesave($data)"></span> 
$scope.onbeforesave = (value) => {
    return myService.update(value);
}

export class ChangeMaterialService {
    update(numberValue: number): boolean {
        // boom, in runtime value can be of type string
    }
}

typescript logo

Zadeklarowaliśmy elegancko, że value jest typem ‚number’ więc dlaczego przepuszcza stringa? Przy kompilacji (transpilacji?) TypeScriptu typ value to any. Zasady są takie, że any pasuje do wszystkiego. W runtime, mimo że value jakby nie pasuje do sygnatury metody update() to już nic nie jest sprawdzane – jesteśmy w świecie JavaScriptu i tam nie ma statycznego typowania.

Rozwiązaniem jest explicit ustawienie typu dla danych przychodzących w metodzie onbeforesave()

$scope.onbeforesave = (value) => {
    return myService.update(value);
}

powyższy kod się nie skompiluje więc musimy zmienić na:

$scope.onbeforesave = (value: string) => {
    return myService.update(Number(value));
}

lub na:

$scope.onbeforesave = (stringValue: string) => {
    let value = Number(stringValue);

    return myService.update(value);
}

Btw możecie podbić tą odpowiedź TypeScript Converting a String to a number na SO bo IMHO ta druga odpowiedź jest bardziej poprawna dla TypeScript.

Stack technologiczny w aktualnym projekcie

  • MVC 5 + WebApi 2 (najnowsze ale nie .NET Core)
  • Angular 1.5 (nie SPA, wtedy gdy trzeba na pojedynczej stronie) + TypeScript
  • TypeLite – biblioteka generuje DTO w TypeScript na podstawie DTO w C#. Gdy zmieniłem coś w C# to dostaję błędy kompilacji w TypeScript.
  • Bootstrap
  • AutoMapper
  • EntityFramework 6
  • MediatR – implementacja wzorca Command
  • SignalR – zarówno między ServerJS Client, jak i WindowFormServer, działa super.
  • Ninject
  • xUnit, Moq, AutoFixture

Jaki wyjątek rzucać gdy mamy nieobsłużonego enuma

Bardziej aktualnie: Prostsze rzucanie wyjątków, gdy nie mamy obsłużonego Enuma

Gdy sterujemy przepływem sterowania za pomocą switcha i tym co sprawdzamy jest enum to warto obsłużyć też przypadek default. Czyli co stanie się gdy enum ma wartość spoza tych wymienionych w case‚ach. Warto wtedy rzucić wyjątek.

Taki kod wygeneruje nam ReSharper (i jest to niezły kod):

void Execute(DocumentStatus documentStatus)
{
    switch (documentStatus)
    {
        case DocumentStatus.Draft:
            // Do sth
            break;
        case DocumentStatus.Released:
            // Do sth
            break;
        default:
            throw new ArgumentOutOfRangeException(nameof(documentStatus), documentStatus, null);
    }
}

Dlaczego jest to ważne? Poniższy kod zarówno się skompiluje jak i wykona poprawnie w runtime, mimo że żadna z wartości nie przyjmuje 111:

DocumentStatus AssignInvalidValue()
{
    return (DocumentStatus) 111;
}

Więcej w Rzutowanie na Enum z niepewnego źródła

Błąd, który dotychczas robiłem to wypełniałem 3-ci argument w konstruktorze klasy ArgumentOutOfRangeException. Pisałem tam coś w stylu:

throw new ArgumentOutOfRangeException(nameof(documentStatus), documentStatus,
    $"Enum '{documentStatus.GetType().Name}' has invalid value '{documentStatus}'");

Dzięki czemu przechwycony wyjątek wyglądał tak:

enum exception

Jest to dla tego przepadku zupełnie zbędne. Gdy przekażemy tam nulla to przechwycony wyjątek będzie wyglądał tak:

enum exception without message

A mając stack trace zobaczymy gdzie co i jak się podziało.

Metoda fabrykująca takie wyjątki

Jednak najlepszym sposobem jest jednak opakowanie tworzenia tego wyjątku. Dla mnie też kiedyś dziwne było żeby zwracać z funkcji wyjątek który dopiero później będzie rzucany. Przekonałem się jednak, że to dobra praktyka, takie scentralizowane tworzenie np message dla wyjątku. Np taka klasa:

public static ArgumentOutOfRangeException CreateMissingEnumException<T>(string paramName, T value)
    where T : struct 
{
    return new ArgumentOutOfRangeException(paramName, value,
        $"Enum '{value.GetType().Name}' has invalid value '{value}'");
}

i wywołanie oraz podgląd przechwyconego wyjątku:

enum create missing enum exception

Przykład fajnej Extension Method

Kod, który mnie natchnął:

var propertiesToDelete = someEntity.Properties
    .Where(x => x.PropertyName == "NAME")
    .ToList();

foreach (var property in propertiesToDelete)
{
    someEntity.Properties.Remove(property);
}

Natchnął bo zmieniłem na:

foreach (var property in someEntity.Properties.Where(x => x.PropertyName == "NAME"))
{
    someEntity.Properties.Remove(property);
}

To było słabe… gdybym sprawdzał po kimś takie coś, to bym może zauważył, ale sam to zrobiłem i nie zauważyłem: modyfikowanie kolekcji po której się iteruje, leci wyjątek. Code Review – wiele oczu patrzy – naprawdę się przydaje, żeby takich problemów unikać. Anyway, nie o tym.

Szerszy kontekst

Properties z powyższego kodu to ICollection<T>, które oznacza zależność jeden do wielu i Entity Framework zmapuje to i dobrze zapisze/wyciągnie z bazy, podobnie będzie też wyglądać odpowiadające mu DTO lub ViewModel:

public class SomeEntity
{
    // składnia C# 6
    public virtual ICollection<Property> Properties { get; } = new List<Property>();
}

Pod spodem jest jednak List<T>. A na typie List jest metoda RemoveAll(Predicate<T> predicate).

Napisałem więc coś takiego

using System;
using System.Collections.Generic;
using System.Linq;
public static class CollectionExtensions
{
public static void RemoveAll<T>(this ICollection<T> @this, Func<T, bool> predicate)
{
List<T> list = @this as List<T>;
if (list != null)
{
list.RemoveAll(new Predicate<T>(predicate));
}
else
{
List<T> itemsToDelete = @this
.Where(predicate)
.ToList();
foreach (var item in itemsToDelete)
{
@this.Remove(item);
}
}
}
}

Kod do odpalenia i testy

Dzięki temu wywołanie kodu z którym wystartowaliśmy będzie wyglądać tak:

someEntity.Properties.RemoveAll(x => x.PropertyName == "NAME");

Co zdecydowanie zwiększa czytelność. Ominiemy też wywołowanie LINQ i wszystko odbędzie się na bardziej wydajnej RemoveAll() z klasy List<T>.

Problemy

Wydaje mi się to fajnym kawałkiem kodu. Czy może ktoś widzi tutaj jakiś potencjalny problem?