Russian Qt Forum
Ноябрь 22, 2024, 16:43 *
Добро пожаловать, Гость. Пожалуйста, войдите или зарегистрируйтесь.
Вам не пришло письмо с кодом активации?

Войти
 
  Начало   Форум  WIKI (Вики)FAQ Помощь Поиск Войти Регистрация  

Голосование
Вопрос: Какое впечатление на Вас производит этот фрагмент кода
Ужасное - 4 (33.3%)
Примитивное "С с классами" - 3 (25%)
Нормальный рабочий код - 3 (25%)
Круто - 0 (0%)
Ваш вариант - 2 (16.7%)
Всего голосов: 11

Страниц: [1] 2   Вниз
  Печать  
Автор Тема: Впечатления от фрагмента кода  (Прочитано 12117 раз)
Igors
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 11445


Просмотр профиля
« : Август 04, 2016, 11:05 »

Добрый день

Вот работаю с библиотекой, приходится вникать/разбираться
Код
C++ (Qt)
void SkMotion::apply ( float t,  
float* buffer, SrBuffer<int>* map_p,
InterpType itype, int* lastframe, bool isAdditive, SBRetarget* retarget )
{
// new version of apply skMotion
int fsize=_frames.size();
if ( fsize<=0 )
return;
if ( t!= 0.f && t<=_frames[0].keytime ) {
#if DEBUG_T
LOG("SkMotion::apply NOTICE: t=%.16f < f[0]:%.16f \n", t, _frames[0].keytime );
#endif
return;
}
 
if ( itype==CubicSpline )
t = _cubic ( t, _frames[0].keytime, _frames[_frames.size() - 1].keytime );
 
#if DEBUG_T
if ( t<_frames[0].keytime ) {
LOG("SkMotion::apply ERR: cubic t=%.16f < f[0]:%.16f \n", t, _frames[0].keytime );
}
#endif
 
// optimize keytime search for sequenced calls to apply
int fini=0;
if ( lastframe )
_last_apply_frame = *lastframe;
if ( _last_apply_frame>0 && _last_apply_frame<fsize ) {
if ( t>_frames[_last_apply_frame].keytime )
fini=_last_apply_frame+1;
}
 
int f;
for ( f=fini; f<fsize; f++ ) {
if ( t<_frames[f].keytime )
break;
}
 
if ( f==_frames.size() ) {
// Apply last frame
//apply_frame( f-1, buffer, map_p );
//return;
t = _frames[_frames.size()-1].keytime;
f = _frames.size()-1;
}
 
if (_frames.size() > 1)
{
f--;
}
_last_apply_frame = f;
if ( lastframe )
*lastframe = _last_apply_frame;
int nxtFrame = _frames.size() > 1 ? f+1 : f;
 
float* fp1 = _frames[f].posture;
float* fp2 = _frames[nxtFrame].posture;
// convert t to [0,1] according to the keytimes:
t = _frames.size() > 1 ? (t-_frames[f].keytime) / (_frames[f+1].keytime-_frames[f].keytime) : 0.f;
 
#if DEBUG_T
if ( t<0.0 ) {
LOG("SkMotion::apply ERR: mapped t=%.16f < 0.0 \n", t );
}
#endif
 
//sr_out<<"t: "<<t<<" frames: "<<f<<srspc<<(f+1)<<"\n";
int num;
int csize = _channels.size();
 
 
float outValue[4], origValue[4], newValue[4]; // at most 4
// Apply to float* buffer
float* v = NULL;
int sum = 0;
for ( int i=0; i<csize; i++ ) {
// Channel size
num = _channels[i].size();
// Find mapped buffer location
int index = i;
 
if (buffer)
{
if (map_p)
{
index = map_p->get( i );
v = buffer + index;
}
else
{
v = buffer + sum;
}
for (int x=0;x<num;x++)
origValue[x] = v[x];
}
else if (_channels[i].joint)
{
_channels[i].get(origValue);
}
else
{
index = -1;
}
 
_channels[i].interp ( newValue, fp1, fp2, t );
if( index >= 0 ) {
if (_channels[i].type == SkChannel::Quat) // rotation channel
{
SrQuat orig(origValue[0], origValue[1], origValue[2], origValue[3]);
SrQuat add(newValue[0], newValue[1], newValue[2], newValue[3]);
SrQuat final = isAdditive ? orig*add : add;
if (retarget)
final = retarget->applyRetargetJointRotation(_channels.mappedName(i),final);
for (int x = 0; x < 4; x++)
outValue[x] = final.getData(x);
}
else if (num <= 3) // positional channel
{
for (int x = 0; x < num; x++)
{
outValue[x] = isAdditive? origValue[x]+newValue[x] : newValue[x];
if (retarget)
outValue[x] = retarget->applyRetargetJointTranslation(_channels.mappedName(i),outValue[x]);
}
}
else
{
// more that 3 channels but not a quaternion? Don't add data, just replace it...
for (int x = 0; x < num; x++)
{
outValue[x] = newValue[x];
if (retarget)
outValue[x] = retarget->applyRetargetJointTranslation(_channels.mappedName(i),outValue[x]);
}
}
if (buffer)
{
for (int x = 0; x < num; x++)
v[x] = outValue[x];
}
else
{
_channels[i].set(outValue);
}
}
// Increment frame data pointers
fp1+=num; fp2+=num; sum+=num;
}
}
 
Нет, конечно а проекте есть классы, виртуалы, std, и даже немного темплейтов и буста. Но все это как-то "сбоку-припеку", основная/подавляющая масса кода как выше, этим делается содержательная часть. И вот как-то здесь я не вижу ну ровным счетом ничего что часто считается "грамотностью" Улыбающийся

Может как-то "модернизировать"? Ну и что я должен делать - насильно втыкать std/boost (как Хрущев кукурузу)? А смысл? В общем
Цитировать
2 мира - 2 детства
Записан
Bepec
Гость
« Ответ #1 : Август 04, 2016, 11:17 »

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

А по вопросу - нормальный код, но явно прослеживает си стайл в работе с переменными и малая информативность названий типо f, t, fsize, csize, v.
Так же часть кода завязана, видимо, на поля класса.
Поэтому чтобы улучшить, оптимизировать, сделать более грамотнее - нужно вообще понять что эта фигня делает в комплексе, как применяется,  в чём неудобство. Хотя - да что я вам рассказываю, вы и сами знаете. Нужно сначала изучить, потом улучшать.
Записан
ssoft
Программист
*****
Offline Offline

Сообщений: 584


Просмотр профиля
« Ответ #2 : Август 04, 2016, 14:04 »

Если код работает, то лучше его не трогать.
Если не нравится способы вызовов методов или набор сущностей, то лучше написать адаптер/обертку - свой API поверх чужой библиотеки.
Если просто сам код не нравится, тогда придется все переписывать самостоятельно В замешательстве, как показывает практика - практически с нуля.
Записан
Igors
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 11445


Просмотр профиля
« Ответ #3 : Август 04, 2016, 14:47 »

Хотя - да что я вам рассказываю, вы и сами знаете.
Первый (скромный) успех - Ваш пост содержит 1 содержательную фразу  Улыбающийся

Если код работает, то лучше его не трогать.
Неверно работает если additive = true. Хотя возможно я не до конца разобрался

Если не нравится способы вызовов методов или набор сущностей, то лучше написать адаптер/обертку - свой API поверх чужой библиотеки.
Там .a файл (компилю статычно) под 400 метров (debug) - это ж сколько "оборачивать" надо?

Если просто сам код не нравится, тогда придется все переписывать самостоятельно В замешательстве, как показывает практика - практически с нуля.
При таком объеме нереально. Нравится или нет - ну от стиля/форматирования написания я не в восторге. А по существу - по-моему вполне норм, разобраться и понять можно. В связи с этим хотелось бы спросить у "воротящих носик" - а что, собсно, так уж плохо? Нет "чего-то модного/крутого"? Так чего? И нужно ли оно здесь?
Записан
Bepec
Гость
« Ответ #4 : Август 04, 2016, 15:32 »

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

Не трогайте чужой код. Исправьте ошибку, а всё остальное оставьте. Не в ваших силах изменить мир Веселый
Записан
ssoft
Программист
*****
Offline Offline

Сообщений: 584


Просмотр профиля
« Ответ #5 : Август 04, 2016, 18:02 »

Там .a файл (компилю статычно) под 400 метров (debug) - это ж сколько "оборачивать" надо?
Оборачивать нужно только то, что необходимо в использовании, в понятные для себя методы. Глядишь потом и собственную реализацию вместо этой подставить).
Стиль и форматирование - для этого есть Beautifying Source Code http://doc.qt.io/qtcreator/creator-beautifier.html и инструменты для рефакторинга. Я частенько использую для анализа кода, даже если только для себя).
Записан
Igors
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 11445


Просмотр профиля
« Ответ #6 : Август 09, 2016, 11:16 »

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

В принципе этот кусочек кода как бы "окно в параллельный мир" программирования. Конечно правила С++ и там роялят, но совсем иначе. От владения std, boost, и.т.д. здесь толку немного, даже скромных зачаточных знаний хватит. Ну а тонна заученных Qt классов вообще бесполезна. Решающее значение имеют алгоритмы и знание предметной части, при этом решаемые задачи могут быть отнюдь не просты.

И такая ситуация всякий раз когда связываюсь с приличной либой. Напр скоро придется опять лезть в FBX - либа совершенно другого плана, но и там "запас популярных знаний" вообще-то бесполезен  Улыбающийся
Записан
Racheengel
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2679


Я работал с дискетам 5.25 :(


Просмотр профиля
« Ответ #7 : Август 09, 2016, 12:49 »

Чисто внешне код страшноват, смесь С и С++.
Названия функций странные - interp(), а рядом applyRetargetJointRotation().
Переменные типа _frames и т.д. тоже моветон, имхо.
Дуст тут, конечно, не нужен.
Оно выглядит достаточно некрасиво и без дуста:)
Записан

What is the 11 in the C++11? It’s the number of feet they glued to C++ trying to obtain a better octopus.

COVID не волк, в лес не уйдёт
Igors
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 11445


Просмотр профиля
« Ответ #8 : Август 09, 2016, 16:13 »

Чисто внешне код страшноват, смесь С и С++.
Названия функций странные - interp(), а рядом applyRetargetJointRotation().
Переменные типа _frames и т.д. тоже моветон, имхо.
Дуст тут, конечно, не нужен.
Оно выглядит достаточно некрасиво и без дуста:)
Ну вообще-то стандартное название линейной интерполяции "lerp" (никак не лучше). С подчеркиванием у него имена членов класса. Я тоже не в восторге от таких имен, но они вполне в рамках допустимого, красиво-некрасиво = субъективное мнение.

Гораздо более "пугающим" выглядит то что код не вертится вокруг каких-то буквариных классов, а делает все сам  Улыбающийся

Записан
Racheengel
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2679


Я работал с дискетам 5.25 :(


Просмотр профиля
« Ответ #9 : Август 09, 2016, 16:41 »

Гораздо более "пугающим" выглядит то что код не вертится вокруг каких-то буквариных классов, а делает все сам  Улыбающийся

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

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

What is the 11 in the C++11? It’s the number of feet they glued to C++ trying to obtain a better octopus.

COVID не волк, в лес не уйдёт
kambala
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 4747



Просмотр профиля WWW
« Ответ #10 : Август 09, 2016, 17:04 »

а что плохого в членах класса, начинающихся с _ ?
Записан

Изучением C++ вымощена дорога в Qt.

UTF-8 has been around since 1993 and Unicode 2.0 since 1996; if you have created any 8-bit character content since 1996 in anything other than UTF-8, then I hate you. © Matt Gallagher
Igors
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 11445


Просмотр профиля
« Ответ #11 : Август 09, 2016, 17:18 »

а что плохого в членах класса, начинающихся с _ ?
Да собсно ничего, дело вкуса

С другой стороны, возможно, это уникальный алгоритм, который нигде ранее не встречался Улыбающийся
А что он делает? Что можно понять по этому кусочку? Конечно разобраться полностью - нужно куда больше исходников и времени, я об этом и не спрашиваю. А что можно сказать "навскидку"?
Записан
Racheengel
Джедай : наставник для всех
*******
Offline Offline

Сообщений: 2679


Я работал с дискетам 5.25 :(


Просмотр профиля
« Ответ #12 : Август 09, 2016, 19:20 »

А что он делает? Что можно понять по этому кусочку? Конечно разобраться полностью - нужно куда больше исходников и времени, я об этом и не спрашиваю. А что можно сказать "навскидку"?

А хз что он делает. Похож на студенческий говнокод из лабы первокурсника.

Автор не понимает, что такое локальные параметры (передается float t в качестве аргумента, потом этот же t меняется по коду - фууу...)

Автор не знает о существовании оператора switch (проверки типов через if).

В общем, я бы не стал его использовать.
Записан

What is the 11 in the C++11? It’s the number of feet they glued to C++ trying to obtain a better octopus.

COVID не волк, в лес не уйдёт
_Bers
Бывалый
*****
Offline Offline

Сообщений: 486


Просмотр профиля
« Ответ #13 : Август 09, 2016, 23:18 »

а что плохого в членах класса, начинающихся с _ ?

это - UB.
Записан
Bepec
Гость
« Ответ #14 : Август 09, 2016, 23:20 »

Почему?
Записан
Страниц: [1] 2   Вверх
  Печать  
 
Перейти в:  


Страница сгенерирована за 0.099 секунд. Запросов: 25.