как правильно устранить “запах” кода “Большой класс”?

#ооп #рефакторинг

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

private static JPanel createTasksMainInfoPanel(JTabbedPane taskTabbedPane) {
    JPanel panelForMainInfo = new JPanel();
    //code...
    addComponentForMainInfoBox(verticalBoxForTaskMainInfo, new String(),
            createTypeChoicePanel(max, min));
    JButton ok = new JButton("OK");
    createListenerForOk(ok, fieldForName, fieldForVarQuantity,
            FieldForLimitQuantity, FieldForCritQuantity, max,
            taskTabbedPane);
    addComponentForMainInfoBox(verticalBoxForTaskMainInfo, new String(), ok);
    panelForMainInfo.add(verticalBoxForTaskMainInfo);
    return panelForMainInfo;
}


Здесь я привел только начало и конец метода. В середине еще строк 50. Плюс в конце
у меня идут вызовы других методов, которые так же нужны что бы создать эту панель.
В них приходится передавать кучу параметров. В общем, в результате я имею длинные методы
с большим списком параметров. И мой класс, где создается GUI разрастается до огромных
размеров. Не лучше ли мне выделить для создания этой панели отдельный класс? Тогда
все что я передавал в параметрах можно было бы сделать полями этого класса. И можно
было бы выделить более компактные методы и без огромных списков параметров. Или может
я вообще зря это затеял. Ведь добавится еще один класс, а значит новые связи между
классами. К тому же, не противоречит ли это принципу единственной обязанности(single
responsibility principle)? Ведь эта панель входит в GUI, то есть ее отрисовка входит
в обязанность этого класса GUI?
    

Ответы

Ответ 1

Под "большим классом" Фаулер понимает не размер в строках, а размер функциональности.
То есть когда в классе много разнородных ответсвенностей класс считается "большим".
Следовательно чтобы назвать класс "большим" надо составить перечень его ответсвенностей
и определить насколько они разнородные.

В вашем случае функция класса состоит в сопоставлении визуальной структуры гуя и
внутренней структуры гуя. Поэтому на нем лежит две ответсвенности: знать визуальную
структуру и знать внутреннюю структуру. 

Визуальная структура гуя это то, что мы видим на экране: кнопка внутри панельки,
панелька на форме, форма слева на окне, нажатие на кнопку выводит диалог, и так далее.
Тут все понятно. 

Внутренняя структура гуя это то, как связаны объекты контролов между собой: кто кого
содрежит, кто кому сообщения посылает, как контролы создаются и настриваются, и прочее.
Но к внутренней структуре гуя (именно гуя) не относится то как объекты контролов связаны
с другими объектами неконтролами! Это другая ответсвенность.

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

Синхронный и асинхронный методы и дублирование кода

#c_sharp #async_await #рефакторинг

Здравствуйте. У меня есть два очень похожих метода, один обычный а другой асинхронный.
Выглядят они так: 

public async Task> GetDataAsync()
{            
    var types = _listCache.Get(CacheKey);
    if (types == null)
    {
        types = await GetDataAsync(_settings.Url, Converter);
        _listCache.Add(CacheKey, types);
    }
    return types;
}

public List GetData()
{            
    var types = _listCache.Get(CacheKey);
    if (types == null)
    {
        types = GetData(_settings.Url, Converter);
        _listCache.Add(CacheKey, types);
    }
    return types;
}


Можно заметить что методы почти одинаковы и код в них дублируется чего хотелось бы
избежать, Можно ли как-то сделать это безболезненно? Будут ли например дедлоки если
синхронную версию написать таким образом: 

public List GetData()
{                                
    return GetDataAsync().Result;
}

    

Ответы

Ответ 1

Да, дедлоки будут.

Допустим, вы вызываете в UI-потоке GetDataAsync().Result. При этом код выполняется
синхронно до момента await GetDataAsync(_settings.Url, Converter);, и далее возвращается
в синхронное ожидание выдачи результата Result.

Что происходит, когда отрабатывает await? Код пытается доставить continuation в основной
поток, но поток заблокирован до получения результата.

Вот вам и дедлок.



Не пытайтесь предоставить API «на все случаи жизни». Если функция по своей сути асинхронная,
предоставляйте только асинхронный интерфейс. Если функция по своей сути синхронная
(например, она не ожидает внешних событий, наподобие чтения файла или прихода информации
по сети), предоставляйте только синхронный интерфейс.

Если синхронный клиент пользуется по сути асинхронным интерфейсом, он по-хорошему
должен стать сам асинхронным. Ну или если хочется костыль, то выгрузите в фоновый поток:
Task.Run(<асинхронная функция>).Result.

Ещё по теме: https://blogs.msdn.microsoft.com/pfxteam/2012/04/13/should-i-expose-synchronous-wrappers-for-asynchronous-methods/


Ответ 2

У подобного решения три проблемы.


Программа повиснет, если такой код будет выполняться в потоке UI или любом другом
однопоточном контексте. Подробнее о причинах и как бороться - тут:
Зависает оператор `await` в оконном приложении / программа висит при вызове Task.Result
или Wait
Вылетевшее исключение обернется в AggregateException, что сделает некрасивым стек
вызовов, и затруднит классификацию исключений в блоках catch.
Вызываемые методы GetDataAsync и GetData могут работать по-разному.

Как сделать данный код более компактным?

#python #инспекция_кода #рефакторинг

def rcvdata(cmd,size):
    global s;
    s.settimeout(1)
    try:
        sendCommand(cmd);data = s.recv(size)

    except:
        try:
           sendCommand(cmd);data = s.recv(size)

        except:
            try:
                sendCommand(cmd);data = s.recv(size)

            except:
                try:
                    sendCommand(cmd);data = s.recv(size)

                except:
                    try:
                        sendCommand(cmd);data = s.recv(size)

                    except:                       
                        try:
                            sendCommand(cmd);data = s.recv(size)

                        except:
                            s.close()

    return(data)

    

Ответы

Ответ 1

def rcvdata(cmd, size):
    data = None
    s.settimeout(1)
    for trying in range(6):
        try:
            sendCommand(cmd)
            data = s.recv(size)
        except:
            if trying == 5:  # Последняя попытка, неудачная
                s.close()
    return data


Или если надобности в каких-то действиях после цикла нет, то можно уменьшить вложенность:

def rcvdata(cmd, size):
    s.settimeout(1)
    for _ in range(6):
        try:
            sendCommand(cmd)
            return s.recv(size)
        except:
            pass  # Здесь по желанию можно воткнуть time.sleep(1)
    # Сюда мы попадаем только после шести неудачных попыток
    s.close()
    return None


Почитайте в любом учебнике Python про циклы.



А ещё нехорошо перехватывать ВСЕ исключения, потому что тогда программу невозможно
будет закрыть (выход — тоже вполне себе исключения SystemExit и иногда KeyboardInterrupt),
а также будут пропущены баги внутри sendCommand. Следует перехватывать только те исключения,
которые здесь ожидаются (здесь, наверно, что-то вроде socket.error или IOError).

А ещё s.recv(size) не гарантирует, что придёт ровно size байт — в зависимости от
качества связи, особенностей ОС, фазы Луны и звёзд на небе может вернуться меньше.
А по таймауту — и вовсе просто ноль байт. Не знаю, нужно ли это в вашем конкретном
случае, но про это желательно не забывать.

В общем, плоховат код всё равно


Ответ 2

Предложу свой вариант с задержкой между вызовом recv в 1 секунду и 10 попытками:

def rcvdata(cmd, size):
    global s
    s.settimeout(1)

    max_count = 10

    while True:        
        try:
            max_count -= 1
            if max_count <= 0:
                return 

            sendCommand(cmd)
            data = s.recv(size)
            return data

        finally:
            s.close()


Ответ 3

Оставлю и я свой вариант_)

def rcvdata(cmd,size):
    global s;
    s.settimeout(1)
    t = 0
    while t!=7:
        try:
            sendCommand(cmd)
            data = s.recv(size)
        except:
            t+=1
    s.close()

Стоит ли так рефакторить? Замена условий на простые математические формулы

#php #рефакторинг

Периодически делаю мелкий рефакторинг своего кода над текущим проектом, я раньше
вообще не занимался этим и только начинаю изучать это дело. Вот хочу привести пример
элементарного кода, который я захотел сделать еще более "элементарным", как мне кажется

Есть таблица Orders, состоящая из id, user_id, created_at, type, value, status, в
данном примере нас интересуют только поля value и type. value - любое целочисленное
положительное число, type - либо 0, либо 1 (0 - списание, 1 - начисление)

И собсно простой метод getPoints в классе User, который подсчитывает количество доступных
очков пользователя

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $k =>
$value) {
            if ($value['type'] == 0)
                $return -= $value['value'];
            if ($value['type'] == 1)
                $return += $value['value'];
        }
        return $return;
}


От нечего делать я решил его немного изменить

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $value)
                $return += (-1 + 2*(int)$value['type'])*$value['value']; //$value['type']
равно либо 0, либо 1. 0 - вычитание, 1 - сумма. Формула -1 + 2*$value['type'] нужна
для сокращения кода -1 + 2*0 = -1 -1 + 2*1 = 1

        return $return;
    }


То есть по сути просто избавился от условий и заменил это дело формулой (предполагается,
что значения 0 и 1 никогда не будут меняться). Так вообще нормально делать? Стоит ли? 
    

Ответы

Ответ 1

Если уже "рефакторить", то где то так

public function getPoints() {
        $return = 0;
        foreach (Order::findAll(['user_id'=> $this->id, 'status' => 1]) as $k =>
$value) {
            $type = $value['type'];
            $val = $value['value'];
            if ($type == 0) {
                $return -= $val;
            else if ($type == 1) {
                $return += $val;
            } else {
              # а тут добавить вывод в лог, может что то пошло не так
            }
        }
        return $return;
}


Ответ 2

Такие сокращения не всегда есть хорошо, первый вариант гораздо быстрее и удобнее
прочитать, и потратить меньше времени на то, что бы разобраться в логике.

P.S. По этому поводу на харбе недавно хорошая статья вышла: https://habrahabr.ru/post/347166/

Как выделить шаблонность из функции?

#cpp #проектирование #шаблоны_с++ #рефакторинг

Имеем - шаблонная функция, еще и рекурсивная, в которой много чего подтянуто из разных
(заголовочных) файлов, а шаблонность только одна - запись найденного значения в итератор.
Что-то примерно такое

template
void terribleFunction(Itor it, type1 param1, type2 param2, ...)
{
    // Всякие дела
    for(....
    {
        // И еще дела

        // Первое использование it
        if(...) terribleFunctiom(it,p1,p2,...);
        // И еще...

        // Второе использование it
        if (...) *it++ = что-то;  

        // И еще...
    }
}


Примерно так. Хочется максимально спрятать весь код - в котором ничего шаблонного
- или в библиотеку, или в отдельный файл - вобщем, как минимум постараться не плодить
зависимости в заголовочном файле, выписывая в нем весь код, постараться максимально
убрать его в какие-нибудь вспомогательные функции, что ли. Но что-то что ни придумываю
- все через одно неприличное место...

Какой бы тут хитрый метод применить, намекните?...
    

Ответы

Ответ 1

Если единственное использование - это X x = expr; *it++ = x;, то эту операцию можно
обернуть в std::function f, передавать не итератор а эту функцию, использовать
как f(x).

Либо можно передавать класс с виртуальными функциями.


Ответ 2

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

Для начала объявим интерфейс итератора:

class IIterator{
public:
    virtual void next() = 0;
    virtual void setValue(int value) = 0;
    virtual int value() const = 0;
    virtual ~IIterator(){}
};


Думаю вы знаете тип значения, которое хотите писать в итератор. Для примера я взял int. 

Теперь нам нужна конкретная реализация, причем для всех возможных итераторов. Да,
снова шаблоны:

template
class Iterator : public IIterator{
    It _iterator;
public:
    explicit Iterator(const It &iterator):
        _iterator(iterator)
    {}
    void next(){
        ++_iterator;
    }
    void setValue(int value){
        *_iterator = value;
    }
    int value() const{
        return *_iterator;
    }
};


Теперь возьмем весь код из void terribleFunction(Itor it), перенесем его в функцию
void terribleFunctionHelper(IIterator &it), и внесем некоторые изменения в использование
итераторов:

void terribleFunctionHelper(IIterator &it){
    //...
    it.next();         //Раньше тут было ++it;
    it.setValue(42);   //Раньше тут было *it = 42;
    //...
}


Код void terribleFunction(Itor it) теперь станет таким:

template
void terribleFunction(It it){
    Iterator iterator(it);
    terribleFunctionHelper(iterator);
}


В результате этих нехитрых манипуляций, все зависимости оказались в нешаблонной terribleFunctionHelper.
Можно смело переносить ее код в cpp файл.    

Полный пример

Создание переменной ссылочного типа при каждой итерации цикла. Это затратно?

#c_sharp #оптимизация #инспекция_кода #рефакторинг

Есть вот такой код:

...
Match match;
foreach (var value in egeQuestionValues)
{
    match = Regex.Match(value, $@"(?<={number}\()\d+(?=.*%\))");
    if (!match.Success)
        throw new ArgumentException($"The record {value} is incorrect", nameof(egeQuestionValues));

    egeQuestionIntValues.Add(int.Parse(match.Value));                
}

return egeQuestionIntValues.Average();


Я создал переменную match специально вне цикла foreach, чтобы в рантайме не создать
ее при каждой итерации. Делал, думаю что это затратно. Но ReSharper мне советует отрефакторить
этот код таким образом:

...
foreach (var value in egeQuestionValues)
{
    var match = Regex.Match(value, $@"(?<={number}\()\d+(?=.*%\))");
    if (!match.Success)
        throw new ArgumentException($"The record {value} is incorrect", nameof(egeQuestionValues));

    egeQuestionIntValues.Add(int.Parse(match.Value));                
}

return egeQuestionIntValues.Average();


т.е. сейчас при каждой итерации переменная match будет создаваться каждый раз. Разве
это не затратно? ReSharper советует поступать так не только с типом Match и с другими
типами так было.
    

Ответы

Ответ 1

Разницы особой нету.

Все переменные ссылочного типа без инициализации автоматически инициализируются null-значением.

Что сама по себе переменная ссылочного типа? Это переменная, которая ссылается на
какой-то адрес.

Смена ссылки - это быстрая операция, так как не создается никаких дополнительных
объектов, как это происходит со значимым типом.

Скорее всего, компилятор приведет код к первому случаю и IL-код будет идентичным.

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

Второй вариант более приятен, так как переменная находится прямо в месте ее использования
и можно сразу понять, что за циклом она нигде использоваться не будет.


Ответ 2

Я что-то отличий не вижу, но может кто чего заметит пусть сигнализирует

static void Main(string[] args)
{
    List people1 = GetWithVarInWhile();

    List people2 = GetWithVarOutWhile();
}

private static List GetWithVarInWhile()
{
    List result = new List();

    int i = 10;
    while (i > 0)
    {
        var p = new Person { Name = $"Name{i}" };
        result.Add(p);
        i--;
    }

    return result;
}

private static List GetWithVarOutWhile()
{
    List result = new List();

    int i = 10;
    Person person = null;
    while (i > 0)
    {
        person = new Person { Name = $"Name{i}" };
        result.Add(person);
        i--;
    }

    return result;
}


Вот первый

.method private hidebysig static class [mscorlib]System.Collections.Generic.List`1 
    GetWithVarInWhile() cil managed
{
  // Размер кода:       71 (0x47)
  .maxstack  4
  .locals init ([0] class [mscorlib]System.Collections.Generic.List`1
result,
       [1] int32 i,
       [2] class ConsoleWhile.Person p,
       [3] bool V_3,
       [4] class [mscorlib]System.Collections.Generic.List`1 V_4)
  IL_0000:  nop
  IL_0001:  newobj     instance void class [mscorlib]System.Collections.Generic.List`1::.ctor()
  IL_0006:  stloc.0
  IL_0007:  ldc.i4.s   10
  IL_0009:  stloc.1
  IL_000a:  br.s       IL_0037
  IL_000c:  nop
  IL_000d:  newobj     instance void ConsoleWhile.Person::.ctor()
  IL_0012:  dup
  IL_0013:  ldstr      "Name{0}"
  IL_0018:  ldloc.1
  IL_0019:  box        [mscorlib]System.Int32
  IL_001e:  call       string [mscorlib]System.String::Format(string,
                                                          object)
  IL_0023:  callvirt   instance void ConsoleWhile.Person::set_Name(string)
  IL_0028:  nop
  IL_0029:  stloc.2
  IL_002a:  ldloc.0
  IL_002b:  ldloc.2
  IL_002c:  callvirt   instance void class [mscorlib]System.Collections.Generic.List`1::Add(!0)
  IL_0031:  nop
  IL_0032:  ldloc.1
  IL_0033:  ldc.i4.1
  IL_0034:  sub
  IL_0035:  stloc.1
  IL_0036:  nop
  IL_0037:  ldloc.1
  IL_0038:  ldc.i4.0
  IL_0039:  cgt
  IL_003b:  stloc.3
  IL_003c:  ldloc.3
  IL_003d:  brtrue.s   IL_000c
  IL_003f:  ldloc.0
  IL_0040:  stloc.s    V_4
  IL_0042:  br.s       IL_0044
  IL_0044:  ldloc.s    V_4
  IL_0046:  ret
} // end of method Program::GetWithVarInWhile


А вот второй

    .method private hidebysig static class [mscorlib]System.Collections.Generic.List`1 
    GetWithVarOutWhile() cil managed
{
  // Размер кода:       73 (0x49)
  .maxstack  4
  .locals init ([0] class [mscorlib]System.Collections.Generic.List`1
result,
       [1] int32 i,
       [2] class ConsoleWhile.Person person,
       [3] bool V_3,
       [4] class [mscorlib]System.Collections.Generic.List`1 V_4)
  IL_0000:  nop
  IL_0001:  newobj     instance void class [mscorlib]System.Collections.Generic.List`1::.ctor()
  IL_0006:  stloc.0
  IL_0007:  ldc.i4.s   10
  IL_0009:  stloc.1
  IL_000a:  ldnull
  IL_000b:  stloc.2
  IL_000c:  br.s       IL_0039
  IL_000e:  nop
  IL_000f:  newobj     instance void ConsoleWhile.Person::.ctor()
  IL_0014:  dup
  IL_0015:  ldstr      "Name{0}"
  IL_001a:  ldloc.1
  IL_001b:  box        [mscorlib]System.Int32
  IL_0020:  call       string [mscorlib]System.String::Format(string,
                                                          object)
  IL_0025:  callvirt   instance void ConsoleWhile.Person::set_Name(string)
  IL_002a:  nop
  IL_002b:  stloc.2
  IL_002c:  ldloc.0
  IL_002d:  ldloc.2
  IL_002e:  callvirt   instance void class [mscorlib]System.Collections.Generic.List`1::Add(!0)
  IL_0033:  nop
  IL_0034:  ldloc.1
  IL_0035:  ldc.i4.1
  IL_0036:  sub
  IL_0037:  stloc.1
  IL_0038:  nop
  IL_0039:  ldloc.1
  IL_003a:  ldc.i4.0
  IL_003b:  cgt
  IL_003d:  stloc.3
  IL_003e:  ldloc.3
  IL_003f:  brtrue.s   IL_000e
  IL_0041:  ldloc.0
  IL_0042:  stloc.s    V_4
  IL_0044:  br.s       IL_0046
  IL_0046:  ldloc.s    V_4
  IL_0048:  ret
} // end of method Program::GetWithVarOutWhile


Ответ 3

Компиляторы сейчас практически никогда не компилируют код вот прямо так, как написано
в программе. Они стали намного умнее, чем были во времена Кернигана и Ричи.

Если у вас появляется переменная, а потом исчезает, и появляется другая переменная,
компилятор не будет наивно освобождать память, чтобы тут же снова её выделить. Он заметит,
что новую переменную можно использовать место от старой переменной, и так и сделает.

Смысла «помочь» компилятору и соптимизировать такие вот мелочи за него нет никакого:
компилятор делает это, поверьте, куда лучше нас с вами.

Если вы хотите соптимизировать, лучше пытаться улучшать алгоритмы. Переход от квадратичного
алгоритма к логарифмическому даст заметное ускорение и улучшение производительности;
выигрыш одной локальной переменной, даже если и имел бы какие-то преимущества, принёс
бы вам существенно меньше микросекунды, и всё равно никем не был бы замечен.

Должен ли проверяющий метод check/validate возвращать boolean или выбрасывать исключение?

#php #рефакторинг #чистый_код

Этот вопрос не обязательно относится к PHP, просто это моя сфера.

При построении методов, которые "узнают" имеет ли объект/переменная что-то, разрешено
ли что-то (is, has, can), то возвращается true/false. Тут всё понятно. 

А если метод должен что-то проверить или свалидировать (check()/validate()), то что
должно быть? Должен ли метод вернуть булево значение, на основе которого мы поймём,
что что-то было некорректно? Или надо выбрасывать исключение (throw new InvalidDataExecption)
и метод валидации оборачивать в try/catch? Какой правильный подход?

Во фреймворках при vlaidate возвращаются какие-то данные (true, false и даже массивы
с ошибками), но ведь мы просто валидируем, а не говорим "Валидно ли вот это (isValid($data))",
как в случае булевых запросов выше.
    

Ответы

Ответ 1

Да как бы up to you. Может вам надо просто подсветить пропущенное при заполнении
формы поле (тру/фолс). А может необходимо вывести дополнительное сообщение об ошибке
(вот вам уже эррей). А где-то надо писать в лог ексепшены. Всё упирается в задачи,
которые валидатор должен решать. имхо)


Ответ 2

Зависит от логики метода. 
Например у вас есть метод validateString, который принимает аргументом строку. 
И что-то в ней проверяет. Тогда логично сделать результат true или false в зависимости
от результата проверки, а exception уже бросать в случае если на проверку пришел не
тот тип данных.

Другой пример. Если работа метода не возможна дальше, когда результат false, то тогда
метод validateString возвращает true если проверка прошла, и бросаете exception если нет.


Ответ 3

С одной стороны конечно было бы круто получить сразу ответ на просьбу валидации данных,
но сам по себе метод validate (по смыслу) должен просто что-то сделать. Как раз, как
указал, check и validate похожи на void. 

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


Формируется класс валидации, где есть метод validate, который выкидывает какие-то
исключения (их надо будет отлавливать)
В качестве обертки для случая true/false, я бы сделал метод isValid, который включал
бы в себя try метода validate и делал return в случае успеха или неудачи


Как вариант еще один метод, который валидировал бы все данные без выбрасывания исключений.
Он бы копил ошибки, а затем возвращал их (или пустой массив, если все корректно).

Таким образом, на одну только валидацию 3 реализации. Можно обернуть в абстрактный
класс для дальнейшего использования. Ну тут уже персонально.

Раздельная компиляция и рефакторинг

#cpp #компиляция #cpp11 #рефакторинг

Например, у меня есть несколько файлов (все по правилу odr).

SuperClass.h SuperClass.сpp

SubClass.h SubClass.cpp (наследуется от SuperClass)

main.cpp

Скомпилировал, получил SuperClass.o, SubClass.o, main.o и все объединил компоновщиком
в programm - все хорошо работает.

Потом решил переделать приватную реализацию SuperClass, добавить некоторые публичные
методы, удалить приватные поля и добавить еще один класс SuperSuperClass, от которого
теперь наследуется SuperClass. 

Скомпилировал SuperSuperClass SuperClass и объединил компоновщиком со всеми остальными
- все хорошо работает, хоть я изменил иерархию наследования, внес изменения, но не
трогал имена функций которые вызываются в остальных файлах. Но правило ODR нарушено?
Я не перекомпилировал другие файлы, в которых осталось старое определение SuperClass
и нет никакой информации об обновлении иерархии наследования.

А вдруг я захочу добавить в SuperSuperClass виртуальну функцию? При каких случаях
нужна перекомпиляция остальных файлов? 
    

Ответы

Ответ 1

На самом деле всё просто.

Если вы поменяли header — все зависимые от него файлы (то есть, все cpp-файлы, которые
включают его прямо или косвенно) должны быть перекомпилированы.

Если вы поменяли cpp-файл, то по идее только его и надо перекомпилировать. (Если
вы, разумеется, не #include-ите его в другие файлы.)

Если вы пользуетесь makefile'ом, зависимости нужно прописать там, тогда повторная
компиляция произойдёт автоматически, соберутся лишь нужные таргеты. Если ваш проект
слишком большой для ручной расстановки зависимостей, вам поможет утилита makedepend,
которая автоматизирует этот процесс.


Ответ 2

Правило ODR тут не причём вообще . То, что у Вас всё работает правильно говорит о
том, что Вам


Повезло. Вы имеете неопределённое поведение, но так уж случилось, что всё работает
правильно.
Опять повезло, но уже с компоновщиком. Компоновщик очень умён, и в o файлах, до последней
сборки содержатся лишь ссылки, которые потом очень умно заменяются. 


Общее правило таково: если хоть что-то в заголовке было изменено(в определении класса,
даже если это просто порядок функций), все эти изменения, должны быть донесены до всех
объектных файлов. В противном случае UB(неопределённое поведение)



Не прав я касательно ODR оказался, правило как раз в этом разделе:


  3.2/6 <...>There can be more than one definition of a class type <...> Given such
an entity named D defined in more than one translation unit, then each definition of
D shall consist of the
  same sequence of tokens; and <...>

Определение стоимости рефакторинга [закрыт]

#рефакторинг

        
             
                
                    
                        
                            Закрыт. На этот вопрос невозможно дать объективный ответ.
Ответы на него в данный момент не принимаются.
                            
                        
                    
                
                            
                                
                
                        
                            
                        
                    
                        
                            Хотите улучшить этот вопрос? Переформулируйте вопрос,
чтобы на него можно было дать ответ, основанный на фактах и цитатах, отредактировав его.
                        
                        Закрыт 3 года назад.
                                                                                
           
                
        
Есть проект, написанный на borland builder c++ 6.
Требуется переделать код под msvc 2015, т.е. по сути сделать рефакторинг.
Объем кода известен. Скажем, суммарно 3 Мб (это без ресурсов: иконок, картинок и
т.п. - чисто код).  

Подскажите, какие есть best practices для определения нижней границы стоимости рефакторинга
 (в любых единицах: человеко-часы, денежные единицы - что угодно). 
    

Ответы

Ответ 1

Эвристический алгоритм для одного разработчика

Пометка: допустим, что 3Mb это примерно 80т строк кода.


Начинаем делать рефакторинг и делаем его в течение, скажем, 4х часов.
Подсчитываем количество строк кода, которые удалось обработать за это время.
Делим это число на 4ч, получаем грубую оценку X строк за час.
Вычисляем общее количество часов, выполняя деление 80000/X, получив Y часов.
Полученное значение необходимо умножить на "коэффициент разработчика" примерно от
0.5 до 4, для того чтобы зарезервировать время на непредвиденные сложности. Т.е. допустим
разработчик-оптимист оценил задачу на 10ч, а по факту вышло 25ч из которых 15ч ушло
на непредвиденные сложности в процессе работы, то коэффициент был бы равен 2.5 для
данной задачи.
Ну и к полученным в ходе данных вычислений часам, необходимо добавить ещё какой-то
процент "на всякий пожарный", скажем 15% от полученного числа.


На примере


За 4ч скажем получилось обработать 250 строк.
250/4 ~~ 62стр/ч.
80000/62 ~~ 1290ч.
Пусть коэффициент будет 3. 1290ч * 3 =  3870ч.
И с 15% будет 4450.5ч.
4450.5ч - это примерно 111 человеко/недель, т.е. около 2-х человеко/лет.


Ну, а зная человеко-часы, вычислить денежные единицы - задача тривиальная.

Чистый код: Switch

#любой_язык #switch #рефакторинг

Корректно ли выносить цепочку Switch в отдельный метод, если цепочка довольно большая?
Или можно оставить ее в том же методе? Какой вариант выглядит лучше/красивее/чаще используется?
    

Ответы

Ответ 1

Вы подходите не совсем правильно. Разбивать функцию на части нужно не по формальным
признакам («длинный switch»), а по логическим. Вы должны задать себе вопрос: имеет
ли ваш кусок кода самостоятельный смысл? (Например: можно ли сказать несколькими словами,
что именно этот кусок делает?)

Если ответ на этот вопрос положительный, вынесите этот кусок в функцию, и назовите
её этими самыми словами. Если отрицательный — оставляйте всё как было.



Небольшое дополнение. Кодировать switch можно по-разному. Можно оставить его как
switch. Если это отображение одного объекта на другой, можно закодировать его как выборку
из std::unordered_map. А возможно, ваш switch лучше представить в виде вызова виртуальной
функции.

Когда именно и как правильно — снова-таки зависит от смысла. Пример:

string text;
switch (code)
{
case 100:
    text = "Continue";
    break;
case 101:
    text = "Switching protocols";
    break;
case 200:
    text = "OK";
    break;
case 301:
    text = "Moved permanently";
    break;
case 404:
    text = "Not found";
    break;
}


по идее лучше закодировать так:

unordered_map message_text
{
    { 100, "Continue" },
    { 101, "Switching protocols" },
    { 200, "OK" },
    { 301, "Moved permanently" },
    { 404, "Not found" }
};


и в коде

string s = message_text[200];


или там

string s;
auto it = message_text.find(200);
if (it != message_text.end())
    s = it->second;


(и выделить в отдельную функцию GetHttpMessageByCode).



Пример того, когда switch разумно заменить на иерархию классов:

switch (employee_kind)
{
case EmployeeKind::regular:
    salary = get_base_salary();
    bonus = total_profit * bonus_ratio / number_of_employees;
    break;
case EmployeeKind::external:
    salary = get_work_hours * get_hourly_rate(employee_id);
    bonus = 0;
    break;
case EmployeeKind::manager:
    salary = get_base_salary();
    bonus = bonus_fund / number_of_managers;
    if (salary < 100)
        bonus += 100 - salary;
    break;
}


Такой код можно заменить на иерархию классов:

class employee
{
protected:
    int get_base_salary() { return 0; }
public:
    virtual void compute_salary_and_bonus() = 0;
};

class regular_employee : public employee
{
public:
    virtual void compute_salary_and_bonus()
    {
        salary = get_base_salary();
        bonus = total_profit * bonus_ratio / number_of_employees;
    }
};

class expernal_employee : public employee
{
public:
    virtual void compute_salary_and_bonus()
    {
        salary = get_work_hours() * get_hourly_rate();
        bonus = 0;
    }
};

class manager : public employee
{
public:
    virtual void compute_salary_and_bonus()
    {
        salary = get_base_salary();
        bonus = bonus_fund / number_of_managers;
        if (salary < 100)
            bonus += 100 - salary;
    }
};


Ответ 2

Зависит от того, когда читабельность будет выше. Если у Вас какой-то простой перекодировщик,
то логично вынести в отдельный метод

int charToInt(char a) {
  switch (a) {
    case '0': return 0;
    case '1': return 1;
    case '2': return 2;
    ...........
    default: throw "Error char"
  }
}


(да, я знаю, что можно кастануть к int и вычесть 0x30)

Если же у Вас какой-то парсер

switch (a) {
  case '0':
    ........
    break;
  case '1':
    ........
    break;
  default: throw "Error char"
}


то лучше оставить в одном методе. Тем более, что могут понадобиться дополнительные
переменные, которые используются в этом методе

Наверное, общий совет будет такой - если Вы можете вынести switch в метод, который
принимает ровно один аргумент, а все break можете заменить на return, то выносите


Ответ 3

Зависит от предпочтений. Я например - выношу и даю название CheckSomething  - something
это то, что проверяется :) И не забудте в любом случае default case :)

Рефакторинг большой функции

#cpp #рефакторинг

Дана функция на 200 строк, обрабатывающая большое количество данных, принимающая
много параметров. Путём неимоверных усилий функция была порезана на 4 отдельных функции,
но появилась следующая проблема: у каждой из функций одинаково большой список параметров.
Если я вдруг захочу что-то изменить, то придётся исправлять список параметров во всех
4 функциях, а не в одной + общее решение уже 300 строк. Что можно сделать в такой ситуации?
Какой рефакторинг использовать?

Оригинальная сигнатура (идентична натуральной):

template
auto original_foo(const std::vector& data_1, const std::map&
data_2, int param_1, int param_2, std::pair& indirect_result) {
 //200 long long lines...
}


Моя попытка отрефакторить вырождается в подобные 4 функции:

template
    auto solve_1_task(const std::vector& data_1, const std::map& data_2, int param_1, int param_2, std::pair& indirect_result) {
     //50 lines...
     solve_2_task(data_1, data_2, param_1, param_2, indirect_result);
    }

    

Ответы

Ответ 1

Если вы делаете правильный рефакторинг, то следуя принципу Single Responsibility
при описании методов, количество аргументов редко превышает значения 1, но не следует
упираться в эту рекомендацию выключая логику и нарушая все здравые смыслы.

Я вижу, что обработка данных в каждой функции затрагивает у вас все данные. Для начала,
посмотрите, нельзя ли как-то обрабатывать данные по отдельности? Вы разбили большую
функцию на маленькую, но алгоритм остался тот же, может есть возможность не затрагивать
все данные? Ведь то, что вы сделали не поменяло суть проблемы...

Лечим ваш код:

Переосмысление функций:


Функция, в идеале, должна выполнять только одну операцию. Она должна выполнять ее
хорошо и ничего другого она делать не должна. Чтобы убедиться в том, что функция "выполняет
только одну операцию", необходимо проверить, что все команды функции находятся на одном
уровне абстракции.


Рефакторинг переменных:

Если функция должна получать более двух или трех аргументов, весьма вероятно, что
некоторые из этих аргументов  стоит упаковать в отдельном классе. Рассмотрим следующие
два объявления:

Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius)


Сокращение количества аргументов посредством создания объектов может показаться жульничеством,
но это не так. Если переменные передаются совместно как единое целое (как переменные
x и y в этом примере), то, скорее всего, вместе они образуют концепцию, заслуживающую
собственного имени.

Далее, нужно посмотреть, нельзя ли в самом классе выполнять какие-то действия с переданными
параметрами, чтобы не выполнять их в функции? 

Некоторые правила рефакторинга переменных


Если данные, передаваемые в метод, можно получить путём вызова метода другого объекта,
применяем замену параметра вызовом метода. Этот объект может быть помещён в поле собственного
класса либо передан как параметр метода.
Вместо того чтобы передавать группу данных, полученных из другого объекта в качестве
параметров, в метод можно передать сам объект, используя передачу всего объекта.
Если есть несколько несвязанных элементов данных, иногда их можно объединить в один
объект-параметр, применив замену параметров объектом. Предположим вы передавали start,
end в функцию генерирующую отчет, а теперь просто передавайте обьект DateRange, где
есть параметры start, end.




Способов рефакторинга вашего кода очень много и зависит от самого кода и выполняемой
задачи, тяжело что-то сказать, когда не видишь кода, Вам стоит почитать книжки по рефакторингу
кода, например "Чистый код. Создание, анализ и рефакторинг" Р. Мартина или книги по
шаблонам проектирования, чтобы все правильно разнести на классы.


Ответ 2

Сошлюсь на авторитета :) Но сначала - вы не просто механически поделили функцию на
четыре? Слишком уж на это намекает то, что все параметры нужны во всех функциях...

Ну, а теперь - слово Фаулеру. О длинных списках параметров он пишет:

В длинных списках параметров трудно разобраться. Они зачастую противоречивы и сложны
в применении, а кроме того, их приходится вечно изменять по мере возникновения необходимости
в новых данных. Если же передавать методам объекты, то изменений потребуется меньше,
так как для получения новых данных, скорее всего, хватит пары запросов.

Рефакторинг “Замена параметра вызовом метода” пригоден тогда, когда можно получить
данные в одном параметре путем вызова метода объекта, который уже известен. Этот объект
может быть полем или другим параметром. Рефакторинг “Сохранение всего объекта” позволяет
заменить набор данных, получаемых от объекта, самим этим объектом. Если же имеется
ряд элементов данных без логического объекта, можно воспользоваться рефакторингом “Введение
объекта параметра”.

Есть одно важное исключение, когда такие изменения вносить не следует. Это случай,
когда мы не хотим создавать явную зависимость между вызываемым и более крупным объектами.
В таких случаях разумно распаковать данные и передавать их по отдельности в виде параметров,
но отдавать себе отчет о стоимости этого решения. Если список параметров оказывается
слишком длинным или модификации слишком частыми, лучше пересмотреть структуру зависимостей.


Ответ 3

Почему последовательные вызовы задач образовали цепочку?
Лучше бы было вызывать ваши четыре задачи из оригинальной функции, последовательно
выделяя некоторый код в отдельную функцию:

original_foo(params) {
  solve_1_task(params);
  solve_2_task(params);
  ...
}


По поводу параметров:
Если параметры действительно нужны всюду, лучшим решением, возможно, было бы выделение
этой функциональности в класс.  

Просто передайте все параметры в конструктор класса, и в методах пользуйтесь членами
класса.  

original_foo(params) {
  Foo foo(params);
  foo.solve_1_task();
  ...
}

Термин, когда в процессе рефакторинга поломали код

#терминология #рефакторинг

Как называется рефакторинг в результате которого избавились от некачественного кода
но при этом сломали половину функционала?

Вроде все компилируется и выглядит идеально, но не работает.
    

Ответы

Ответ 1

рефакторинг в результате которого избавились от некачественного кода но при этом
сломали половину функционала


Это называется "регресс" или "регрессия", причём неважно что вы делали: добавляли
новую функциональность и поломали старую или просто рефакторилили без добавления новых
фич и переделки текущих.

Возьмём в качестве примера несколько определений.

Рой Ошеров, книга "Искусство юнит-тестирования":


  Регрессией называется одна или несколько единиц работы, которые
  когда-то работали, а теперь перестали.


International Software Testing Qualifications Board (сертфикационная программа) в
глоссарии даёт определение:


  regression — A degradation in the quality of a component or system due
  to a change.


Есть совокупность мероприятий направленных на то, чтобы постараться диагностировать
подобные вещи — регрессионное тестирование или тестирование на регресс.

Как правило, подобное тестирование вещь настолько сложная, что полное тестирование
на регресс команда Q&A делает в конце итерации.

Рефакторинг в коде, содержащем тесты

#юнит_тесты #рефакторинг

Всем известно, что проводить периодически рефакторинг в коде - полезное, важное и
нужное занятие. Известные авторы в своих книгах пишут, что программист не должен бояться
постоянно изменять свой код. В полезности этого утверждения я убедился уже не раз.
Но вот какой вопрос меня беспокоит: Как вы считаете, нужно ли так же трепетно и внимательно
относиться к чистоте кода в юнит-тестах (или других типах тестов)? Нужно ли, скажем,
выделять общие интерфейсы, или общие части реализации в тестовых классах в абстракции?
Одним словом - избавляться от плохих запахов в тестовом коде.
    

Ответы

Ответ 1

Большинство юнит-тестов - это код, который пишется 1 раз и больше не изменяется.
В идеале, такие все вообще тесты. Код, который никогда не изменяется, рефакторить не нужно.

Более того, рефакторинг тестов зачастую еще и нежелателен - ведь тесты должны быть
независимыми, а выделение "общих частей" сделает их зависимыми друг от друга.

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

Тем не менее, основная задача юнит-тестов - это все-таки экономия времени, а не его
отъем. Если написание каждого нового теста требует кучи повторяющихся действий, или
некоторый тест приходится регулярно переписывать - значит, пора тесты рефакторить.

Еще один допустимый вид рефакторинга тестов - разбиение сложного теста на несколько
простых.


Ответ 2

Юнит-тесты — такой же код, как и весь остальной код в проекте. Если не следить за
чистотой кода, то код превратится в неподдерживаемую лапшу, и это в такой же мере верно
для юнит-тестов.

Скажем, добавился какой-нибудь запрашиваемый через DI интерфейс. Кто-то решил в нескольких
тестах сделать моки под этот интерфейс. После нескольких итераций процесса на месте
короткого теста запросто может оказаться монстр на 50 строк, в котором ничего невозможно
разобрать, который невозможно поддерживать, который ломается от каждого чиха, и который
тормозит разработку (и будем честны: в этот момент тест уже ничего не тестирует по сути).

Если тестируемый код написан нормально, если публичный интерфейс более-менее стабильный
(обратная совместимость не слишком часто ломается), то обычно тесты трогать не надо.

Однако периодически всё-таки стоит заглядывать в код тестов и убеждаться, что с ними
ничего страшного не произошло. Если произошло, то что-то не так или с юнит-тестами,
или с самим тестируемым кодом, и в обязанности программиста входит исправить эти проблемы.

Сложную архитектуру разводить не стоит, тесты должны быть простыми. У тестов свои
стандарты качества: должно быть деление на arrabge-act-assert, не должно быть ветвлений
и т. п. И именно этих стандартов нужно придерживаться, а не приходить с трёхслойными
абстракциями из основного кода.

Как почистить замусоренную боевую бд?

#база_данных #sql_server #рефакторинг #sql

Дано: есть проект, который на боевом сервере живет несколько лет и постоянно дорабатывается
и перерабатывается разными людьми по живому, часто в режиме "дедлайн был вчера".
Там есть база, субд mssql 2008. За время существования проекта в базе скопилось множество
всяческих хранимок, функций и временных таблиц, значительная часть из которых определенно
не используется и никогда не будет: может больше не нужны, может в процессе разработки
создали временную и забыли удалить. 
И когда я смотрю на все эти авгиевы конюшни у меня чешутся руки вычерпать оттуда
ведерко-другое. Однако сразу же встает проблема: как не выплеснуть с водой младенца,
то есть не выпилить случайно что-нибудь лишнее.

Вопрос: есть ли какие-нибудь способы с помощью механизмов встроенных в субд выяснить,
что такая-то хранимая процедура или функция точно вызывалась за последнее время? Были
ли обращения к этой таблице? Может быть, какой-нибудь анализ кешей или что-нибудь в
этом роде?
Как бы Вы решали аналогичную проблему?

Сразу говорю - вопрос скорее теоретический, поэтому замечательный практический совет
"Работает - не трожь" мне не нужен, я итак скорее всего ему последую.
Пока мне в голову приходит:
1). Определить подозрительные процедуры, встроить в них логирование и посмотреть
есть ли что-нибудь в логах через некоторое время. Однако я подозреваю что какой-нибудь
встроенный механизм логирования есть итак.
2). Распарсить серверные исходники на предмет обращения к базе и посмотреть что они
цепляют. Но во-первых, серверные исходники - еще большее болото. Во-вторых, придется
потом парсить результаты первого шага, потому что не все же цепляется напрямую из кода,
некоторое только базой.    

Ответы

Ответ 1

Для начала рекомендую взглянуть сюда: sys.dm_db_index_usage_stats - тут использование
всех индексов с момента последнего перезапуска SQL Server'а. Поскольку любая таблица
- это тоже индекс, с типом HEAP или с типом CLUSTERED, вы сможете получить таблицы,
к которым ни разу не обращались с перезапуска сервера.
Естественно, это не поможет от выпиливания "очень важной и нужной" таблицы, к которой
обращаются раз в год, но без нее никак. Так что лучше, конечно, всех кандидатов на
удаление для начала поискать в коде серверной части.
По хранимым процедурам статистики нет, но можно выгрузить код хранимых процедур,
триггеров и функций, и опять же простым поиском поискать таблицы - кандидаты на удаление.
Если хранимая процедура обращается к таблице, которая ни разу не запрашивалась - есть
подозрение, что хранимая процедура тоже не используется. Такой поиск еще поможет обнаружить
ситуации, когда таблица редко, но используется именно из хранимой процедуры, например,
для сбора информации о сбоях.


Ответ 2

Для начала запустила бы sys.dm_db_index_usage_stats 
После того, как получила неиспользуемые объекты - проверила, какие другие объекты
с ними связаны (right click по имени таблицы,  из меню выбрать View Dependecies), и
затем Profiler-ом отлавливала к ним обращения

Как рефакторить метод со многими вложенными конструкциями в C# или Java

#java #c_sharp #шаблоны_проектирования #рефакторинг

Слышал, что не очень хорошо, когда в методе много вложенных конструкций. Видимо,
так говорят потому, что код становится не читабельным.

Например,

public void Do(int a, bool b, List c, object d = null)
{
    if ()
    {
        foreach ()
        {
            if ()
            {

            }
            else if ()
            {
                if ()
                {
                    foreach ()
                    {
                        if ()
                        {
                        }
                    }
                }   
            }
        }
    }
}


Какие существуют подходы, как такие методы рефакторить в уже написанном коде? Особенно,
в случаях, когда у метода много входных параметров и все они как-то переплетены внутри
него, так что и не догадаешься, как разбить на разные методы. Причем, когда код заново
не получится написать, потому, что всего там не понимаешь, риск, что перестанет правильно
работать. Существуют ли какие-нибудь шаблонные подходы? 

Приходит в голову:


использовать ключевые слова типа return, continue, break


Например, вместо

if (x != null)
{
    //code
}


писать так:

if (x == null)
    return;
//code


Еще чего-нибудь посоветуете?
    

Ответы

Ответ 1

Почитайте книгу Роберта Мартина "Чистый код" (Clean Code).

Не все мысли автора бесспорны, но почитать точно стоит. По крайней мере после чтения,
многие вещи будут восприниматься немного по другому.

Ссылка на книгу в ozon.ru

P.S. Электронная версия легко гуглится.


Ответ 2

Можно выполнить декомпозицию, что бы в каждом отдельном методе выполнялся 1 цикл,
который вызывает другой метод.

В этом случае получается самодокументированный код.

Если в if находится какое-то большое выражение, то можно сделать отдельную функцию,
которая возвращает true/false и опять же получается саомдокументированный код.

В некоторых случаях if можно поменять на switch

В "Чистый код", как раз такие советы и даются.



Еще можно foreach и if заменить на LINQ выражение и несколько строчек сократить до
1. В некоторых случаях(когда условие небольшое) это не скажется на читаемости, а в
итоге несколько строчек сократятся до 1-2.

Исключить дублирование кода в функциях с разной константностью

#cpp #рефакторинг

Рассмотрим такой код:

struct B {};
struct D1 : B {};
struct D2 : B {};

#define get if (s) return d1; else return d2;

volatile bool s;

struct C {
    const B& f() const { get }
    B& f() { get } 
private:
    D1 d1;
    D2 d2;
};


Здесь видно, что разные версии f() должны возвращать один и тот же объект (в одном
случае - константный, в другом - нет), основываясь на некоторой логике выбора, которая
может быть достаточно сложной. В примере для исключения дублирования кода этой логики
использована макроподстановка через #define. 

Можно ли избежать дублирования кода в разных версиях f() не прибегая к услугам препроцессора?
    

Ответы

Ответ 1

Проблема дублирования кода из-за соображений константности обычно встречается в двух
вариантах: 


На уровне отдельных функций: когда есть две функции с одинаковой реализацией, отличающиеся
лишь константностью входных типов (в т.ч., как частный случай, константностью *this
в методе класса) и соответствующей константностью возвращаемого значения. 

В языке С одной из известных идиом для решения этой проблемы является написание одной-единственной
функции, которая решает поставленную задачу в рамках соблюдения константности входных
данных, а затем просто безусловно снимает константность с возвращаемого значения (см.,
например, стандартную функцию strstr). В данном случае предполагается, что вызывающий
код, будучи в курсе ситуации с константностью данных, "по-джентельменски" вернет "потерянную"
в процессе вызова функции константность на место.

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

const return_type *foo(const input_type *argument)
{
  ...
}

return_type *foo(input_type *argument)
{
  return const_cast(foo(const_cast(argument));
}


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

В такой ситуации один из жизнеспособных подходов - реализация общей функциональности
в виде шаблонного класса, параметризованного необходимым количеством типов (в простейшем
случае - одним), и реализация требуемых финальных классов через специализации этого
шаблона. Что-то вроде

template 
class list 
{
  template  class iterator_impl {
    ...
  };

  typedef iterator_impl iterator;
  typedef iterator_impl const_iterator;
  ...
};



P.S. Ваш собственный ответ, использующий шаблонную функцию - это фактически адаптирование
вышеприведенного второго подхода к первой ситуации. Работать, без сомнения, будет,
однако именно в такой ситуации банальный вариант с const_cast, как мне кажется, выглядит
проще и уместнее.


Ответ 2

Можно написать что-то вроде следующего

struct C {
    const B& f() const { if (s) return d1; else return d2; }
    B& f() { return const_cast( const_cast( this )->f() ); } 
private:
    D1 d1;
    D2 d2;
};


Вот демонстрационная программа

#include 

struct B {};
struct D1 : B {};
struct D2 : B {};

bool s;

struct C {
    const B& f() const 
    {
        std::cout << "const B & f() const" << std::endl;
        if (s) return d1; else return d2; 
    }
    B& f() 
    { 
        std::cout << "B & f()" << std::endl;
        return const_cast( const_cast( this )->f() ); 
    } 
private:
    D1 d1;
    D2 d2;
};

int main() 
{
    C c1;
    c1.f();

    std::cout << std::endl;

    const C c2;
    c2.f();

    return 0;
}


Ее вывод на консоль

B & f()
const B & f() const

const B & f() const


Ответ 3

Придумал вариант с шаблонной дружественной функцией:

template
R& g(T* t);

struct C {
    const B& f() const { return g(this); }
    B& f() { return g(this); }
private:
    D1 d1;
    D2 d2;
    template 
    friend R& g(T* t);
};

template
R& g(T* t) {
    if (s) return t->d1; else return t->d2;
}


Или можно вовсе перенести в класс:

struct C {
    const B& f() const;
    B& f();
private:
    D1 d1;
    D2 d2;
    template 
    static R& g(T* t) {
        if (s) return t->d1; else return t->d2;
    }
};

const B& C::f() const { return g(this); }

B& C::f() { return g(this); }


А т.к тип R по сути может быть выведен из факта наличия константности в T, то этот
тип можно вовсе убрать из шаблона:

template 
static std::conditional_t, const B&, B&> g(T* t) {
    if (s) return t->d1; else return t->d2;
}


Т.о. необходимость явно указывать тип при вызове g отпадает:

const B& C::f() const { return g(this); }

B& C::f() { return g(this); }


Ответ 4

Очевидный вариант:

class A {
    bool flag;
    int a;
    int b;

public:
    int& get() { return flag ? a : b; }
    const int& get() const { return ((A*)this)->get(); }
};


Пример:

int main()
{
    A a {};
    const A ca {};
    static_assert(std::is_same::value, "!!");
    static_assert(std::is_same::value, "!!");
}

Ориентиры в определении сроков разработки и рефакторинга веб-проектов

Встала задача поддерживать сайт, который писался много лет и представляет собой лютый говнокод. Ни документации, ни комментариев нет. О форматировании автор не слышал. Зато есть острое желание владельцев сайта понаделать заплаток там, где - на их взгляд - самые проблемные места. В ближайшее время предстоит разговор о сроках работы и мне хотелось бы отталкиваться не от компромиссов (типа такого: я думаю, что у меня уйдет полгода только на минимально необходимую ревизию кода, заказчик думает, что надо все сделать за неделю => договорились на три месяца), а от объективных ориентиров Пример для затравки (цифры условные). Общий объем кода - 50 тыс. строк кода. В день я могу написать 50 строк нормального, покрытого тестами и документированного кода. Отсюда имеем 50000/50 = 1000 трудодней = 200 недель = 3 года 10 месяцев работы одного программиста (без учета отпусков и праздников). Примерно такие объективные ориентиры в оценке времени анализа/рефакторинга/создания веб-приложения хотелось бы услышать. Ну и логика оценки тоже будет крайне интересна.

Ответ

Если ответ интересен чисто теоретически, есть такая метрика как цикломатическая сложность, которую вполне можно измерить. Вполне вероятно есть еще какие-то.
Еще видел статью на хабре с размышлениями на эту тему

Рефакторинг XAML-разметки

Читаю Роберта Мартина и пытаюсь постичь все тонкости рефакторинга.
Если с C#-кодом все более-менее понятно и код мало-помалу начинает радовать глаз, то с XAML всё печально. Разметка громоздкая и трудночитаемая. Понятно, что многое связано с xml-"наследственностью". Xml избыточен и не очень приятен для глаз, но всё же. Какие есть способы и кто что применяет для рефакторинга xaml?
В идеале хотелось бы избавиться от десятиуровневой иерархии и получить короткие "методы" по 5-7 строчек (как это у меня сделано в c#-коде).
Привожу разметку одного из окон моего последнего WPF-приложения (все 320 строчек "фарша"):

Как избавиться от каскада switch case

Есть викторина. В ней пользователь угадывает один из 3-х вариантов. Каждый вариант имеет свой вес:
Вариант 1: 20 очков Вариант 2: 30 очков Вариант 3: 50 очков
Если игрок отгадывает вариант, то получает весь вес. Если нет, то только его часть, распределение которой строго регламентировано.
Ответ игрока || Правильный ответ || Очки 1 1 100% 2 1 75% 3 1 50% 1 2 75% 2 2 100% 3 2 75% 1 3 25% 2 3 50% 3 3 100%
Можно запросто написать что-нибудь в духе:
calcRateScore: function(fact, user) {
var rateScore = 0;
switch (fact) { case 1: switch (user) { case 1: rateScore = 20; break; case 2: rateScore = 20 * 0.75; break; case 3: rateScore = 20 * 0.5; break; } break; case 2: switch (user) { case 1: rateScore = 30 * 0.75; break; case 2: rateScore = 30; break; case 3: rateScore = 30 * 0.75; break; } break; case 3: switch (user) { case 1: rateScore = 50 * 0.25; break; case 2: rateScore = 50 * 0.5; break; case 3: rateScore = 50; break; } break; }
return rateScore; }
Но выглядит это просто ужасно. Подскажите, как избавиться от этих switch / case (желательно от всех)?

Ответ

var answers = [ null, { weight: 20, fractions: [0, 1, 0.75, 0.5] }, { weight: 30, fractions: [0, 0.75, 1, 0.75] }, { weight: 50, fractions: [0, 0.25, 0.5, 1] } ];
calcRateScore: function(fact, user) {
var rateScore = 0;
if (answers[fact] && answers[fact].fractions[user]) { rateScore = answers[fact].weight * answers[fact].fractions[user]; }
return rateScore; }

Создание переменной ссылочного типа при каждой итерации цикла. Это затратно?

Есть вот такой код:
... Match match; foreach (var value in egeQuestionValues) { match = Regex.Match(value, $@"(?<={number}\()\d+(?=.*%\))"); if (!match.Success) throw new ArgumentException($"The record {value} is incorrect", nameof(egeQuestionValues));
egeQuestionIntValues.Add(int.Parse(match.Value)); }
return egeQuestionIntValues.Average();
Я создал переменную match специально вне цикла foreach, чтобы в рантайме не создать ее при каждой итерации. Делал, думаю что это затратно. Но ReSharper мне советует отрефакторить этот код таким образом:
... foreach (var value in egeQuestionValues) { var match = Regex.Match(value, $@"(?<={number}\()\d+(?=.*%\))"); if (!match.Success) throw new ArgumentException($"The record {value} is incorrect", nameof(egeQuestionValues));
egeQuestionIntValues.Add(int.Parse(match.Value)); }
return egeQuestionIntValues.Average();
т.е. сейчас при каждой итерации переменная match будет создаваться каждый раз. Разве это не затратно? ReSharper советует поступать так не только с типом Match и с другими типами так было.

Ответ

Разницы особой нету.
Все переменные ссылочного типа без инициализации автоматически инициализируются null-значением.
Что сама по себе переменная ссылочного типа? Это переменная, которая ссылается на какой-то адрес.
Смена ссылки - это быстрая операция, так как не создается никаких дополнительных объектов, как это происходит со значимым типом.
Скорее всего, компилятор приведет код к первому случаю и IL-код будет идентичным.
На мой взгляд овчинка выделки не стоит и это не узкое место, ведь вы программируете на высокоуровневом языке программирования, где на первое место нужно ставить понятность кода, а уж потом если где-то что-то тормозит, то оптимизировать.
Второй вариант более приятен, так как переменная находится прямо в месте ее использования и можно сразу понять, что за циклом она нигде использоваться не будет.