среда, 7 сентября 2011 г.

DRY: Part 5 - Data duplication

Другие части эпической саги о вреде дублирования:

И последний, на мой взгляд, тип дублирования - дублирование данных. Магические строки и магические числа ужасны сами по себе, потому что их значения очевидны только для автора в момент написания кода. Дублирование этой магии ужасно вдвойне.

Небольшой пример, вычисляющий начисленную зарплату:

private decimal CalculateGrossPayment(int hoursWorked)
{
    decimal result = Math.Min(40, hoursWorked) * _hourlyPayRate;

    if (hoursWorked > 40)
        result += (hoursWorked - 40) * _hourlyPayRate * 1.5M;

    return result;
}

Казалось бы всем очевидно, что 40 - это продолжительность рабочей недели. Но вот в Дании рабочая неделя равна 37 часам. Да и наше правительство завтра может установить 45-часовую рабочую неделю, как в Чили. Придётся искать и изменять все копии этого магического числа. Но даже в таком простом случае, нет гарантии, что изменив первое число, вы не отвлечётесь посмотреть список самых сексуальных жещин, который вам по скайпу отправил коллега. Поэтому лучше сразу убрать дублирование с помощью рефакторинга Replace Magic Number with Symbolic Constant:

private decimal CalculateGrossPayment(int hoursWorked)
{
    const int workweekHours = 37;
    int regularPayHours = Math.Min(workweekHours, hoursWorked);
    decimal result = regularPayHours * _hourlyPayRate;

    if (hoursWorked > workweekHours)
        result += (hoursWorked - workweekHours) * _hourlyPayRate * 1.5M;

    return result;
}

То же самое относится к различного рода магическим строкам (строки соединений, строки форматирования и т.д.). Любая строка - потенциальный источиник очепятки. Например, строками задаются имена категорий тестов:

[Test, Category("Integration")]
public void ShouldDoX()
{
    // blah-blah
}

[Test, Category("Domain Model")]
public void ShouldDoY()
{
    // blah
}

[Test, Category("Inergation")]
public void ShouldDoZ()
{
    // blah-blah-blah
}

Я умудрился сделать опечатку в слове Integration, и вряд ли этот тест будет запущен. Знания о том, какие категории тестов существуют, должны быть представлены в единственном экземпляре:

public static class TestCategory
{
    public const string Integration = "Integration";
    public const string DomainModel = "Domain Model";
    // etc.
}

Теперь опечатки не страшны:

[Test, Category(TestCategory.Integration)]
public void ShouldDoX()
{
    // blah-blah
}

[Test, Category(TestCategory.DomainModel)]
public void ShouldDoY()
{
    // blah
}

[Test, Category(TestCategory.Integration)]
public void ShouldDoZ()
{
    // blah-blah-blah
}

И, наконец, самое неожиданное дублирование (которое можно было бы отнести к отдельному виду) - комментарии! Часто комментарии дублируют знания о том, как работает код. И, как и с любым другим видом дублирования, изменения должны быть внесены во все копии знаний.

Заботливые программисты обильно поливают код комментариями, когда логика вычислений не очевидна:

public decimal CalculatePayment()
{
    // divide annual salary by weeks count
    decimal grossPayment = _annualSalary / 52;
    // gross payment - taxes
    return grossPayment - _federalTax - _stateTax
           - grossPayment * 7.65M / 100; // ss & medicare
}

Беда в том, что комментарии непосредственно не влияют на работоспособность приложения. Поэтому многие программисты не обновляют их, внося изменения в комментируемый код. Так комментарии устаревают и начинают нагло врать. Например, если потребуется выплачивать зарплату раз в две недели, код наверняка будет изменён, а вот комментарии - далеко не всегда:

public decimal CalculatePayment()
{
    // divide annual salary by weeks count
    decimal grossPayment = _annualSalary / 26;
    // gross payment - taxes
    return grossPayment - _federalTax - _stateTax
           - grossPayment * 7.65M / 100; // ss & medicare
}

Чему теперь верить? Комментарий явно указывает, что делить надо на количество недель в году. Может ваш коллега не знает, что их 52?

Не нужно создавать копию знаний о том, что делает замысловатый код. Надо попытаться сделать код понятным. Самый простой способ сделать это - дать переменной или методу имя, раскрывающее его назначение. Найти хорошее имя довольно просто - оно уже содержится в комментарии:

public decimal CalculatePayment()
{
    const int payPeriodsInYear = 26;
    decimal grossPayment = _annualSalary / payPeriodsInYear;
    return grossPayment - CalculateTaxes(grossPayment);
}

private decimal CalculateTaxes(decimal grossPayment)
{
    return _federalTax + _stateTax
        + GetSocialSecurityTax(grossPayment)
        + GetMedicareTax(grossPayment);
}

private decimal GetSocialSecurityTax(decimal grossPayment)
{
    return grossPayment * 6.2M / 100;
}

private decimal GetMedicareTax(decimal grossPayment)
{
    return grossPayment * 1.45M / 100;
}

Комментариев нет:

Отправить комментарий