clean code

Czytelność kodu/nazw – przykłady

Posted on Updated on

Taki przykład w javascript(typescript):

private deepClone(item) {
    return jQuery.extend(true, {}, item);
}

to że akurat deep copy zrobi “jQuery.extend(true, {}, item)” to implementacja. Myśląc “implementacja” myślę “szczegół implementacyjny”, a dalej myślę i traktuję to jako “szczegół”. Czytając metodę która coś tam robi (akurat tutaj zmienia relację parent-child na indeksy, zeby klonowanie nie miało cykli, klonuje delikwenta i ustatwia z powrotem relacje), wystarczy nam wiedza że robimy “deepClone”. Nie tylko wystarczy, ale większa wiedza byłaby szkodliwa.

private changeParentRelationsToIds(item: ItemViewModel): ItemViewModel {
    // Cloned data must be without parent and children relations but must be added again after copy finishes
    let parent = item.parent;
    let children = item.children;

    item.parentId = this.changeToNullIfParentIsRoot(item.parent.id);
    item.parent = undefined;
    item.children = [];

    let clonedItem = this.deepClone(item);

    item.parent = parent;
    item.children = children;

    return clonedItem;
}

Przytoczona tutaj implementacja deepCopy nie jest jedyną. http://stackoverflow.com/questions/122102/what-is-the-most-efficient-way-to-clone-an-object to pytanie na SO ma 3 tysiące łapek w górę i jest o czym dyskutować w odpowiedziach i komentarzach. W innym kontekście mógłbym użyć innej implementacji bo aktualna nie działa. A ja nie chcę o tym wszystkim wiedzieć, w mojej metodzie robiącej coś konkretnego chcę wywołać:

let clonedItem = this.deepClone(item);

Nieużywane argumenty lambd

Posted on

ScrollView.ViewForZoomingInScrollView += x => PageImageView;

Jest tutaj nieużywany argument. Gdyby był potrzebny i wyglądało to np tak:

ScrollView.ViewForZoomingInScrollView += x => PageImageView(x);

to wszystko byłoby w porządku. Jeśli jednak mamy taki nieużywany możemy go sprytnie ominąć korzystając z podkreślenia (podłogi)

ScrollView.ViewForZoomingInScrollView += _ => PageImageView;

Dzięki temu od razu widzimy, że ten argument jest nieistotny i czytając nie musimy się na nim skupiać.

Zbyt długie linie – jak łamać?

Posted on

Tak może wyglądać automatyczne podzielenie linii przez R# gdy nie mieści mu się w zadanym limicie.

var applicationForm =
    _applicationFormService.GetApplicationForm(
        applicationState.GetFieldValueAsString(Furi.ReferenceNumber));

Co można zrobić z takim potworkiem?

Na przykład coś takiego:

var referenceNumber = applicationState.GetFieldValueAsString(Furi.ReferenceNumber);
var applicationForm = _applicationFormService.GetApplicationForm(referenceNumber);

Wiem, że może to wyglądać na małe oszustwo bo wprowadziliśmy dodatkową zmienną. To jednak nic złego. W ten sposób warto poprawiać kod.

Refactoring

Posted on

Ciężko temu refactoringowi nadać nazwę ale bardzo go lubię:

Przed:

if (attributeData != null)
{
    destinationProperty.SetValue(plainDestination, sourceProperty.GetValue(encryptedSource, null), null);
}
else
{
    destinationProperty.SetValue(plainDestination, DecryptValue(encryptedSource, sourceProperty, destinationProperty), null);
}

Po

WIP

Nazywanie zmiennych w lambdach #2 (x versuss t,p,pi…)

Posted on

Podoba mi się konwencja aby nazyw zmiennych w lambdach nazywać ‘x’, jeśli potrzeba kolejnej ‘y’, a jeśli kolejnej (nie zdażyło mi się, pewnie mało widziałem, młody jestem ;)) to ‘z’. Poniżej na przykładach. Gdy jednak uważasz że trzeba je nazwać bardziej opisowo to odsyłam do Nazywanie zmiennych w lambdach.

Zła nazwa

advisers.Single(t => t.AdviserId == adviserId);

Zła nazwa

advisers.Single(a => a.AdviserId == adviserId);

Dobra nazwa

advisers.Single(x => x.AdviserId == adviserId);

Bardzo zła nazwa (i na pewno trzeba z czymś innym kojarzyć)

interns.Single(i => i.Id == choosenInternId);

Gdy trzeba więcej ‘literek’ użyć

products.Where(x => x.Categories.Any(y => y.Id == categoryId));

Kopiowanie kodu i wędrujace złe nazwy

Ludzie kopiuja kod. Nie jest to złe samo w sobie. Np trzeba wykorzystać zgrabne kilkulinijkowe Linq w innym miejscu, a uwspólnianie kodu nie ma sensu. Przy takim kopiowaniu nie sprawdza się najczęściej czy nazwy lambd są sensowne i contractor nie stał się nagle literką h. Gdy trzymamy się konwencji x,y,z to nie zacznie nam się to rozjeżdżać.

Nazywanie zmiennych w lambdach

Posted on Updated on

Czasem się zastanawiam czy zmienne wewnątrz lambd powinny by pisane pełną nazwą czy raczej skracane. Dla porównania dwa poniższe kody:

Mapper.CreateMap<User, UserModel>()
    .MapProperty(target => target.City, source => source.Address.City)
    .MapProperty(target => target.PostalCode, source => source.Address.CityPostalCode);
Mapper.CreateMap<User, UserModel>()
    .MapProperty(x => x.City, src => src.Address.City)
    .MapProperty(x => x.PostalCode, src => src.Address.CityPostalCode);

Jest powszechnie wiadomo że nazwy zmiennych powinny być opisowe, nieskrótowe i nie powinny pozostawiać wątpliwości co do znaczenia w danym kontekście. To wszystko prawda. Zrobiłbym jednak jeden wyjątek.

W przypadku lambd forma skrócona jest lepsza, ponieważ nie chcemy aż tak skupiać się na zmiennej. Chcemy żeby była krótka (x, src) właśnie dlatego żeby nie zastanawiać się za bardzo co ona robi. Myślę o krótkich (jednolinijkowych) lambdach.

Dodatkowym atutem przemawiającym za stosowaniem niednoliterowych zmiennych jest, że w kilkulinijkowych lambdach możemy od razu rozpoznać, która zmienna jest parametrem metody anonimowej stworzonej przez lambdę, a która zmienną zadeklarowaną wewnątrz lambdy lub spoza lambdy (closure).