![]() |
Здравствуйте, гость ( Вход | Регистрация )
![]() |
AD |
![]()
Сообщение
#1
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
Собственно предлагаю сюда задавать и выкладывать куски кода (с пояснениями, конечно, что этот код делает) для того, чтобы помогли его ну как упростить, сделать более читабельным. Возможно, кому-то это поможет.
Если разрешите, то могу начать. |
|
|
![]() |
void* |
![]()
Сообщение
#2
|
![]() Программист-самоучка ![]() ![]() ![]() Группа: Участник Сообщений: 429 Регистрация: 4.6.2008 Пользователь №: 193 Спасибо сказали: 28 раз(а) Репутация: ![]() ![]() ![]() |
ну вот у меня такая дилема. Нижеприведенный код показывает в дереве (QTreeWidget'e) содержимое определенной папки методом рекурсии, однако состоит это дело из двух функций, и мне почему-то кажется что можно это как-то сделать и через одну функцию:
код
|
|
|
ViGOur |
![]()
Сообщение
#3
|
![]() Мастер ![]() ![]() ![]() ![]() ![]() ![]() Группа: Модератор Сообщений: 3296 Регистрация: 9.10.2007 Из: Москва Пользователь №: 4 Спасибо сказали: 231 раз(а) Репутация: ![]() ![]() ![]() |
Ну я дуаю ты сам мог бы догадаться, если бы был внимательней.
Не нужно два цикла, вместо entryList используй entryInfoList в и foreach получай QFileInfo с помощью которого ты можешь узнать файл это или директория (isFile или isDir). Ну а как потянешь за эту ниточку, остальное думаю наложится, если не будет понятно что-то спрашивай. p.s. как сделаешь код здесь выложи, чтобы покритиковали если будет что. ![]() |
|
|
void* |
![]()
Сообщение
#4
|
![]() Программист-самоучка ![]() ![]() ![]() Группа: Участник Сообщений: 429 Регистрация: 4.6.2008 Пользователь №: 193 Спасибо сказали: 28 раз(а) Репутация: ![]() ![]() ![]() |
ViGOur, насчет entryInfoList я знал, просто мне казалось что так нагляднее. А так - пока что повозиться с этим кодом нет времени и свободных рук
![]() |
|
|
Red Devil |
![]()
Сообщение
#5
|
![]() Студент ![]() Группа: Участник Сообщений: 68 Регистрация: 6.6.2008 Из: Saint-Petersburg Пользователь №: 194 Спасибо сказали: 1 раз(а) Репутация: ![]() ![]() ![]() |
Сообщение отредактировал Red Devil - 6.8.2008, 12:01 |
|
|
void* |
![]()
Сообщение
#6
|
![]() Программист-самоучка ![]() ![]() ![]() Группа: Участник Сообщений: 429 Регистрация: 4.6.2008 Пользователь №: 193 Спасибо сказали: 28 раз(а) Репутация: ![]() ![]() ![]() |
упростил однако
![]() ![]() хотя так оно конечно возможно понятнее |
|
|
ViGOur |
![]()
Сообщение
#7
|
![]() Мастер ![]() ![]() ![]() ![]() ![]() ![]() Группа: Модератор Сообщений: 3296 Регистрация: 9.10.2007 Из: Москва Пользователь №: 4 Спасибо сказали: 231 раз(а) Репутация: ![]() ![]() ![]() |
Вызывается так:
|
|
|
void* |
![]()
Сообщение
#8
|
![]() Программист-самоучка ![]() ![]() ![]() Группа: Участник Сообщений: 429 Регистрация: 4.6.2008 Пользователь №: 193 Спасибо сказали: 28 раз(а) Репутация: ![]() ![]() ![]() |
ViGOur, спасибо! примерно такое у меня и вертелось в голове, только никак не мог выразить это в коде
![]() |
|
|
Tonal |
![]()
Сообщение
#9
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
2 ViGOur
Я бы всё таки выделил из этого кода функции создания итема для файла и для директории. Тогда код получается максимально наглядный и все занимаются своим делом:
Сообщение отредактировал Tonal - 7.8.2008, 8:12 |
|
|
ViGOur |
![]()
Сообщение
#10
|
![]() Мастер ![]() ![]() ![]() ![]() ![]() ![]() Группа: Модератор Сообщений: 3296 Регистрация: 9.10.2007 Из: Москва Пользователь №: 4 Спасибо сказали: 231 раз(а) Репутация: ![]() ![]() ![]() |
|
|
|
AD |
![]()
Сообщение
#11
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
Выдалась возможность, может сможете и мне упростить функцию?
Вот ее код:
Для того, чтобы было чуть понятнее привел кусочек класса, где определена эта функция. Ну что можно сказать: функция выполняет расчет и добавление в вектор значений в зависимости от значения GAPTYPE! Я подумывал о том, чтобы вынести расчет для каждого case в отдельную функцию, но посчитал количество параметров этой функции и стало грустно! Может сможете улучшить данный код? Просьба приводить пример улучшения кода. Заранее спасибо! |
|
|
Tonal |
![]()
Сообщение
#12
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
Вынеси длинные проверки с next -> status... в отдельные функции - сразу жить станет легче, и станит видно что делать дальше.
![]() |
|
|
AD |
![]()
Сообщение
#13
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
Вынеси длинные проверки с next -> status... в отдельные функции - сразу жить станет легче, и станит видно что делать дальше. ![]() Если честно, наглядность не сильно увеличилась: Исходник
|
|
|
Tonal |
![]()
Сообщение
#14
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
Без тега кода всяко только уменьшилась!
![]() По коду:
Вторую аналогично. После проверок где isContinue = false и checkStatusBreak сразу выходить. Проверку на init_gaps в начало функции. и тоже по false сразу выходить. Изменение, которое после checkStatusDist тоже в 2 отдельные функции. Ну и из (time_t)fabs(double(next -> time_marker - cur -> time_marker)) тоже функцию. |
|
|
AD |
![]()
Сообщение
#15
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
Tonal, я не понял как и что. Можно все-таки кодом, пожалуйста!
|
|
|
AD |
![]()
Сообщение
#16
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
А мне помогут упростить код?
|
|
|
Tonal |
![]()
Сообщение
#17
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
Чё непонятно-то?
|
|
|
AD |
![]()
Сообщение
#18
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
|
|
|
Tonal |
![]()
Сообщение
#19
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
Цитата Дай - понимаю, курить - понимаю, дай курить - не понимаю! Какое предложение объяснить? Да, и функции, вместо checkStatus* я бы назвал isStatus* - во первых сразу понятно что они возвращают и зачем, во вторых соответствует стилю Qt. ![]() |
|
|
AD |
![]()
Сообщение
#20
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
После проверок где isContinue = false и checkStatusBreak сразу выходить. Проверку на init_gaps в начало функции. и тоже по false сразу выходить. Изменение, которое после checkStatusDist тоже в 2 отдельные функции. Ну и из (time_t)fabs(double(next -> time_marker - cur -> time_marker)) тоже функцию. Что сделать надо, что именно и куда вынести? |
|
|
Tonal |
![]()
Сообщение
#21
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 452 Регистрация: 6.12.2007 Из: Новосибирск Пользователь №: 34 Спасибо сказали: 69 раз(а) Репутация: ![]() ![]() ![]() |
1) После проверок где...
Код
заменить на
2) Проверку на.. Код
заменить на
3) Изменение, которое после checkStatusDist... У тебя 3 ветки. Если смотреть на код этих веток после if(isContinue && init_gaps)... то ясно что третья ветка - это объединение первых 2. Стало быть мы выносим этот код из 1ий и 2ой ветви в отдельные функции а в третьей ветви вызываем обе этих функции. 4) Ну и из... Из кода (time_t)fabs(double(next -> time_marker - cur -> time_marker)); Делаем функцию timeDist 5) Функции checkStatus* переименовываем в isStatus*. |
|
|
AD |
![]()
Сообщение
#22
|
Профессионал ![]() ![]() ![]() ![]() ![]() Группа: Участник Сообщений: 2003 Регистрация: 4.2.2008 Из: S-Petersburg Пользователь №: 84 Спасибо сказали: 70 раз(а) Репутация: ![]() ![]() ![]() |
Теперь это выглядит более внятно и легче править. Спасибо!
Source
Сообщение отредактировал AD - 11.8.2008, 15:06 |
|
|
Red Devil |
![]()
Сообщение
#23
|
![]() Студент ![]() Группа: Участник Сообщений: 68 Регистрация: 6.6.2008 Из: Saint-Petersburg Пользователь №: 194 Спасибо сказали: 1 раз(а) Репутация: ![]() ![]() ![]() |
Код
Причина редактирования: обернул в тэг expand
|
|
|
rich |
![]()
Сообщение
#24
|
![]() Участник ![]() ![]() Группа: Участник Сообщений: 123 Регистрация: 1.3.2008 Пользователь №: 109 Спасибо сказали: 6 раз(а) Репутация: ![]() ![]() ![]() |
Может кому поможет:
"Рефакторинг. Улучшение существующего кода" Мартин Фаулер. заказ книг ещё заказ книг и ещё заказ книг |
|
|
Admin |
![]()
Сообщение
#25
|
Администратор ![]() ![]() ![]() ![]() Группа: Администратор Сообщений: 646 Регистрация: 9.10.2007 Из: crossplatform.ru Пользователь №: 1 Спасибо сказали: 17 раз(а) Репутация: ![]() ![]() ![]() |
rich, а ты темой не ошибся? Причем тут упрощение кода?
|
|
|
alex977 |
![]()
Сообщение
#26
|
![]() Активный участник ![]() ![]() ![]() Группа: Участник Сообщений: 310 Регистрация: 19.6.2008 Из: Россия, МО, г.Мытищи Пользователь №: 206 Спасибо сказали: 77 раз(а) Репутация: ![]() ![]() ![]() |
rich, а ты темой не ошибся? Причем тут упрощение кода? ИМХО, имеет отношение: http://ru.wikipedia.org/wiki/Рефакторинг |
|
|
rich |
![]()
Сообщение
#27
|
![]() Участник ![]() ![]() Группа: Участник Сообщений: 123 Регистрация: 1.3.2008 Пользователь №: 109 Спасибо сказали: 6 раз(а) Репутация: ![]() ![]() ![]() |
|
|
|
![]() ![]() ![]() |
![]() |
|
Текстовая версия | Сейчас: 4.5.2025, 8:56 |