refactoring

Refactoring wyrzucający części niezmienne poza IFa

Posted on

W IF’ach powinniśmy robić tak mało jak to tylko możliwe, przykład:

if (HasInternalDocuments(AdditionalDocuments))
{
    AdditionalDocumentsGroup = new AdditionalDocumentsGroup(AdditionalDocuments);
}
else
{
    AdditionalDocumentsGroup = new AdditionalDocumentsGroup(new List<Document>());
}

To co mi się nie podoba jest zaznaczone poniżej na niebiesko. Jest to część która jest identyczna zarówno dla sekcji IF jak i ELSE. Dopiero kod podkreślony na czerwono jest różny w IF oraz ELSE i to jest miejsce którym powinniśmy “sterować” właśnie za pomocą instrukcji warunkowych.

Duplication inside IF statement
Duplication inside IF statement

Refactoring który wyrzuca to co jest “wspólne” poza instrukcję warunkową

IEnumerable<Document> internalDocuments;

if (HasInternalDocuments(AdditionalDocuments))
{
    internalDocuments = AdditionalDocuments;
}
else
{
    internalDocuments = new List<Document>();
}

AdditionalDocumentsGroup = new AdditionalDocumentsGroup(internalDocuments)

Jeszcze dalej można to zamienić na coś poniższego. Nie jest to już jednak istota tego refactoringu to syntactic sugar który nam daje .NET:

var internalDocuments = HasInternalDocuments(AdditionalDocuments)
    ? AdditionalDocuments
    : Enumerable.Empty<Document>();

AdditionalDocumentsGroup = new AdditionalDocumentsGroup(internalDocuments);

To jest taki minimalny przykład. Najgorsze są długie switche w których kopiowane są kilku linijkowe wyrażenia gdzie tak naprawdę wartość testowanej w switchu wartości ma tylko wpływ na jedną zmienną.

Refactoring prowadzi do dalszych usprawnień

Ten typ zmieniania kodu jest bardzo niedoceniany. Zazwyczaj nie chodzi tylko o to że zmienimy trochę kodu. BARDZO CZĘSTO prowadzi to do dalszych usprawnień, bo zauważyliśmy że:

  • inne warunki można rozpisać inaczej lub opuścić
  • kolejne duplikacje kodu można uwspólnić
  • w jakieś kawałki kodu nie wejdziemy nigdy

Argumenty domyślne w C# i jak ich unikać

Posted on Updated on

TL;DR

ARGUMENTY DOMYŚLNE SĄ PRZEREKLAMOWANE I NADUŻYWANE

Domyślne wartości dla parametrów to śliski temat. Tak naprawdę nie ma nic fajnego w tym że możemy deklarować funkcje wyglądające jak:

User CreateUser(string name = "", int age = 18)
{
    return new User(name, age);
}

Nie będę tłumaczył dlaczego nie powinniśmy pisać kodu w ten sposób. Podchodziłem do opisania tego tematu już kilka razy i za każdym razem byłem niezadowolony na tyle, żeby się tym nie chwalić. Dziś po prostu przykład jak wygląda kod z domyślnym argumentem i bez.

Przed:

public void DrawShape(Rectangle rectangle, Image image, double heightRatio = 0.0D)
{
    if (heightRatio == 0.0D)
    {
        heightRatio = _screenService.DefaultHeightRatio;
    } 

    // ...
} 

Po:

public void DrawShapeBetter(Rectangle rectangle, Image image, double heightRatio)
{
    DrawShapeBetterInternal(rectangle, image, heightRatio);
}

public void DrawShapeBetter(Rectangle rectangle, Image image)
{
    var heightRatio = _screenService.DefaultHeightRatio;
    
    DrawShapeBetterInternal(rectangle, image, heightRatio);
}

private void DrawShapeBetterInternal(Rectangle rectangle, Image image, double heightRatio)
{
    // ...
}

Unikamy niepotrzebnego IF’a.

Mały refaktor – przerzucenie sprawdzania nulla

Posted on

Pierwszy przykład

Poprawny kod:

void Foo(object defaultValue)
{
    string defaultValueAsString = != null ? defaultValue.ToString() : string.Empty;
    // ...
}

Czytelniejszy kod:

void Foo(object defaultValue)
{
    string defaultValueAsString = (defaultValue ?? string.Empty).ToString();// 
    // ...
}

Oczywiście ktoś może sie doczepić że to nie jest to samo bo wywołamy dodatkowy ToString() na string.Empty – w mojej opinii pomijalne.

Drugi przykład

Poprawny kod:

// sting[] names;
return names == null ? string.Empty : string.Join(",", names);

Czytelniejszy kod:

// sting[] names;
return string.Join(",", names ?? Enumerable.Empty<string>());

Refactoring: EnableDisableButtons

Posted on Updated on

Było tak:

private void EnableButtons()
{
    SortOptionByReferenceButtonIsEnabled = true;
    SortOptionByTabletVersionButtonIsEnabled = true;
    SortOptionByOnlineVersionButtonIsEnabled = true;
    SortOptionByProductButtonIsEnabled = true;
    SortOptionByLabelButtonIsEnabled = true;
}

private void DisableButtons()
{
    SortOptionByReferenceButtonIsEnabled = false;
    SortOptionByTabletVersionButtonIsEnabled = false;
    SortOptionByOnlineVersionButtonIsEnabled = false;
    SortOptionByProductButtonIsEnabled = false;
    SortOptionByLabelButtonIsEnabled = false;
}

Zamieniamy na:

private void EnableButtons()
{
    EnableDisableButtons(true);
}

private void DisableButtons()
{
    EnableDisableButtons(false);
}

private void EnableDisableButtons(bool isEnabled)
{
    SortOptionByReferenceButtonIsEnabled = isEnabled;
    SortOptionByTabletVersionButtonIsEnabled = isEnabled;
    SortOptionByOnlineVersionButtonIsEnabled = isEnabled;
    SortOptionByProductButtonIsEnabled = isEnabled;
    SortOptionByLabelButtonIsEnabled = isEnabled;
}

Jak dobrze nazwać metodę EnableDisableButtons?

TODO: pokazać jak w R# szybko takie rzeczy się refactoruje.

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