Russian Qt Forum

Программирование => Общий => Тема начата: Igors от Июнь 23, 2019, 14:09



Название: Рефакторим древний код
Отправлено: Igors от Июнь 23, 2019, 14:09
Добрый день

Есть такой "кал мамонта" начала 90-x (псевдокод)
Код
C++ (Qt)
void AddData2Undo( Object * obj,   // объект
                       int dataID,      // id сохраняемых данных
                       int dataSize,   // размер
                       const void * data ) // указатель на данные
{
 Stream & stream = GetUndoStream();  
 stream.Write(obj->GetUniqueID());
 stream.Write(dataID);
 stream.Write(dataSize);
 stream.WriteBlock((char *) data, dataSize);
}  
Пример использования
Код
C++ (Qt)
AddData2Undo(theObject, UNDOPART_COLOR, sizeof(theObject->mColor), &theObject->mColor);

Ф-ция вызывается ровно 500 раз. Нужно ли ее переделывать? Если да то как ?

Спасибо


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июнь 23, 2019, 17:32
Мне кажется, именно на эту функцию может и можно освежителем пшикнуть, но погоды это не сделает, т.к. наверняка в мамонте есть более зловонные места.

Можно написать шаблонную перегрузку, зашаблонить параметр data, чтоб размер сам считался, и вызывалась исходная AddData2Undo. Можно будет вызывать её с одним аргументом меньше. obj передавать по константной ссылке.


Название: Re: Рефакторим древний код
Отправлено: Igors от Июнь 24, 2019, 07:11
Мне кажется, именно на эту функцию может и можно освежителем пшикнуть, но погоды это не сделает, т.к. наверняка в мамонте есть более зловонные места.
Конечно есть, сколько угодно. Да, погоды не сделает, но на кратности 500 даже экономия на sizeof имеет смысл.

Можно написать шаблонную перегрузку, зашаблонить параметр data, чтоб размер сам считался, и вызывалась исходная AddData2Undo. Можно будет вызывать её с одним аргументом меньше. obj передавать по константной ссылке.
Что-то типа такого (если не так понял - поправьте)
Код
C++ (Qt)
template<class T>
void AddData2Undo( Object * obj,  int dataID, const T & data )
{
AddData2Undo_Base(obj,  dataID, sizeof(T), &data);
}
 
Аргумент сэкономили - хорошо. Правда не всегда, напр есть такие вызовы
Код:
std::vector<int> vec;
...
AddData2Undo(obj, UNDOPART_INDICES, vec.size() * sizeof(int), &vec[0]);
Может стоит задаться вопросом "А что здесь плохо и что хотим улучшить?". Или все и так хорошо?  :) Думаю плохо то что контроля никакого, не раз уже получал по интерфейсу
Код
C++ (Qt)
struct SomeData {
 int m_id;
 float m_width;
 ...
 QString m_name;  // этого члена раньше не было, но вот понадобилось добавить
};
И выцарапывай потом эти undo...  :'(


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июнь 24, 2019, 13:07
Что-то типа такого (если не так понял - поправьте)

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

Код
C++ (Qt)
template<class T>
void AddData2Undo( const Object & obj,  int dataID, const T & data )
{
 static_assert(!std::is_pointer_v<T>);
 
AddData2Undo(std::addressof(obj),  dataID, sizeof(data), std::addressof(data));
}

Может стоит задаться вопросом "А что здесь плохо и что хотим улучшить?". Или все и так хорошо?  :) Думаю плохо то что контроля никакого, не раз уже получал по интерфейсу
...
И выцарапывай потом эти undo...  :'(

Тут уже надо смотреть на текущие достижения в области сериализации объектов в С++ и возможность применения их в данном проекте :).


Название: Re: Рефакторим древний код
Отправлено: Igors от Июнь 24, 2019, 16:30
Тут уже надо смотреть на текущие достижения в области сериализации объектов в С++ и возможность применения их в данном проекте :).
С сериализацией что-то общее есть, но не более того. Иногда удается записывать в undo весь объект (т.е. просто использовать код сериализации) но чаще нет.

...только проверок добавить, чтобы всякая чушь не проходила.
На первый взгляд шикарно так
Код
C++ (Qt)
template<class T>
void AddData2Undo( const Object & obj,  int dataID, const T & data )
{
 static_assert(std::is_pod<T>::value);
Но увы. Во-первых POD ничего не гарантирует, напр может быть член-указатель. А главное - есть масса "простых" классов которые не POD но успешно пишутся/читаются "блоком". Напр QRect, QPoint, QVector3D - не именно они, но такого же плана. Заметим что проблемы выравнивания и ендианы здесь не волнуют т.к. undo работает в рамках одного запуска/сессии.

Поэтому хотелось бы как-то (удобно) перечислить эти классы и, если ни один из них не найден - тогда уже is_pod, Как это сделать? Разумеется хотим ловить га этапе компиляции


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июнь 24, 2019, 18:42
Во-первых POD ничего не гарантирует, напр может быть член-указатель. А главное - есть масса "простых" классов которые не POD но успешно пишутся/читаются "блоком". Напр QRect, QPoint, QVector3D - не именно они, но такого же плана. Заметим что проблемы выравнивания и ендианы здесь не волнуют т.к. undo работает в рамках одного запуска/сессии.

Можно попробовать проверять на std::is_standard_layout (к тому же std::is_pod чего-то "deprecated in C++20"). В общем случае это не хорошо, так лазить в приватные данные, как говорится "на свой страх и риск".

Поэтому хотелось бы как-то (удобно) перечислить эти классы и, если ни один из них не найден - тогда уже is_pod, Как это сделать? Разумеется хотим ловить га этапе компиляции

Организовать принадлежность типа к какой-то проверке/категории, можно например так:
Код
C++ (Qt)
template <class T>
struct is_block_streamable : public std::false_type {};
// Helper variable template.
template <class T>
inline constexpr bool is_block_streamable_v = is_block_streamable<T>::value;
 
template <>
struct is_block_streamable<QRect> : public std::true_type {};
template <>
struct is_block_streamable<QVector3D> : public std::true_type {};


Название: Re: Рефакторим древний код
Отправлено: Igors от Июнь 25, 2019, 13:45
Можно попробовать проверять на std::is_standard_layout (к тому же std::is_pod чего-то "deprecated in C++20"). В общем случае это не хорошо, так лазить в приватные данные, как говорится "на свой страх и риск".
Тогда может приятнее std::is_trivially_copyable ?

Организовать принадлежность типа к какой-то проверке/категории, можно например так:
Ну это "по Александреску", а может как-то половчее с пресловутыми "variadic"? 


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июнь 26, 2019, 14:33
Тогда может приятнее std::is_trivially_copyable ?
Может и приятнее.

Ну это "по Александреску", а может как-то половчее с пресловутыми "variadic"?

Variadic несколько в ином контексте работает. Если есть возможность перечислить все нужные типы в одном месте, то можно объявить большой tuple с ними, а потом проверять тип на вхождение в этот tuple. Если же хотите в разных местах программы добавлять типы в is_block_streamable по несколько штук списком, то я сейчас не могу сказать как это можно сделать простым способом. Но может кто другой знает :).


Название: Re: Рефакторим древний код
Отправлено: Igors от Июнь 27, 2019, 15:25
"Попробывал" с is_trivially_copyable. Из 500 вызовов осталось 12 оригинальных (для сохранения векторов и.т.п.), остальные "легли". Нашел 2 бага. В общем - неплохо


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июнь 28, 2019, 14:40
А если std::is_standard_layout попробовать, большая разница будет?


Название: Re: Рефакторим древний код
Отправлено: Igors от Июнь 30, 2019, 07:11
А если std::is_standard_layout попробовать, большая разница будет?
Со сделанными изменениями разницы никакой, с std::is_standard_layout тоже компилится. Не нашел способа отловить такую простецкую ситуевину
Код
C++ (Qt)
struct A {
..
 int * b;
...
};
Заманчивое std::is_member_pointer делает совсем не то  :'(


Название: Re: Рефакторим древний код
Отправлено: ssoft от Июль 01, 2019, 09:07
Здесь нужно анализировать has_member_pointer).
Можно воспользоваться подходом описанным здесь https://habr.com/ru/post/344206/.


Название: Re: Рефакторим древний код
Отправлено: Igors от Июль 01, 2019, 10:40
Здесь нужно анализировать has_member_pointer).
Можно воспользоваться подходом описанным здесь https://habr.com/ru/post/344206/.
"Мракобесие" (как говорили в советские времена)  :)


Название: Re: Рефакторим древний код
Отправлено: ViTech от Июль 01, 2019, 12:14
"Мракобесие" (как говорили в советские времена)  :)

Низменные внутренности редко бывают красивыми :). Чтобы исследовать поля структуры нужна рефлексия, которая ещё не скоро появится. Из ближайшего будущего может что полезное найдётся в концептах (https://en.cppreference.com/w/cpp/language/constraints). Хотите "здесь и сейчас" - придётся идти путём Александреску :).