На мой взгляд CSewingDlg — это обьект-бог, он делает столько всего, что аж дух захватывает. Накожено, может быть и ничего (стиль определенно чувствуется), но быстро разобраться в этом очень трудно, не говоря уже о поддержке такого. Это моя субьективная оценка
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Ну, этот код по читаемости гораздо лучше того, что пишу например я.
Чтобы разговор вышел из детсадовского уровня сформулируй свои претензии в виде слов — что тебе не нравится.
Я уверен, что твой сотрудник пойдёт тебе навстречу, если ты правильно сформулируешь.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
GC>
GC>// SewingDlg.h
// мне казалось, что визард лепит стандартный инклуд-гард наряду с pragma once
// для MSVC его отсутствие, в общем-то, некритично, но всё же... культуру себе надо прививать#ifndef __SEWING_DLG__H__
#define __SEWING_DLG__H__
// и в конце#undef//__SEWING_DLG__H__
GC>#pragma once
GC>#include"MainHeaders.h"// почему не засунуто в "stdafx.h" ?
GC>#include"afxwin.h"// <afxwin.h> чтобы подчеркнуть, что этот хедер вне проекта, из системной библиотеки
GC>#include <vector>
GC>#include <map>
#define NOMINMAX // где-нибудь в самом начале stdafx.h
GC>#ifdef max
GC>#undef max
GC>#endif
GC>#ifdef min
GC>#undef min
GC>#endif
Вот эту часть я бы вынес в отдельный хедер. Явно же она нужна где-то ещё, и зачем в нагрузку к ней тащить диалог?
GC>class KompasObjectHolder // немец, что ли? CompassObjectHolder
GC>{
GC>protected:
GC> reference m_reference; // не видя, как определён тип reference, сложно сказать, нужны ли все эти танцы с бубном...
GC>public:
GC> KompasObjectHolder() : m_reference(0) {}
GC> KompasObjectHolder(reference object_reference) : m_reference(object_reference) {}
GC> KompasObjectHolder(KompasObjectHolder& prev_holder) : m_reference(prev_holder.m_reference) { prev_holder.m_reference = 0; }
GC> KompasObjectHolder& operator =(KompasObjectHolder& prev_holder) {
GC> if (!m_reference) m_reference = prev_holder.m_reference, prev_holder.m_reference = 0; return *this; }
GC> ~KompasObjectHolder()
GC> {
GC> if (m_reference) DeleteObj(m_reference);
GC> m_reference = 0;
GC> }
GC> operator reference() { return m_reference; }
GC> operator bool() { return m_reference != 0; }
GC>};
// я бы сделал прощеtypedef/*boost|std*/ shared_ptr<reference> compass_obj_ptr;
compass_obj_ptr make_compass_obj_ptr(reference r) { return compass_obj_ptr(r, DeleteObj); }
// вот это - кажется архитектурным злом. Наследовать один умный указатель от другого умного указателя...
GC>class KompasIteratorHolder : public KompasObjectHolder
GC>{
GC>public:
GC> KompasIteratorHolder() {}
GC> KompasIteratorHolder(reference iterator_reference) : KompasObjectHolder(iterator_reference) {}
GC> KompasIteratorHolder& operator =(KompasIteratorHolder& prev_holder) {
GC> if (!m_reference) m_reference = prev_holder.m_reference, prev_holder.m_reference = 0; return *this; }
GC> ~KompasIteratorHolder()
GC> {
GC> if (m_reference) DeleteIterator(m_reference);
GC> m_reference = 0;
GC> }
GC>};
// быстрое-грязное решение (но опять же, я не знаю, что за сущность такая - reference)
compass_obj_ptr make_compass_obj_iter(reference r) { return compass_obj_ptr(r, DeleteIterator); }
В самом диалоге очень сильно настораживает обилие голых reference.
Надо уж определиться: или они подлежат какому-то контролю за временем жизни, или этот контроль не больно-то нужен.
Особенно это касается вектора и мапа. Ну хоть бы какое-то словесное примечание было: например, что время жизни диалога заведомо меньше времени жизни этих reference, переданных туда извне.
GC>class CSewingDlg : public CDialog // диалоговое окно CSewingDlg
GC>{
GC> DECLARE_DYNAMIC(CSewingDlg)
GC>public:
GC> CSewingDlg(CWnd* pParent = NULL); // стандартный конструктор
GC> virtual ~CSewingDlg();
GC> reference Draw2DElement(reference element, bool closed = false);
GC> reference DrawLineSeg(reference element);
GC> reference DrawCircle(reference element);
GC> reference DrawArc(reference element);
GC> reference DrawEllipse(reference element);
GC> reference DrawEllipseArc(reference element); // BAD when coercing EllipseArc & Nurbs ??bag in KOMPAS
GC> reference DrawEllipseArcWithNurbs(reference element); // new API7+IMath2D version - !EllipseArc as case of Nurbs
GC> reference DrawRectangle(reference element);
GC> reference DrawRegularPolygon(reference element);
GC> reference DrawBezier(reference element, bool closed = false); // new API7 version - allows Bezier 2 type (Equidistants fail too)
GC> reference DrawBezierWithNurbs(reference element, bool closed); // new API7+IMath2D version - !Bezier as case of Nurbs
GC> reference DrawEquidistant7(IDrawingObjectPtr& idcContour,
GC> EquidistantParam *equiparam, bool swapped); // new API7 version - no adv.features
GC> reference DrawEquidistant(EquidistantParam *equiparam, bool swapped); // new API5 version - ?
GC> reference DrawPolyline(reference element, bool closed = false);
GC> reference DrawApproximation(reference element, int curve_type);
GC> reference DrawContour(reference element, bool closed = false);
GC> reference DrawNurbs(reference element, bool closed = false); // new API7+IMath2D version - !works
GC> reference SmallNurbs3(double x1, double y1, double x2, double y2, unsigned short style);
GC> double CurvesDeviation(reference old_curve, ICurve2DPtr& new_curve, double precision, double* tpars, int points_count);
GC> inline void KompasCorrectAngles(double& dBeginAngle, double& dEndAngle, bool clockwise);
GC> int PseudoProcessAcceptedData(void* pInputData, EquidistantParam *equiparam, bool PseudoGroupMode);
GC> void PrepareContoursMakeEquidistants(reference obj_iterator, EquidistantParam* equiparam);
GC> void ProcessMarks(reference curve, reference contour, EquidistantParam* equiparam);
GC> bool CheckSwapBegPoint(double& x1, double& y1, double& x2, double& y2);
GC> bool CheckTipWithScratch(double& x1, double& y1);
GC> bool CheckSwapCurve(reference curve);
GC> reference DestroyContoursGroup(int tipSearch);
GC> double GetEquidistantPerimeter(reference element); // using destroyObj to get true length of Equidistant
GC> int CheckContourSide(reference contour, bool swapped, bool issingle);
GC> int CheckIntersections(reference contour, bool swapped, double t);
GC> bool TestInterrupt(LPSTR msg, int count);
GC>void api7test();
GC> bool Activated;
GC> PointParam m_BegPoint;
GC> reference m_grpAuxScratches;
GC> unsigned short m_curves_style;
GC>// Данные диалогового окна
GC> enum { IDD = IDD_SEWINGDLG };
GC> enum { enCurves_All = 0, enCurves_Selected, enCurves_Unselected };
GC>protected:
GC> virtual BOOL OnInitDialog();
GC> void ParseDoubleData(int nID);
GC> virtual void DoDataExchange(CDataExchange* pDX); // поддержка DDX/DDV
GC> DECLARE_MESSAGE_MAP()
GC>public:
GC> afx_msg void OnBnClickedDrawOperation();
GC> afx_msg void OnActivate(UINT nState, CWnd* pWndOther, BOOL bMinimized);
GC> afx_msg void OnKeyDown(UINT nChar, UINT nRepCnt, UINT nFlags);
GC> afx_msg void OnBnClickedCancel();
GC> afx_msg void OnBnClickedUndo();
GC> afx_msg void OnBnClickedRedo();
GC>protected:
GC> int m_StockOutCheck;
GC> int m_StockInCheck;
GC> int m_MarkingCheck;
GC> double m_StockOut;
GC> double m_StockIn;
GC> double m_MarkingStep;
GC> int m_SelectType;
GC>private:
GC> int m_UndoRedo;
GC> int m_ProcessClosedOnly;
GC> int m_ContourHasAnyLineStyles;
GC> int m_ProcessEnclosedContours;
GC> int m_MaxSplitNum;
GC> int m_MaxMarksNum;
GC> double m_MmToUnits;
GC> double m_precision;
GC> std::map<reference, reference> m_CurveParts;
GC> std::vector<reference> equi_curves; // tmp vector needed to correct Equidistant behavior
GC> int equi_curves_count; // may differ from equi_curves.size() due to m_grpAuxScratches
GC> bool m_swapped;
GC> bool m_issingle;
GC> reference m_aux_scratch;
GC> bool m_skip_scratch; // in API5 Contour()-mode all references are 0 = i.e. temp intrinsic objects for Contour()
GC>public:
GC> afx_msg void OnBnClickedManualMarks();
GC> afx_msg void OnEnKillfocusPrecision();
GC> afx_msg void OnEnKillfocusStockout();
GC> afx_msg void OnEnKillfocusStockin();
GC> afx_msg void OnEnKillfocusMarking();
GC> afx_msg void OnEnChangeMarking();
GC>};
GC>
Извини, дальше смотреть код было уже лень. Во-первых, его слишком много, во-вторых, он замусорен /**/ и плохой табуляцией, — как черновик, сойдёт, а на публику такое выносить не очень прилично.
Каким аргументам я задал значение а каким нет, гадайте. Да?
do { // pseudo-cycle to destruct tempRasters BEFORE doc-switching & CloseDocument()
KompasIteratorHolder tempApprox(CreateIterator(ALL_OBJ, 0)); // GetViewReference(0)));if(!tempApprox) break;
elem = MoveIterator(tempApprox, 'F');
} while (0);
вот это что за цикл на одно действие?
Код сложно читать -> поддерживать, есть риск получить трудноуловимые ошибки.
В коде присутствуют слишком универсальные классы.
Код, может быть и сойдет, но явно говорить, что он хорош — это глупо.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Если чесно, то код ужас. Как по стилю, так и по подпоркам и затычкам.
Но так сейчас многие пишут. Какие-то странные имена Типа GjrThowAllocWPgo, ифы типа
if ( ! (a!=0) )e=4;
break;
Нет разбивки на файлы, мало коментариев. мало пустых строк — код НЕ оформлен. Всякая экзотика в виде запятых.
Но я бы попросил его оформить как надо. И все. Ну в смысле не называл это плохим кодом.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Я думаю что с форматированием проблемы из-за смешения табов и пробелов.
Т.е. у тебя просто размер табуляции стоит не такой, как у коллеги, поэтому выглядит жутковато.
Код конечно не торт, но подобный ппц вполне ожидаем в продукте с N-летней историей.
В общем зря ты на него бочку катишь, "переписать все заново" — это скорее мечта,
а ему премию выдать надо за то что он в этом разбирается, или по-крайней мере молоко за вредность
Здравствуйте, bnk, Вы писали:
bnk>Код конечно не торт, но подобный ппц вполне ожидаем в продукте с N-летней историей. Этот код был написан в течении последних двух месяцев практически с нуля. Так сказать, свежачок....
Здравствуйте, niXman, Вы писали:
X>и да, функции размером в двести строк, тоже доставляют %)
Тут есть функция CSewingDlg::PseudoProcessAcceptedData(), которая начинается на 2029 строке и заканчивается на строке номер 3433
Здравствуйте, smeeld, Вы писали:
S>Видели бы вы такие функции
эта функция лучше, потому что в ней комментариев с избытком.
но да, можно было и ее разделить на *надцать поменьше — все бы только выиграли
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, GhostCoders, Вы писали:
GC>Здравствуйте, bnk, Вы писали:
bnk>>Код конечно не торт, но подобный ппц вполне ожидаем в продукте с N-летней историей. GC>:) Этот код был написан в течении последних двух месяцев практически с нуля. Так сказать, свежачок....
Это всё писалось целых два месяца? А как же план для программиста в 500 строк в день?
N> do { // pseudo-cycle to destruct tempRasters BEFORE doc-switching & CloseDocument()
N> KompasIteratorHolder tempApprox(CreateIterator(ALL_OBJ, 0)); // GetViewReference(0)));
N> if(!tempApprox) break;
N> elem = MoveIterator(tempApprox, 'F');
N> } while (0);
N>
N>вот это что за цикл на одно действие?
Это прикольнейшая идиома. Это не цикл, а goto end. В роли goto используется break.
N>Код сложно читать -> поддерживать, есть риск получить трудноуловимые ошибки.
Здравствуйте, GhostCoders, Вы писали:
bnk>>Код конечно не торт, но подобный ппц вполне ожидаем в продукте с N-летней историей. GC> Этот код был написан в течении последних двух месяцев практически с нуля. Так сказать, свежачок....
Здравствуйте, smeeld, Вы писали:
S>Жалкие двести строк и кому-то не нравится? S>Видели бы вы такие функции
Мой сотрудник на самом деле не намного отстал.
Тут есть функция CSewingDlg::PseudoProcessAcceptedData(), которая начинается на 2029 строке и заканчивается на строке номер 3433
Не хотелось бы такое поддерживать в будущем. Код без автора я бы править не стал.
Ещё смущает обилие конструкций "Points.push_back(.." в вектор на стеке. Производительность кода всех устраивает?
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох. GC>...
Довольно странно, что вы вот так просто выставляете собственность компании на всеобщее обозрение. Если конечно это не ваша компания, но все равно странно как-то.
Может это даже ваш код и, вместо того чтобы в этом признаться и показаться не очень профессиональным в глазах коллег, вы придумали сотрудника.
По существу: текст не помешает пропустить сквозь авто-формат.
Еще можно провести рефакторинг: сделать наименование однообразным, например, окончательно определиться с "m_" в именах членов класса:
int equi_curves_count;
bool m_swapped;
А вообще, качество кода определяется наличием ошибок и соответствием ТЗ. Если программа делает то, что требуется, не течет и не тормозит, то нечего придираться.
N> do { // pseudo-cycle to destruct tempRasters BEFORE doc-switching & CloseDocument()
N> KompasIteratorHolder tempApprox(CreateIterator(ALL_OBJ, 0)); // GetViewReference(0)));
N> if(!tempApprox) break;
N> elem = MoveIterator(tempApprox, 'F');
N> } while (0);
N>
N>вот это что за цикл на одно действие?
Стандартная императивная конструкция, предоставляет блок (scope), из которого можно выйти по break. Также широко используется в препроцессорных дефайнах, если нужен блок, но { ... } использовать нежелательно.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
GC>
Здравствуйте, Qbit86, Вы писали: Q>Стандартная императивная конструкция, предоставляет блок (scope), из которого можно выйти по break. Также широко используется в препроцессорных дефайнах, если нужен блок, но { ... } использовать нежелательно.
Ну конкретно тут в чем плюс его использования?
Понимаю если бы там макросы были, а так...
Здравствуйте, Qbit86, Вы писали:
Q>Здравствуйте, -n1l-, Вы писали:
N>>
N>> do { // pseudo-cycle to destruct tempRasters BEFORE doc-switching & CloseDocument()
N>> KompasIteratorHolder tempApprox(CreateIterator(ALL_OBJ, 0)); // GetViewReference(0)));
N>> if(!tempApprox) break;
N>> elem = MoveIterator(tempApprox, 'F');
N>> } while (0);
N>>
N>>вот это что за цикл на одно действие?
Q>Стандартная императивная конструкция, предоставляет блок (scope), из которого можно выйти по break. Также широко используется в препроцессорных дефайнах, если нужен блок, но { ... } использовать нежелательно.
И насчет стандартности я бы поспорил, да трюк do {} while (false) для макросов
это стандартная идиома,
но вот блок для которого можно выйти по "break",
редко встречающаяся.
Здравствуйте, Zhendos, Вы писали:
Z>Разве не намного проще?
Возможно, перед нами артефакт незавершённого упрощения — изначально блок был сложнее и содержал больше выходов по break.
Z>но вот блок для которого можно выйти по "break", Z>редко встречающаяся.
Иногда встречается в длинных многошаговых инициализациях, где по break осуществляется фоллбэк, аналог goto-перехода к метке error.
Здравствуйте, -n1l-, Вы писали:
К>>Это прикольнейшая идиома. Это не цикл, а goto end. В роли goto используется break. N>И типа if тут никак? Чем это хуже?
Здравствуйте, -n1l-, Вы писали:
N>Ну конкретно тут в чем плюс его использования?
Вообще по правильному ( в теории, но не на практике ) вместо такого while(1) {;;; break;} или do {}while(0); надо делать функцию в которой уже выходить на end просто по return.
Но на практике особенно там где код не оптимизируют ( ядро например ) но производительность требуется используют такой вот метод.
while (1){
HANDLE h = open();
if ( !h )
break;
while (1){
s=write(h,"{");
if ( !s )
break;
write(h,"abc,def,uuu");
write(h,"}");
break;
}
close(h);
break;
}
то есть это по уму, по идеализму надо заменять на функцию даже если она один раз вывывается, но еще не понятно что яснее — функция которая написана только для того чтобы брековаться по return вместо goto.
То есть есть такие операции которые требуют "операций скобок" — открыл -> надо закрыть, записал начало "{" надо записть и конец "}". И так делее.
В этом случае в конце кода действий возникает логика — а открыли мы или нет, и если открыли надо закрыть. И так далее.
Но если применять логику иную — типа создавать объект у которого есть свойсто "открыть" и есть констуктор и есть деструктор то тогда такая логика с проверкой а мы открыли или нет в том же деструкторе выглядит логично. Но если это просто небольшой код, и нет нужды городить обьекты и классы, ну как я уже сказал в ядре например. Где эфективность важнее красоты, а оптимизации от компилятора нет, то пишут вот так — ну не очень красиво, но по привыкнешь — нормально.
Здравствуйте, uzhas, Вы писали:
U>эта "идиома" в плюсовом коде не нужна. в старом добром Си — на здоровье
Предложи наглядное решение на С++. Мне вот интересно.
Здравствуйте, Кодт, Вы писали:
К>>>Это прикольнейшая идиома. Это не цикл, а goto end. В роли goto используется break. N>>И типа if тут никак? Чем это хуже?
По-моему этой идиоме место только в коде на C и только для идеологических противников goto cleanup
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Вы еще попробуйте код показать на http://codereview.stackexchange.com/
Я думаю, там будет устроен разнос.
На мой взгляд, код ужасен.
Чтобы не быть субъективным, нужно принять стандарт оформления кода и встроить линтер на прекоммит.
Тогда никто не сможет писать в одном месте "m_", "_" или просто "someMemberName".
Принять решение о написании тестов. Тогда никто не будет писать таких огромных классов.
Использовать VCS, их куча на выбор, тогда не будет никаких закомментированных участков кода.
Организовать код-ревью если не каждого коммита в master, то хотя бы важных фич.
Тогда Вы сможете аргументировать.
Я понимаю, что это может показаться сложным и довольно дорогим в смысле времени и денег.
Но представьте, сколько Вы готовы потратить на содержание сотрудников, которые пишут ужасный код, но разбираются в нем, и поэтому их не уволить?
На отладке, потому что нет тестов и все проверяется вручную?
На поддержке, потому что нужно по пеплу орла и кофейной гуще гадать что сломалось у клиента?
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Плохое форматирование, проблемы с конвенциями именования.
Bad smells of code прямо по фаулеру: дубликаты, магические числа, длинные метода.
Очень плохо с архитектурой: нарушение SRP, не выделены классы которые прямо-таки напрашиваются на создание (вроде Point)Думаю, не помешало бы использовать пару структурных паттёрнов.
Ещё мне кажется, что человек не знает о назначении ASSERT.
Из плюсов, человек знает как про double и его проблемы.
Здравствуйте, Zhendos, Вы писали:
Z>т.к. что и куда нужно возвращать из контекста не ясно, Z>то просто не ловим исключения, пусть они идут наверх
Ну ок, ок. Напрягает то, что сущностей X, Y, Z как-то не выделено.
Здравствуйте, _Dreamer, Вы писали:
_D>На мой взгляд, код ужасен.
Плюсую. _D>Чтобы не быть субъективным, нужно принять стандарт оформления кода и встроить линтер на прекоммит.
Дико плюсую. _D>Принять решение о написании тестов. Тогда никто не будет писать таких огромных классов.
Заплюсовываю! _D>Использовать VCS, их куча на выбор, тогда не будет никаких закомментированных участков кода.
Там скорее всего есть, встречается такое, что в ветках комменты. _D>Организовать код-ревью если не каждого коммита в master, то хотя бы важных фич.
Просто дико заплюсовываю. Нигде не видел системы codereview, там где работал, кроме одного проекта QT'a.
Когда был контрибьютером, просто офигел от того, как четко и правильно работают люди. _D>Тогда Вы сможете аргументировать.
Согласен, поддерживаю, всецело отдаю свой голос за.
N>>> do { // pseudo-cycle to destruct tempRasters BEFORE doc-switching & CloseDocument()
N>>> KompasIteratorHolder tempApprox(CreateIterator(ALL_OBJ, 0)); // GetViewReference(0)));
N>>> if(!tempApprox) break;
N>>> elem = MoveIterator(tempApprox, 'F');
N>>> } while (0);
N>>>
N>>>вот это что за цикл на одно действие?
К>>Это прикольнейшая идиома. Это не цикл, а goto end. В роли goto используется break.
N>И типа if тут никак? Чем это хуже?
N>
Не вникал в остальной код, но здесь разное время жизни tempApprox. В первом случае он уничтожается в конце блока, в твоем — живет себе дальше. Но твой вариант можно в еще один блок пихнуть.
Переубедить Вас, к сожалению, мне не удастся, поэтому сразу перейду к оскорблениям.
Здравствуйте, Kernan, Вы писали:
K>Здравствуйте, Zhendos, Вы писали:
K>try-catch забыл. Я не особо смотрел на код, но вроде после do-while было что-то ещё.
Эти С++-ники такие С++-ники. Кто-то думает что с try{}catch будет производительнее?
В try{}catch произойдёт вызов __cxa_throw в котором вызовется __Unwind_RaiseException,
а там, мерзопакостный
_Unwind_RaiseException(struct _Unwind_Exception *exc)
{
..................................................
/* Phase 1: Search. Unwind the stack, calling the personality routine
with the _UA_SEARCH_PHASE flag set. Do not modify the stack yet. */
while (1)
{
_Unwind_FrameState fs;
/* Set up fs to describe the FDE for the caller of cur_context. The
first time through the loop, that means __cxa_throw. */
code = uw_frame_state_for (&cur_context, &fs);
if (code == _URC_END_OF_STACK)
/* Hit end of stack with no handler found. */
return _URC_END_OF_STACK;
if (code != _URC_NO_REASON)
/* Some error encountered. Usually the unwinder doesn't
diagnose these and merely crashes. */
return _URC_FATAL_PHASE1_ERROR;
/* Unwind successful. Run the personality routine, if any. */
if (fs.personality)
{
code = (*fs.personality) (1, _UA_SEARCH_PHASE, exc->exception_class,
exc, &cur_context);
if (code == _URC_HANDLER_FOUND)
break;
else if (code != _URC_CONTINUE_UNWIND)
return _URC_FATAL_PHASE1_ERROR;
}
/* Update cur_context to describe the same frame as fs. */
uw_update_context (&cur_context, &fs);
}
/* Indicate to _Unwind_Resume and associated subroutines that this
is not a forced unwind. Further, note where we found a handler. */
exc->private_1 = 0;
exc->private_2 = uw_identify_context (&cur_context);
cur_context = this_context;
code = _Unwind_RaiseException_Phase2 (exc, &cur_context);
if (code != _URC_INSTALL_CONTEXT)
return code;
uw_install_context (&this_context, &cur_context);
}
Но кроме этого ещё куча работы и куча вызовов.
Может лучше сразу while(1){ ... break/return } ?
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Проводим, значит, код ревью
GC>
GC>// SewingDlg.h
GC>
У вас coding conventions вообще есть? где оформленный заголовок с именем файла, автором, датой создания, названием проекта?
GC>
GC>#ifdef max
GC>#undef max
GC>#endif
GC>#ifdef min
GC>#undef min
GC>#endif
GC>
Вот это нужно комментировать.
GC>
GC>class KompasObjectHolder
GC>
Стоп. Безотносительно того, что было уже сказано по поводу частокола конструкторов: Где комментарии? Где разъяснение, зачем это нужно. Где обоснование невозможности генеразизации класса (шаблоны)? И, чтобы уж добить, зачем оно вообще надо, если есть smart pointers?
GC>
GC>class KompasIteratorHolder : public KompasObjectHolder
GC>
Все то же самое плюс — почему в одном заголовочном файле объявлено более одного класса?
GC>
GC>class CSewingDlg : public CDialog // диалоговое окно CSewingDlg
GC>
Дальнейшие поскипанные объявления методов тоже не проходят ревью по поводу отсутствия сколько-нибудь внятной документации. Об универсальном всемогуторе поговорим позже.
GC>
GC>reference CSewingDlg::Draw2DElement(reference element, bool closed)
GC>{
GC> if (ReturnResult()) ResultNULL();
GC> if (!ExistObj(element)) return 0;
GC> switch (GetObjParam(element, 0, 0, ALLPARAM))
GC> {
GC> case CONTOUR_OBJ:
GC> return DrawContour(element); // ONLY APPROXIMATION NOW
GC> case LINESEG_OBJ:
// Вызовы отрисовок для остальных типов фигур в ужасе поскипаны
GC> break;
GC> }
GC> return 0;
GC>}
GC>
А это что вообще такое? Такое слово, как инкапсуляция, Вашему подчиненному вообще знакомо?
Это не C++ код, это страшное проявление C в его худшем виде, которое нынче еще поискать надо.
На этом в нашей компании код ревью бы прекратился за его дальнейшей бессмысленностью.
L>У вас coding conventions вообще есть? где оформленный заголовок с именем файла, автором, датой создания, названием проекта?
для всего этого есть система контроля версий.
есть смысл разве что писать копирайт, и то не очень-то нужно.
GC>>
GC>>#ifdef max
GC>>#undef max
GC>>#endif
GC>>#ifdef min
GC>>#undef min
GC>>#endif
GC>>
L>Вот это нужно комментировать.
не нужно. те кто пишут под винду и так знают про эту проблему. другое дело что лучше NOMINMAX
GC>>
GC>>class KompasObjectHolder
GC>>
L>Стоп. Безотносительно того, что было уже сказано по поводу частокола конструкторов: Где комментарии? Где разъяснение, зачем это нужно.
кажется имя класса и так говорит о том зачем это нужно.
GC>>
GC>>class KompasIteratorHolder : public KompasObjectHolder
GC>>
L>Все то же самое плюс — почему в одном заголовочном файле объявлено более одного класса?
а почему бы и нет? особенно если классы используются вместе
Здравствуйте, smeeld, Вы писали:
S>Здравствуйте, Kernan, Вы писали:
K>>Здравствуйте, Zhendos, Вы писали:
K>>try-catch забыл. Я не особо смотрел на код, но вроде после do-while было что-то ещё.
S>Эти С++-ники такие С++-ники. Кто-то думает что с try{}catch будет производительнее? S>В try{}catch произойдёт вызов __cxa_throw в котором вызовется __Unwind_RaiseException, S>а там, мерзопакостный
Непонятно с чем вы спорите. В коде с "do {} while" обрабатываются ошибки выделения памяти,
т.е. вероятность выброса исключения очень мала,
код для бросания исключений и их обработки в частности ваш "__Unwind_RaiseException",
будет вызван только в случае нештатной ситуации, иными словами
в данном случае, с использованием современных C++ компиляторов,
измененный код будет работать не медленнее чем первоночальный,
а возможно и быстрее.
Конечно, если вероятность не получить какой-то ресурс велика,
то придется писать по другому, например сначала,
захватывая ресурс, и если нет ошибок передача его объекту класса
обертки, который освободит ресурс в деструкторе. Тем самым
избавляясь от необходимости кидать исключение из конструктора объекта обертки.
Выше брезгливо морщились от выхода из последовательности операций, при
наступлении ошибки на каком-то этапе, с помощью do{ ...break..}while(1)
и говорилось про необходимость использовать это try{ throw x }catch();
А это как бы менее производительно, чем вариант с while{ break};
И вызов __Unwind_RaiseException произойдёт при всяком throw, ибо как есть стандарт.
L>>У вас coding conventions вообще есть? где оформленный заголовок с именем файла, автором, датой создания, названием проекта? A>для всего этого есть система контроля версий.
Иногда, читай вероятность этого стремится к единице, информация об истории может потеряться. Происходит это при переезде на другую систему контроля версий или при просто копипасте файла из одного места в другое.
A>есть смысл разве что писать копирайт, и то не очень-то нужно.
Заголовок с копирайтом позволяет сразу понять, что это, откуда оно взялось и кто в этом виноват. Даже когда работаешь из движущегося поезда без подключения к системе. Но я не настаиваю, хозяин-барин, это и правда не так уж и важно.
GC>>>
GC>>>#ifdef max
GC>>>#undef max
GC>>>
L>>Вот это нужно комментировать. A>не нужно. те кто пишут под винду и так знают про эту проблему. другое дело что лучше NOMINMAX
Я знаю. Это не отменяет того, что хорошей инженерной практикой является комментирование подобного. Даже если это всем известно, это не перестает быть грязным хаком.
GC>>>
GC>>>class KompasObjectHolder
GC>>>
L>>Стоп. Безотносительно того, что было уже сказано по поводу частокола конструкторов: Где комментарии? Где разъяснение, зачем это нужно. A>кажется имя класса и так говорит о том зачем это нужно.
Комментарий должен быть. Он должен обосновывать необходимость этого холдера, в т.ч. содержать объяснение, почему аффтар не стал использовать умный указатель вместо этого, а также рассказывать, с какого такого бодуна объект вдруг стал называться ссылкой и зачем ему может понадобиться внешний удалятор. Любое инженерное решение должно быть обосновано. У меня сразу возникает вопрос — зачем нужно убивать объект, который является членом класса, неким внешним функтором. Если бы автор потрудился это описать в комментарии, он бы примерно на полпути, скорее всего, догадался бы, что инкапсуляция — не просто красивое слово, которое непонятно как правильно транслителировать на русском языке, а весьма правильный подход к проектированию подобных классов. Ну или бы четко описал, почему это должно быть сделано именно так, а не иначе.
GC>>>
GC>>>class KompasIteratorHolder : public KompasObjectHolder
GC>>>
L>>Все то же самое плюс — почему в одном заголовочном файле объявлено более одного класса? A>а почему бы и нет? особенно если классы используются вместе
В итоге все классы в программе используются вместе так или иначе. Это не повод всех их запихивать в один файл.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Не стоит тащить на форум огромный код для его разбора. Достаточно небольшого фрагмента.
Разбирать код по принципу "я евангелист и этот код не вписывается в моё евангелие" не стоит.
Если код делается в команде, то должны быть выработаны командные требования по различным аспектам, включая оформление кода. Если этого сделано не было, то жаловаться на плохое оформление не стоит.
Прежде чем давать оценку, стоит честно ответить на вопросы: 1) этот код решает проставленную задачу?, 2) есть ли в нем ошибки?, 2) не жрет ли он неоправданно много ресурсов?
Если код работоспособен, то во многих случаях заниматься превращением его в памятник программистскому искусству не стоит. Иначе поставят памятник вашей компании. Посмертный.
Если же очень хочется и денег заработать, и шедевр написать, то начинать надо всё-таки с проработки хорошей архитектуры.
template <class T>
T Replace_null(T &t) { T ret(t); t=T(); return ret; }
/* class KompasObjectHolder */class KompasObjectHolder // Этот класс лучше превратить в шаблон
{
protected:
reference ptr; // лучше писать явно указатель на объект KompasObject *public:
// constructors
KompasObjectHolder() : ptr(0) {}
KompasObjectHolder(reference ptr_) : ptr(ptr_) {}
KompasObjectHolder(KompasObjectHolder &obj) : ptr(obj.ptr) { obj.ptr=0; }
KompasObjectHolder(KompasObjectHolder &obj) : ptr(Replace_null(obj.ptr)) {} // Здесь следует использовать такую функцию
KompasObjectHolder & operator = (KompasObjectHolder &obj)
{
if( !ptr ) ptr = obj.ptr, obj.ptr = 0;if( !ptr ) ptr=Replace_null(obj.ptr); // И здесь тоже
// Кстати говоря, эта логика поведения мне не совсем понятна return *this; // не экономь на строчках!
}
~KompasObjectHolder()
{
if( ptr ) DeleteObj(ptr); // Нормальная функция DeleteObj должна принимать нулевой указательptr=0;// Это не нужно
}
// methodsoperator reference() const { return ptr; } // здесь нужен constoperator bool() const { return ptr!=0; }// и здесь тоже, более того, этот оператор не нужен и даже вреден, его функции будет выполнять первый оператор, будут проблемы с разрешением перегрузки
};
Здравствуйте, GhostCoders, Вы писали:
GC>Здравствуйте, bnk, Вы писали:
bnk>>Код конечно не торт, но подобный ппц вполне ожидаем в продукте с N-летней историей. GC> Этот код был написан в течении последних двух месяцев практически с нуля. Так сказать, свежачок....
Почерк динозавра просматривается
IMHO, ничего особо страшного, если не планируется этот код кому-либо отдавать или долго развивать-поддерживать, т.е. "выстрелил и забыл".
Здравствуйте, Шахтер, Вы писали:
Ш>Вообщем, начинать надо с проработки базовых вещей. Проект сделан неряшливо. И я думаю, что вина здесь не только автора данного кода.
Здравствуйте, -n1l-, Вы писали:
N>Здравствуйте, _Dreamer, Вы писали:
N>встроить линтер на прекоммит.
N>Так и называется, "Линтер"? В svn такое есть?
ну если погуглить "cpp lint", то один из первых находится Cpplint + еще что-то.
и да, оно легко прикручивается к svn, много всего настраивается (вкл\выкл) и в основном заточено на google style, который не всем нравится и мне в частности тоже, поэтому кучу фич нужно было выключать, и да, как и описано, довольно часто возникают false positive и приходится затыкать глупыми комментами // NOLINT.
как по мне, лучше тим лиду подписаться на коммиты и бегло просматривать их, выдавая люлей коммитерам нарушающим стиль. и параллельно с этим проводить code review, на которых мемберы учатся т.ч. правильному стилю.
я взглянул только на заголовочный файл и мои глаза закровоточили. Работа с графикой должна быть отделена от работы с юаем, т.е. должна идти работа с абстрактным графическим контекстом — хотя бы обёрткой над hbitmap, всё что должен делать диалог — обрабатывать ввод пользователя и звать методы этого контекста, в onPaint блитить к себе.
А также куча сгенерированного кода. Свой и генерируемый код — нужно разделять, ибо поддержка IDE может перестанет работать. Я бы на вашем месте генерируемый код тут не постил.
Да и нотации типа m_, в новом коде я не приемлю, уже есть нормальные IDE.
Здравствуйте, vpchelko, Вы писали:
V>Да и нотации типа m_, в новом коде я не приемлю, уже есть нормальные IDE.
помимо нормальных ide код может оказаться в блокноте, вьювере, письме, распечатан, передан по скайпу, запосчен на форум и стековерфлоу. Чем m_ помешал-то?
Здравствуйте, vpchelko, Вы писали:
V>Здравствуйте, GhostCoders, Вы писали:
V>Меня очень смущают "наполнители" V>
GC>> y = epEllipseParam.yc;
GC>> double angle = epEllipseParam.ang;
GC>> double a = epEllipseParam.a;
GC>> double b = epEllipseParam.b;
V>
V>Таких "наполнителей" в коде дохрена.
Наверное чтобы в дебаггере было удобно смотреть (возможно что в вотч эти самые x, y, a, b добавлены)
Хотя да, общая неряшливось присутствует. Явно у чувака с чувством прекрасного напряженка.
V>Да и нотации типа m_, в новом коде я не приемлю, уже есть нормальные IDE.
"m_" ничем не хуже "_" разве что на один символ длиннее, к тому же там mfc
Про поддержку IDE — она позволяет забыть про m_lpstr (венгерскую нотацию), но не про члены класса.
Здравствуйте, antropolog, Вы писали:
A>помимо нормальных ide код может оказаться в блокноте, вьювере, письме, распечатан, передан по скайпу, запосчен на форум и стековерфлоу.
Проблемы индейцев шерифа не волнуют. A>Чем m_ помешал-то?
Потому-что номенклатура.
Здравствуйте, bnk, Вы писали:
bnk>Наверное чтобы в дебаггере было удобно смотреть (возможно что в вотч эти самые x, y, a, b добавлены) bnk>Хотя да, общая неряшливось присутствует. Явно у чувака с чувством прекрасного напряженка.
Зато нам удобно смотреть — видим большую функцию на пол экрана — а по сути в ней ничего нет.
V>>Да и нотации типа m_, в новом коде я не приемлю, уже есть нормальные IDE.
bnk>"m_" ничем не хуже "_" разве что на один символ длиннее, к тому же там mfc bnk>Про поддержку IDE — она позволяет забыть про m_lpstr (венгерскую нотацию), но не про члены класса.
ИМХО номенклатура. У меня локальные переменные и члены класса по разному подсвечиваются.
Здравствуйте, smeeld, Вы писали:
S>Здравствуйте, Шахтер, Вы писали:
Ш>>Вообщем, начинать надо с проработки базовых вещей. Проект сделан неряшливо. И я думаю, что вина здесь не только автора данного кода.
S>А что Вы скажете про код из опенсорсов?
Это зависит от проекта, конечно же. Скажем boost сделан достаточно хорошо. Многие open source проекты, однако, сделаны довольно плохо. Скажем, в том же Linux е нет нормальной дисциплины имен, что для крупного C проекта просто необходимо.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Здравствуйте, vpchelko, Вы писали:
V>Здравствуйте, bnk, Вы писали:
bnk>>Тогда конечно можно и без "_". У меня одиниково. А как это в студии настроить?
V>Я использую VA для таких целей.
Понятно что VA.
Я просто что-то не знаю у него такой настройки, чтобы переменные и мемберы показывать по-разному?
Здравствуйте, bnk, Вы писали:
bnk>Понятно что VA. bnk>Я просто что-то не знаю у него такой настройки, чтобы переменные и мемберы показывать по-разному?
там в Fonts & Colors есть опция Local symbols in bold ( правда у меня достаточно старая версия, не знаю как в новых )
Здравствуйте, antropolog, Вы писали:
A>Здравствуйте, bnk, Вы писали:
bnk>>Понятно что VA. bnk>>Я просто что-то не знаю у него такой настройки, чтобы переменные и мемберы показывать по-разному?
A>там в Fonts & Colors есть опция Local symbols in bold ( правда у меня достаточно старая версия, не знаю как в новых )
А понял про что ты, пасиб. В новых так же. Просто тогда будет разный шрифт — мне это не нравится
Здравствуйте, landerhigh, Вы писали:
L>>>У вас coding conventions вообще есть? где оформленный заголовок с именем файла, автором, датой создания, названием проекта? A>>для всего этого есть система контроля версий.
L>Иногда, читай вероятность этого стремится к единице, информация об истории может потеряться. Происходит это при переезде на другую систему контроля версий или при просто копипасте файла из одного места в другое.
A>>есть смысл разве что писать копирайт, и то не очень-то нужно.
L>Заголовок с копирайтом позволяет сразу понять, что это, откуда оно взялось и кто в этом виноват. Даже когда работаешь из движущегося поезда без подключения к системе. Но я не настаиваю, хозяин-барин, это и правда не так уж и важно.
тут есть два противоположных подхода —
ориентироваться на то что у тебя практически всегда есть самые современные инструменты, и использовать их,
или ориентироваться на то что однажды у тебя этих инструментов не будет, и по этому использовать эти замечательные инструменты по минимуму.
кажется что для максимальной продуктивности надо оптимизировать именно повседневную работу, а не те редкие случаи когда тебе пару раз в год надо поработать оффлайн или без VPN.
потери истории изменений и т.п. случаются, но такое бывает ну очень редко.
L>Комментарий должен быть. Он должен обосновывать необходимость этого холдера, в т.ч. содержать объяснение, почему аффтар не стал использовать умный указатель вместо этого, а также рассказывать, с какого такого бодуна объект вдруг стал называться ссылкой и зачем ему может понадобиться внешний удалятор. Любое инженерное решение должно быть обосновано. У меня сразу возникает вопрос — зачем нужно убивать объект, который является членом класса, неким внешним функтором. Если бы автор потрудился это описать в комментарии, он бы примерно на полпути, скорее всего, догадался бы, что инкапсуляция — не просто красивое слово, которое непонятно как правильно транслителировать на русском языке, а весьма правильный подход к проектированию подобных классов. Ну или бы четко описал, почему это должно быть сделано именно так, а не иначе.
это и есть умный указатель. код очевидно писался для С++03, по этому unique_ptr с кастомным делитером нету.
зачем используется функция DeleteObj — наверное это понятно из ее реализации. может там какой-то отладочный код, или возможность подключения глобального кастомного аллокатора.
и я совершенно не согласен с тем что любое инженерное решение должно быть обосновано комментариями.
многие паттерны и идиомы общеизвестны, легко распознаются на глаз, и потому в комментариях не нуждаются.
L>>>Все то же самое плюс — почему в одном заголовочном файле объявлено более одного класса? A>>а почему бы и нет? особенно если классы используются вместе L>В итоге все классы в программе используются вместе так или иначе. Это не повод всех их запихивать в один файл.
есть существенная разница между "запихнуть всё в 1 файл", и "запихнуть несколько в 1 файл".
требование "1 файл — 1 класс" звучит так же экстремально как и попытка запихнуть всю программу в 1 файл.
Здравствуйте, RonWilson, Вы писали:
RW>этого сотрудника не Александром случаем зовут?
Нет, это Юра Лазарев.
Динозар еще тот. Он еще в 80-х на Фортране что-то писал.
Анархист и вообще, человек со странностями (не только в коде).
Оказывается, он про do{ }while(false) у МакКонела прочитал.
Бить функции на более мелкие — категорически отказывается, типа для него это плохо,
приходиться "скакать как козлу" с метода на метод, непонятным код становиться.
Заменить стиль линий (константа 1 в нескольких местах в этом коде) на const int TMP_STYLE_LINE = 1; согласился сделать только после часа дискуссий (!).
При этом мы в какие дебри только не лезли, он и в метафизику какую-то забрел и черт знает еще что (это когда я ему пытался доказать что магические константы — это зло).
При этом, он обвиняет меня в том, что я невнимательно прочитал "МакКонела" ("Совершенный код"),
а он прочитал его очень внимательно, результат прочтения данной книжки — это его выше приведенный код.
Я спрашиваю его — а что МакКонел рекомендует использовать магические константы и методы по 1000 строк в длину?
А он на какие-то неправильные аналогии выходит, например, "муж в квартире чинит свет. Летят стружки,
а жена его пилит. А муж в интересах жены делает". — поэтому у него в коде много балласта,
отладочной информации, которая портит форматирование и т.д.
Я совершенно серьезно, не шучу.
Вообще непробиваемый. С ним можно спорить и час и два и весь день. Толку почти 0.
Упрямый. Систему контроля версий (у нас GIT), не любит. Его перл это "это гит виноват".
Можно ему дать какую-нидубь книжку, например, "Рефакторинг" Мартина Фаулера — он ее прочтет, а делать все будет по-старому.
Могу составить список замечаний к его коду — только все равно, он его исправлять не будет.
Я уже махнул на него рукой — код ревью делать бессмысленно.
Поэтому он не работает в основной команде, а одиночкой над своим проектом.
Проект у него небольшой, в стиле "сделали" проект, сдали и забыли. Да и ничего там особо секретного нет, поэтому я выложил его тут как есть.
Никто туда лезть не собирается.
Может как-нибудь видео дискуссии с ним на видео сниму — круче Петросяна будет.
С ним спорить вообще невозможно — у меня некоторые сотрудники хватаются за голову.
Приемами демагогии владеет он отлично. Причем выходит так — что он один пишет отличный код,
"внимательно" прочитал "Совершенный Код" МакКонелла, а мы читали только поверхностно это книжку, и толком ее-то не знаем.
Еще типа что пишем одни только "обвертки", которые ничего не делают, много куча коротких функций,
которые тоже ничего не делают, только вызывают другие функции и т.д.
есть у меня такой знакомый.
так у меня когда с коллегами аргументы заканчивались — мы лезли на форум, задавали вопрос, и потом мнение большинства приводили как пример.
так он отвечал что-то типа "так они все идиоты, кого вы мне в пример приводите?"
в итоге, все забили ему что-либо объяснять...
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, GhostCoders, Вы писали:
RW>>этого сотрудника не Александром случаем зовут? GC>Нет, это Юра Лазарев.
был у меня на оочень данвней работе подобный, читаю Ваш пост как про него почти. Единственное, что помогло вытащить как клеща наружу в реальный мир (не, не новомодные вещи какие-то) этого кренделя, так чуть подтолкнуть, сделать почву в собственных сомнениях, а потом он сам старался передовое отдать мне, а сам возвращался в свой погребок. Есть такие люди, с ними приходится работать. Плакать не надо, люди разные.
Здравствуйте, Abyx, Вы писали:
A>тут есть два противоположных подхода - A>ориентироваться на то что у тебя практически всегда есть самые современные инструменты, и использовать их, A>или ориентироваться на то что однажды у тебя этих инструментов не будет, и по этому использовать эти замечательные инструменты по минимуму.
A>кажется что для максимальной продуктивности надо оптимизировать именно повседневную работу, а не те редкие случаи когда тебе пару раз в год надо поработать оффлайн или без VPN.
Мне офлайн работать приходится весьма регулярно.
A>потери истории изменений и т.п. случаются, но такое бывает ну очень редко.
Достаточно одного раза в 10 лет, и каждый раз придется заново проводить расследование "откуда вообще мы это взяли".
A>это и есть умный указатель. код очевидно писался для С++03, по этому unique_ptr с кастомным делитером нету.
Каким местом это умный указатель? Ты повнимательнее посмотри. Если это и умный указатель, то кривой на всю голову.
И еще, сегодня 2014, С++03 совершенно неочевиден.
A>зачем используется функция DeleteObj — наверное это понятно из ее реализации. может там какой-то отладочный код, или возможность подключения глобального кастомного аллокатора.
Еще раз, DeleteObj применяется к значению, а не указателю. Это либо самый страшный случай нарушения принципов ООП, который я видел в этом году, либо что-то чрезвычайно хитрое. В обоих случаях такое решение должно быть обосновано, и обосновано именно здесь.
A>и я совершенно не согласен с тем что любое инженерное решение должно быть обосновано комментариями.
Ты можешь не соглашаться, но это не делает тебя правым. Я требую обосновывать абсолютно все классы. Без исключения.
В частности и особенно те, которые очень похожи на что-то стандартное.
И знаешь что? Во многих случаях аффтар подобного говнокода, со скрипом рожая камент (каменты писать некогда, копать нужно), где-то на третьей строчке понимает, что написал какую-то фигню, и что все это можно заменить стандартным(бустовским) shared_ptr, например.
A>многие паттерны и идиомы общеизвестны, легко распознаются на глаз, и потому в комментариях не нуждаются.
Тут распознается лишь один паттерн.. и это вовсе не хорошие новости.
L>>>>Все то же самое плюс — почему в одном заголовочном файле объявлено более одного класса? A>>>а почему бы и нет? особенно если классы используются вместе L>>В итоге все классы в программе используются вместе так или иначе. Это не повод всех их запихивать в один файл.
A>есть существенная разница между "запихнуть всё в 1 файл", и "запихнуть несколько в 1 файл". A>требование "1 файл — 1 класс" звучит так же экстремально как и попытка запихнуть всю программу в 1 файл.
Здравствуйте, GhostCoders, Вы писали:
GC>Нет, это ***. GC>При этом, он обвиняет меня в том, что ..., Вообще непробиваемый ... Упрямый ... GC>Я уже махнул на него рукой ... Поэтому он не работает в основной команде, а одиночкой над своим проектом GC>С ним спорить вообще невозможно — у меня некоторые сотрудники хватаются за голову. Приемами демагогии владеет он отлично ... GC>Еще типа что пишем одни только "обвертки", которые ничего не делают...
Как-то не комильфо... Раскрываете имена, да и не о C++ совсем. Если всё так плохо, почему он ещё работает в компании?
GC> Бить функции на более мелкие — категорически отказывается, типа для него это плохо, приходиться "скакать как козлу" с метода на метод, непонятным код становиться. Заменить стиль линий (константа 1 в нескольких местах в этом коде) на const int TMP_STYLE_LINE = 1; согласился сделать только после часа дискуссий (!).
Спорить о таких вещах странно: не стоит тратить нервную энергию на то, что можно описать в корпоративном стандарте кодирования..
Здравствуйте, GhostCoders, Вы писали:
GC>Оказывается, он про do{ }while(false) у МакКонела прочитал.
в целом, я положительного мнения об этом труде
решил найти подтверждения:
в главе 16 Циклы можно найти параграф "Аномальные циклы с выходом", где демонстрируется замена "плохой практики (С++)"
goto Start:
while ( expression ) {
// Делаем что-то.
...
Start:
// Делаем что-то еще.
...
}
на православный лучший вариант
while ( true) {
Блоки перед и после break поменяны местами
// Делаем что-то еще.
...
if ( !( expression ) ) {
break;
}
// Делаем что-то.
...
}
то есть показано, как заменить цикл со странным входом на обычный цикл с break выходом. подчеркну, что здесь описываются циклы (то есть когда подразумевается наличие нескольких итераций)
в следующей же главе обсуждается goto и не упоминается применение "аномальных" циклов, как альтернативы. даже название уже намекает, что использовать их нежелательно
Здравствуйте, -n1l-, Вы писали:
N>Здравствуйте, _Dreamer, Вы писали:
N>встроить линтер на прекоммит.
N>Так и называется, "Линтер"? В svn такое есть?
Уже сказали про cpplint, по факту это может быть что угодно для проверки стиля.
Хоть clang-format (http://clang.llvm.org/docs/ClangFormatStyleOptions.html)
У cpplint действительно есть false-positive срабатывания, но с этим можно жить, а можно его допиливать.
Так что это можно прикрутить не только на svn.
Здравствуйте, zaufi, Вы писали:
Z>как по мне, лучше тим лиду подписаться на коммиты и бегло просматривать их, выдавая люлей коммитерам нарушающим стиль. и параллельно с этим проводить code review, на которых мемберы учатся т.ч. правильному стилю.
Это может быть куча времени, к сожалению. А скрипт не устает и не отвлекается, как раз для него работа.
К тому же, Вы предлагаете оценивать коммиты постфактум?
Именно этого я бы и не стал допускать.
Потому что по разным причинам коммит может так и остаться в ненадлежащем виде.
А вот тупо не дать закоммитить без -n уже гораздо действеннее, не смотря на некоторые неудобства.
Здравствуйте, smeeld, Вы писали:
S>И вызов __Unwind_RaiseException произойдёт при всяком throw, ибо как есть стандарт.
Речь как бы не совсем про это была.
Здравствуйте, GhostCoders, Вы писали:
GC>Вообще непробиваемый. С ним можно спорить и час и два и весь день. Толку почти 0. GC>Упрямый. Систему контроля версий (у нас GIT), не любит. Его перл это "это гит виноват".
вина лида очевидна. Не позднее 4го дня затихшего сотрудника надо заставить систематизировать то, что он делает. "2 месяца" это фейспалм.
Как там было "Поросеночек рос, рос и выросло ... Что выросло, то выросло!". Тем более когда человек нуждается в помощи по наведению порядка.
В нынешнем виде этот код обуза для проекта и тянет его ко дну. Включать в билд можно только если экономика требует. Иначе в папочку "музей".
Здравствуйте, landerhigh, Вы писали:
L>Заголовок с копирайтом позволяет сразу понять, что это, откуда оно взялось и кто в этом виноват. Даже когда работаешь из движущегося поезда без подключения к системе.
Работал однажды над легаси проектом. Там такое было принято. Причём когда файл кто-то редактировал, то про коммите изменений в шапку дописывалось примерно следующее:
<номер изменения> <автор> <сообщение> <дата>
Ну и что оно помогает понять? Особенно когда количество изменений переваливает за сотню (так и было), а кода который люди добавляли в файле уже и не осталось. Без системы контроля версий это просто нагромождение бессмысленной "информации".
К счастью, в какой-то момент от этого решили избавиться.
L>Я знаю. Это не отменяет того, что хорошей инженерной практикой является комментирование подобного. Даже если это всем известно, это не перестает быть грязным хаком.
Не уверен. А то так можно далеко зайти. Скажем, "pragma once" стандартом так и не стала. Тем не менее, (практически) все используют и сомневаюсь, что комментируют. А что? Тоже своего рода "нестандартный хак".
Здравствуйте, vpchelko, Вы писали:
V>Да и нотации типа m_, в новом коде я не приемлю, уже есть нормальные IDE.
m_ пишут не вместо подсветки, а чтобы не писать this-> если есть параметр или локальная переменная с тем же именем.
struct Foo {
int x;
void f(int x) {
this->x = x;
}
void g() {
...
int x = ...
...
this->x = x;
}
};
Здравствуйте, smeeld, Вы писали:
S>Здравствуйте, Kernan, Вы писали:
K>>Речь как бы не совсем про это была.
S>Короче, вместо try{ throw }catch() используем старый добрый do{ break; while(0) S>так правильней.
Нет, правильнее выделить сущности и использовать RAII, try-catch там вообще может быть не нужен. Если бы там были только математические вычисления то подход с do-break имел бы смысл.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
О, да! Руку мастера сразу видно ! Особенно тернарный оператор нравится (что? switch? enum? ну не надо усложнять такой компактный код!): GC>
DE>Работал однажды над легаси проектом. Там такое было принято. Причём когда файл кто-то редактировал, то про коммите изменений в шапку дописывалось примерно следующее: DE><номер изменения> <автор> <сообщение> <дата>
У нас тоже такое было. Это отросло из старых проектов, когда VCS либо не было, либо были в зачаточном состоянии.
DE>Ну и что оно помогает понять? Особенно когда количество изменений переваливает за сотню (так и было), а кода который люди добавляли в файле уже и не осталось. Без системы контроля версий это просто нагромождение бессмысленной "информации".
Шапка помогает отследить происхождение файла. Кто, когда, какой проект. Сразу понятно, есть ли смысл искать аффтара.
Нередко бывает, кто какой-то файл копируется из проекта в проект, во внтутренние тулзы, откуда он опять попадает в продакшен и т.п.
DE>К счастью, в какой-то момент от этого решили избавиться.
Наши тоже прекратили ручное отслеживание истории при переходе на VCS.
L>>Я знаю. Это не отменяет того, что хорошей инженерной практикой является комментирование подобного. Даже если это всем известно, это не перестает быть грязным хаком. DE>Не уверен. А то так можно далеко зайти. Скажем, "pragma once" стандартом так и не стала. Тем не менее, (практически) все используют и сомневаюсь, что комментируют. А что? Тоже своего рода "нестандартный хак".
Вещи вроде
#pragma once
обычно относят в coding conventions, где описывается, используются ли guard macro или эта прагма.
Здравствуйте, landerhigh, Вы писали:
L>У нас тоже такое было. Это отросло из старых проектов, когда VCS либо не было, либо были в зачаточном состоянии.
лично мне недостает (ибо на текущем месте никто не использует) той шапки, где VCS автоматом прописывает дату коммита, автора и ревизию (как тут: https://www.cs.utah.edu/dept/old/texinfo/cvs/cvs_16.html ). все же помогало ориентироваться в проекте и выявлять случаи, когда "ой, забыл твой фикс затащить" или "кто эту тут пошевелил код, вчера было по-другому": по шапке можно быстро понять неактуальность файла. внешние тулзы все же более неповоротливы. минусом только было сложность анализа диффа файлов для одного проекта в разных бранчах сторонними утилитами типа Araxis — дифф показывался для всех файлов из-за разницы в шапке.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Обычно именно сишники выдают такой С++ код. Или драйверисты.
В общем случае не лечится. Их период воспитывания вкуса пришелся на другую эпоху, для них подобный код действительно неплох. Можно дать им книжку типа Фаулера ("Рефакторинг") и надеятся, что привьются правильные подходы. Но не факт...
Проект Ребенок8020 — пошаговый гайд как сделать, вырастить и воспитать ребенка.
Здравствуйте, Abyx, Вы писали:
A>m_ пишут не вместо подсветки, а чтобы не писать this-> если есть параметр или локальная переменная с тем же именем. A>
A>struct Foo {
A> int x;
A> void f(int x) {
this->>x = x;
A> }
A> void g() {
A> ...
A> int x = ...
A> ...
this->>x = x;
A> }
A>};
A>
Я в таких случаях за использование this->x = x;
К тому же я еще пишу много на java — там такой стиль обще принят. Например автогенерация кода для getter/setter и конструкторов инициализирующих поля объекта.
Мне например очень удобно, когда код смотрится одинаково на C++ и на Java — да и других языках. Да и в принципе мне все равно какой язык использовать. Но эталон кодинг стайла — пока что Java.
код едва вытягивает на оценку "удовлетворительно", но шанс у этого кода есть — его исправить можно.
недостатки обсосали выше, но особенно "порадовало" использование тернарных операторов вкупе с плохим форматированием — такой код читать тяжко.
огромные методы, которые не понятно что делают, и которые можно читать с перекурами вызывают желание основательно отрефакторить этот код.
Всё сказанное выше — личное мнение, если не указано обратное.
Здравствуйте, omgOnoz, Вы писали:
A>>m_ пишут не вместо подсветки, а чтобы не писать this-> если есть параметр или локальная переменная с тем же именем. A>>
A>> void f(int x) {
A>> this->x = x;
A>> }
A>>
O>Я в таких случаях за использование this->x = x;
и если ты забудешь написать this-> то получишь трудно отлавливаемый баг.
Здравствуйте, MTD, Вы писали:
MTD>Здравствуйте, Andrew.W Worobow, Вы писали:
AWW>>мало коментариев.
MTD>В хорошем коде комментариев быть не должно.
Не согласен,
например мы реализовали какой-то сложный математический
метод, нужен хотя бы комментарий, типа метод такой-то, см. статью такую-то,
а лучше бы еще объяснение почему выбран именно метод (хотя это можно и в документацию
засунуть).
Иными словами как бы хорош код не был, всегда существуют такие алгоритмы,
которые сложно понять имея только код.
A>>потери истории изменений и т.п. случаются, но такое бывает ну очень редко.
L>Достаточно одного раза в 10 лет, и каждый раз придется заново проводить расследование "откуда вообще мы это взяли".
Здравствуйте, Zhendos, Вы писали:
A>>>потери истории изменений и т.п. случаются, но такое бывает ну очень редко.
L>>Достаточно одного раза в 10 лет, и каждый раз придется заново проводить расследование "откуда вообще мы это взяли".
Z>Бэкапы делать пробовали?
А подумать, что означает "потеря истории изменений", как она происходит и как и когда обнаруживается, и каким боком тут бекапы, подумать не пробовали, прежде чем написать?
Здравствуйте, Zhendos, Вы писали:
Z>например мы реализовали какой-то сложный математический Z>метод, нужен хотя бы комментарий, типа метод такой-то, см. статью такую-то,
Это единственный случай когда в хорошем коде комментарий уместен.
Здравствуйте, MTD, Вы писали:
Z>>например мы реализовали какой-то сложный математический Z>>метод, нужен хотя бы комментарий, типа метод такой-то, см. статью такую-то,
MTD>Это единственный случай когда в хорошем коде комментарий уместен.
Не, далеко не единственный. Есть куча ситуаций, где недостаточно внимательный разработчик может нарваться на неприятности. Я, например, всегда комментирую "проваливающиеся" (без break) ветки в switch, или места, где важен порядок объявления членов класса. Со сторонними библиотеками тоже всяких приколов хватает. Вообще, нарвался на неожиданность — прокомментируй. Не понял чего-то быстро (засомневался, а все ли тут делается корректно и вынужден перепроверять, к примеру) в своем же коде через год после написания — прокомментируй, по свежим впечатлениям.
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, MTD, Вы писали:
MTD>Это единственный случай когда в хорошем коде комментарий уместен.
"Хороший код" иллюстрирует максимум только то, что он делает. Он не может описать того, что он не делает, и почему он этого — ещё или уже — не делает.
Всегда есть множество вариантов сделать одно и то же. И простое предъявление одного из вариантов не говорит о том, почему именно ему отдано предпочтение перед другими. Планируется какой-то рефакторинг, учтены какие-то ограничения одного из компиляторов, воркэраунд вокруг недокументированного поведения сторннего API, etc, etc, etc. "В хорошем коде комментариев быть не должно" — это весьма популярный, яркий, лаконичный, ёмкий и совершенно неправильный принцип. Используется в основном теоретиками и прочими гуманитариями.
Здравствуйте, Qbit86, Вы писали:
Q>"Хороший код" иллюстрирует максимум только то, что он делает. Он не может описать того, что он не делает, и почему он этого — ещё или уже — не делает.
Действительно, почему функция OpenFile не форматирует диск? Непорядок, надо фичу добавить, но аккуратно задокументировать!
Q>Используется в основном теоретиками и прочими гуманитариями.
А обвинения в увлечении теорией, по моему опыту, звучат в основном от любителей трясти, а не думать.
Здравствуйте, Хон Гиль Дон, Вы писали:
ХГД>Есть куча ситуаций, где недостаточно внимательный разработчик может нарваться на неприятности. Я, например, всегда комментирую "проваливающиеся" (без break) ветки в switch, или места, где важен порядок объявления членов класса.
Перепиши код проще, чтобы недостаточно внимательный разработчик не нарвался — гарантии, что он прочтет твой комментарий нет.
ХГД>Со сторонними библиотеками тоже всяких приколов хватает. Вообще, нарвался на неожиданность — прокомментируй.
Здравствуйте, MTD, Вы писали:
Q>>"Хороший код" иллюстрирует максимум только то, что он делает. Он не может описать того, что он не делает, и почему он этого — ещё или уже — не делает. MTD>Действительно, почему функция OpenFile не форматирует диск? Непорядок, надо фичу добавить, но аккуратно задокументировать!
OpenFile? Что ещё за OpenFile? Почему в этом месте разраюотчик решил открыть какой-то файл? Этот код может же выполняться на мобильной платформе, где нельзя просто так взять, и открыть файл. Или не будет выполняться? Зачем открывать какие-то файлы, если в любом языке и библиотеке есть примитивы для работы с XML или другими форматами данных? Я программировал на C++ и C# под десктоп, iOS и Android. И везде OpenFile почти наверняка вызвал бы вопросы.
MTD>А обвинения в увлечении теорией, по моему опыту, звучат в основном от любителей трясти, а не думать.
Если человек не может вообразить примеры, когда полезны комментарии в коде — значит, он чужой код не читает. Может, он джедай-одиночка; свой-то код, понятно, "всегд хороший и не нуждается в комментариях".
Здравствуйте, Хон Гиль Дон, Вы писали:
ХГД> Я, например, всегда комментирую "проваливающиеся" (без break) ветки в switch, или места, где важен порядок объявления членов класса.
Ага, классный принцип — закладывать мины и обставлять их флажками
Здравствуйте, MTD, Вы писали:
Z>>например мы реализовали какой-то сложный математический Z>>метод, нужен хотя бы комментарий, типа метод такой-то, см. статью такую-то,
MTD>Это единственный случай когда в хорошем коде комментарий уместен.
Не совсем. IMHO, при развитии/поддержке очень полезны комментарии, отвечающе не на вопрос "что делает код",
а на вопрос "почему/зачем это делается" или "почему это делается именно так, а не иначе".
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
лютый трындец. write-only. надо переделать чтоб был единый code design style — префиксы переменных, какие отступы у скопов и так далее, комментарии должны быть комментариями, а не отладочным выводом и функции типа equal_values не должны иметь параметров, которые не используются никогда.
Функция которая открывает файл.
Q>Почему в этом месте разраюотчик решил открыть какой-то файл?
Действительно, зачем в методе ReadConfigFromFile (ПрочестьКонфигурациюИзФайла) разработчику нужно открыть файл? Непонятно! Надо чтобы диск форматировался и само-собой оставить об этом комментарий.
Q>Этот код может же выполняться на мобильной платформе
Придумай что-нибудь оригинальней, передергивания стали скучными.
Q>Если человек не может вообразить примеры, когда полезны комментарии в коде — значит, он чужой код не читает. Может, он джедай-одиночка; свой-то код, понятно, "всегд хороший и не нуждается в комментариях".
Я читал очень много кода, был код который читался замечательно без комментариев, было много говнокода "отлично" прокомментированного:
file f = open("xxx") // Открываем файл
x = read(f, 5) // Читаем из файла 5 байт
А еще, попадался код с историей в десяток лет, где изначально осмысленные комментарии устаревали и порой вообще вводили в заблуждение.
Здравствуйте, MTD, Вы писали:
Q>>"Хороший код" иллюстрирует максимум только то, что он делает. Он не может описать того, что он не делает, и почему он этого — ещё или уже — не делает.
MTD>Действительно, почему функция OpenFile не форматирует диск? Непорядок, надо фичу добавить, но аккуратно задокументировать!
OpenFile — очень хороший пример функции, документирование которой строго обязательно. Описание, какие режимы можно использовать, на каких платформах (если поддерживает больше одной) и файловых системах и как они будут работать, а также прямое описание ошибочных ситуаций.
Более того, во многих случаях необходимо документировать также и использование оной функции. Чтобы указать, почему используется именно этот режим открытия, или почему тут совершенно необходимо использование юникодной версии и префикса "\\?\" и т.д.
Здравствуйте, landerhigh, Вы писали:
L>OpenFile — очень хороший пример функции, документирование которой строго обязательно. Описание, какие режимы можно использовать, на каких платформах (если поддерживает больше одной) и файловых системах и как они будут работать, а также прямое описание ошибочных ситуаций.
L>Более того, во многих случаях необходимо документировать также и использование оной функции. Чтобы указать, почему используется именно этот режим открытия, или почему тут совершенно необходимо использование юникодной версии и префикса "\\?\" и т.д.
это какая-то утопия, имхо =)
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, niXman, Вы писали:
L>>Более того, во многих случаях необходимо документировать также и использование оной функции. Чтобы указать, почему используется именно этот режим открытия, или почему тут совершенно необходимо использование юникодной версии и префикса "\\?\" и т.д.
X>это какая-то утопия, имхо =)
Отнюдь. Харрасмент, буллинг и геноцид, ковровые бомбардировки и обыкновенный фашизм во время код ревью творят живительные чудеса. Через полгода подопытные (если не уволились раньше) даже перестают воспринимать критику кода как личное оскорбление.
Здравствуйте, landerhigh, Вы писали:
L>Отнюдь. Харрасмент, буллинг и геноцид, ковровые бомбардировки и обыкновенный фашизм во время код ревью творят живительные чудеса. Через полгода подопытные (если не уволились раньше) даже перестают воспринимать критику кода как личное оскорбление.
я согласен с тем, что такой код хорошо и приятно поддерживать. но я сомневаюсь что такое возможно в нашей-то реальности
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, niXman, Вы писали:
X>я согласен с тем, что такой код хорошо и приятно поддерживать. но я сомневаюсь что такое возможно в нашей-то реальности
К совершенству надо стремиться. Даже если оно недостижимо. Иначе бренность бытия поглотит и само существование станет бессмысленным
Здравствуйте, landerhigh, Вы писали:
L>К совершенству надо стремиться. Даже если оно недостижимо. Иначе бренность бытия поглотит и само существование станет бессмысленным
да. об этом я совсем забыл =)
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, landerhigh, Вы писали:
L>Отнюдь. Харрасмент, буллинг и геноцид, ковровые бомбардировки и обыкновенный фашизм во время код ревью творят живительные чудеса. Через полгода подопытные (если не уволились раньше) даже перестают воспринимать критику кода как личное оскорбление.
расскажите практические примеры, у меня фантазия закончилась на угрозах семье, но код стал реально чище
Здравствуйте, landerhigh, Вы писали:
L>OpenFile — очень хороший пример функции, документирование которой строго обязательно. Описание, какие режимы можно использовать
Хороший код сам является в значительной мере документацией. Можно обозвать метод open и сказать, что он открывает вообще все, детали смотрите в документации. А можно сделать классы File, TcpSocket, SerialPort с методами Open. Затем, метод Open класса File может две строки — путь к файлу и режим открытия, тут без документации не разобраться. А может принимать enum Mode { ReadOnly, WriteOnly, ... } и объект типа FilePath, который представляет собой путь к файлу.
L>на каких платформах (если поддерживает больше одной)
Сова затрещала. Нормальная библиотека имеет список поддерживаемых платформ — вот ответ. Библиотека которая ведет себя по разному на разных платформах отправляется в топку.
L>и файловых системах и как они будут работать
Ага и на каких дисках, например WD-only, тебе же голову больше занять нечем. Файл — понятие высокоуровневое, забивать себе голову файловыми системами, секторами и блоками как-то неадекватно.
L>а также прямое описание ошибочных ситуаций.
Это да. Только речь то шла о комментариях, документирование отдельная большая тема.
MTD>Хороший код сам является в значительной мере документацией. Можно обозвать метод open и сказать, что он открывает вообще все, детали смотрите в документации. А можно сделать классы File, TcpSocket, SerialPort с методами Open. Затем, метод Open класса File может две строки — путь к файлу и режим открытия, тут без документации не разобраться. А может принимать enum Mode { ReadOnly, WriteOnly, ... } и объект типа FilePath, который представляет собой путь к файлу.
Ага, а как ты узнаешь поведение в случае, если ты хочешь открыть файл для чтения, а он — ридонли? Из документации только, как ты не расписывай API.
L>>на каких платформах (если поддерживает больше одной)
MTD>Сова затрещала. Нормальная библиотека имеет список поддерживаемых платформ — вот ответ. Библиотека которая ведет себя по разному на разных платформах отправляется в топку.
Еще раз — разные платформы имеют право вести себя по-разному. Это обязано быть отражено в документации.
L>>и файловых системах и как они будут работать
MTD>Ага и на каких дисках, например WD-only, тебе же голову больше занять нечем. Файл — понятие высокоуровневое, забивать себе голову файловыми системами, секторами и блоками как-то неадекватно.
Сектора и блоки ты сам сюда притащил. Это называется "довести до абсурда".
А вот про системы, в которых файлов вообще может не быть, тебе уже сказали
Здравствуйте, RonWilson, Вы писали:
RW>расскажите практические примеры, у меня фантазия закончилась на угрозах семье, но код стал реально чище
Фи, как грубо.
Тоньше надо действовать, тоньше. Для разработчика что самое страшное? Что его код могут НЕ ДОПУСТИТЬ К КОММИТУ!!!
Вот отсюда и надо плясать
Здравствуйте, landerhigh, Вы писали:
L>Для разработчика что самое страшное? Что его код могут НЕ ДОПУСТИТЬ К КОММИТУ!!!
ой не факт.
знаю я одного, для которого самое страшное было в том, если его код для обсуждения опубликуют на каких-то форумах
пачка бумаги А4 стОит 2000 р, в ней 500 листов. получается, лист обычной бумаги стОит дороже имперского рубля =)
Здравствуйте, landerhigh, Вы писали:
L>Ага, а как ты узнаешь поведение в случае, если ты хочешь открыть файл для чтения, а он — ридонли?
Сильно подозреваю, что он откроется нормально и его можно будет прочесть.
L>Из документации только, как ты не расписывай API.
Что значит расписывай API? При чем тут документация, если мы комментарии в коде обсуждали?
L>Еще раз — разные платформы имеют право вести себя по-разному. Это обязано быть отражено в документации.
Нет. Кроссплатформенная библиотека — абстракция над разными платформами, вести себя она должна одинаково иначе не понятно, зачем она нужна, проще нативные библиотеки использовать.
L>Сектора и блоки ты сам сюда притащил. Это называется "довести до абсурда".
Ты с этого начал, я просто сказал, что логично ожидать, что функция с названием OpenFile не будет форматировать диск.
L>А вот про системы, в которых файлов вообще может не быть, тебе уже сказали
И что? Давай развивай тему, поговорим о системах с памятью в 4 Кб, о системах без мониторов, с вотчдогами, с 9 битами в байте. Разное бывает, дальше то что?
Здравствуйте, MTD, Вы писали:
L>>Ага, а как ты узнаешь поведение в случае, если ты хочешь открыть файл для чтения, а он — ридонли?
MTD>Сильно подозреваю, что он откроется нормально и его можно будет прочесть.
Сколько самолетов упало и сколько педалей газа заклинило из-за вот такого "сильно подозреваю"...
L>>Из документации только, как ты не расписывай API.
MTD>Что значит расписывай API? При чем тут документация, если мы комментарии в коде обсуждали?
Совершенно верно. Коменты в коде. Каждый юнит кода должен обосновывать свое существование, описывать, как он работает и почему он работает именно так, а не иначе и сообщать всю необходимую для его использования информацию.
L>>Еще раз — разные платформы имеют право вести себя по-разному. Это обязано быть отражено в документации.
MTD>Нет. Кроссплатформенная библиотека — абстракция над разными платформами, вести себя она должна одинаково иначе не понятно, зачем она нужна, проще нативные библиотеки использовать.
поведение файловой системы определяется платформой. Кроссплатформенная библиотека не всегда может или имеет право это поведение менять.
L>>Сектора и блоки ты сам сюда притащил. Это называется "довести до абсурда".
MTD>Ты с этого начал, я просто сказал, что логично ожидать, что функция с названием OpenFile не будет форматировать диск.
А вот что она будет делать при попытке открытия файла, которого нет? тут вариантов до чертиков и все они вполне логичные.
Здравствуйте, landerhigh, Вы писали:
L>>>Ага, а как ты узнаешь поведение в случае, если ты хочешь открыть файл для чтения, а он — ридонли?
MTD>>Сильно подозреваю, что он откроется нормально и его можно будет прочесть.
L>Сколько самолетов упало и сколько педалей газа заклинило из-за вот такого "сильно подозреваю"...
Во-первых, при разработке самолетов требования особые, если так разрабатывать обычный софт то он будет стоить неприемлемо дорого.
Во-вторых, поделись, что должно по-твоему произойти при открытии рид-онли файла на чтение?
В-третьих, проблемы надежности комментарии вообще никак не решают.
L>Совершенно верно. Коменты в коде.
Документация — это не комменты в коде.
L>Каждый юнит кода должен обосновывать свое существование, описывать, как он работает и почему он работает именно так, а не иначе и сообщать всю необходимую для его использования информацию.
Если цель осваивать бюджеты при нулевом выхлопе без такого не обойтись.
L>поведение файловой системы определяется платформой. Кроссплатформенная библиотека не всегда может или имеет право это поведение менять.
Значит библиотека не смогла предоставить адекватную абстракцию, то не нужна такая библиотека, надо использовать нативные.
L>А вот что она будет делать при попытке открытия файла, которого нет? тут вариантов до чертиков и все они вполне логичные.
Здравствуйте, MTD, Вы писали:
Z>>например мы реализовали какой-то сложный математический Z>>метод, нужен хотя бы комментарий, типа метод такой-то, см. статью такую-то, MTD>Это единственный случай когда в хорошем коде комментарий уместен.
Единственный? Завидую, но не верю.
Почти любая оптимизация нуждается в комментариях почему она ничего не ломает (это часто "нелогичное" отсутствие лишних проверок, локов и тыдытыпы).
Любой обход (workaround) нуждается в комментариях что именно мы чиним (железо? косяк чужой библиотеки?).
Кусок какого нибудь протокола (сеть? ipc?) с подозрением на нелогичность требует комментариев.
Иногда бывает что кажется будто есть более простые/логичные варианты реализации функции, комментарий почему сделано так, а не иначе — нужен.
Ну, понятно, что комментировать в стиле void *p = malloc(256); /* Allocates memory */ нафиг не надо.
Здравствуйте, landerhigh, Вы писали:
aik>>Ну, понятно, что комментировать в стиле void *p = malloc(256); /* Allocates memory */ нафиг не надо. L>Даже здесь нужен комментарий, объясняющий почему void, и почему именно maloc и что это за магическая константа.
Неа, вот тут как раз и не нужен, потому что код должен быть типа:
и никаких комментариев. И почему именно malloc — надо писать если это c++ или glibc (а лучше заменять на new/g_malloc), а если голый си + libc — то зачем это комментировать?
Здравствуйте, MTD, Вы писали:
L>>Сколько самолетов упало и сколько педалей газа заклинило из-за вот такого "сильно подозреваю"...
MTD>Во-первых, при разработке самолетов требования особые, если так разрабатывать обычный софт то он будет стоить неприемлемо дорого.
Сказки. При разработке самолетов обкладывание кода бумажками совершенно особое, но документацию разработчики читают самую обычную.
MTD>Во-вторых, поделись, что должно по-твоему произойти при открытии рид-онли файла на чтение?
Конечно, я хотел написать "открытие ридонли файла на чтение и запись". И вот что при этом произодйет, и должна объяснять документация к коду
MTD>В-третьих, проблемы надежности комментарии вообще никак не решают.
L>>Совершенно верно. Коменты в коде.
MTD>Документация — это не комменты в коде.
Это. Коменты в коде — это документация к коду. Если у вас иначе, то мне вас жаль.
L>>Каждый юнит кода должен обосновывать свое существование, описывать, как он работает и почему он работает именно так, а не иначе и сообщать всю необходимую для его использования информацию.
MTD>Если цель осваивать бюджеты при нулевом выхлопе без такого не обойтись.
То-то нулевой выхлоп при диких бюджетах наблюдается в основном при подходе "некогда документацию писать, копать надо"
Здравствуйте, landerhigh, Вы писали:
L>Сказки. При разработке самолетов обкладывание кода бумажками совершенно особое, но документацию разработчики читают самую обычную.
А ты для самолетов что писал? Вот мне довелось немного пописать критически важный код, там подходы к надежности совсем другие, например, софт пишут 3 независимые команды, затем в зависимости от входных данных 3 разные программы выдают результат, действие производится только если совпали все 3 результата, иначе чрезвычайная ситуация. Комментарии в коде безопасности никак не добавляют, гораздо лучше, например, ассертами описывать все контракты.
MTD>>Документация — это не комменты в коде.
L>Это. Коменты в коде — это документация к коду. Если у вас иначе, то мне вас жаль.
Покажи свой код, дабы не быть голословным. Доксигеновские описания — это никак не комментарии в коде, кроме того, для адекватной документации их мало.
L>>>Каждый юнит кода должен обосновывать свое существование, описывать, как он работает и почему он работает именно так, а не иначе и сообщать всю необходимую для его использования информацию.
MTD>>Если цель осваивать бюджеты при нулевом выхлопе без такого не обойтись.
L>То-то нулевой выхлоп при диких бюджетах наблюдается в основном при подходе "некогда документацию писать, копать надо"
Qbit86 меня обвинил в увлечении теории, когда я сказал, что комментарии в коде бардака не решат и надо думать как и что писать. Ты ударился в другую крайность — описывать вообще все. Извини, это ты менеджерам рассказывай, что все красиво оформлял, поэтому показать нечего, а я за взвешенный подход.
И не уходи от вопроса с которого началась дискуссия — адекватно функции открытия файла диск форматировать?
Говорить что код без комментариев плохой, также неверно как и говорить что комментарии не нужны.
Комментарии не всегда идут на пользу, точно также как не всегда оправданно и их отсутствие.
Неоднократно встречал как из комментариев делают карго-культ.
Например, возле пустого деструктора, который был абсолютно не нужен, стояло "это деструктор, он вызывается в конце жизни объекта" — и такого рода мусора по всему коду, перед каждой функцией, классом и строчкой кода, без всякой на то причины, аккуратненько так.
Такого рода комментарии как минимум раздражают, так как ожидаешь что-то осмысленное, несущее важную информацию — а там вода.
Комментарий это, очевидно, дополнительная сущность, которая отвлекает внимание, за которой нужно следить, держать в синхронизации с кодом.
Если комментарий не добавляет никакой новой информации способствующей пониманию кода, в то время как всё и так моментально ясно после беглого взгляда — то такой комментарий приносит вред.
Более того, нужно искать варианты замены комментария более простым и выразительным кодом. Тривиальный и классический пример — литерал с комментарием заменить константой с говорящим именем.
В то же время, при реализации каких-то нетривиальных концепций, с которыми читающий не обязан быть знаком, комментарии действительно необходимы.
В общем, как и всегда, нужно искать баланс и не допускать перекосы в ту или иную сторону
L>>Сказки. При разработке самолетов обкладывание кода бумажками совершенно особое, но документацию разработчики читают самую обычную.
MTD>А ты для самолетов что писал?
Прежде чем доставать линейку, предлагаю тебе подумать еще раз. А потом еще раз. А потом еще. Иначе конфуз получится.
MTD>Комментарии в коде безопасности никак не добавляют, гораздо лучше, например, ассертами описывать все контракты.
Имя того критически важного софта не затруднит привести? С ассертами.
MTD>>>Документация — это не комменты в коде.
L>>Это. Коменты в коде — это документация к коду. Если у вас иначе, то мне вас жаль.
MTD>Покажи свой код, дабы не быть голословным.
Коммерческая тайна.
MTD>Доксигеновские описания — это никак не комментарии в коде, кроме того, для адекватной документации их мало.
Значит, вы их готовить не умеете. Обычная ситуация, впрочем.
L>>>>Каждый юнит кода должен обосновывать свое существование, описывать, как он работает и почему он работает именно так, а не иначе и сообщать всю необходимую для его использования информацию.
MTD>>>Если цель осваивать бюджеты при нулевом выхлопе без такого не обойтись.
L>>То-то нулевой выхлоп при диких бюджетах наблюдается в основном при подходе "некогда документацию писать, копать надо"
MTD>Qbit86 меня обвинил в увлечении теории, когда я сказал, что комментарии в коде бардака не решат и надо думать как и что писать. Ты ударился в другую крайность — описывать вообще все. Извини, это ты менеджерам рассказывай, что все красиво оформлял, поэтому показать нечего, а я за взвешенный подход.
Это ты тут чего-то навыдумывал, приписал это собеседникам и теперь героически со своими же страхами борешься. Для начала потрудись доказать, что "красиво офомленный код" — это "нечего показать", что ли.
MTD>И не уходи от вопроса с которого началась дискуссия — адекватно функции открытия файла диск форматировать?
Не знаю, может быть для вашего свех-критического софта и адекватно
Здравствуйте, Evgeny.Panasyuk, Вы писали:
EP>Например, возле пустого деструктора, который был абсолютно не нужен, стояло "это деструктор, он вызывается в конце жизни объекта" — и такого рода мусора по всему коду, перед каждой функцией, классом и строчкой кода, без всякой на то причины, аккуратненько так.
Так бывает, когда требование о комментировании спускается сверху, а у разрабов нет даже зачатков инженерной культуры, чтобы понимать, зачем вообще нужны комментарии. И отсутствует лидер, который пряником, кнутом, огнем, мечом и инквизицией им эти зачатки привьет.
Я же сразу отметил бестолковость комментариев, присутствующих в коде, приведенном ТС и полное отсутствие комментариев там, где надо.
такая же история, кстати, случается и с юнит-тестами.
(хотя я сам комментирую объявление pure virtual destructor. На разный лад, а с определенной поры и на разных языках. Это моя фирменная фишка)
EP>Такого рода комментарии как минимум раздражают, так как ожидаешь что-то осмысленное, несущее важную информацию — а там вода.
EP>Комментарий это, очевидно, дополнительная сущность, которая отвлекает внимание, за которой нужно следить, держать в синхронизации с кодом. EP>Если комментарий не добавляет никакой новой информации способствующей пониманию кода, в то время как всё и так моментально ясно после беглого взгляда — то такой комментарий приносит вред.
Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно). Очень хорошо написать, какие паттерны используются. Хороший комментарий в заголовке класса убирает необходимость заглядывать в определение методов от слова совсем.
Здравствуйте, landerhigh, Вы писали:
MTD>>А ты для самолетов что писал?
L>Прежде чем доставать линейку, предлагаю тебе подумать еще раз. А потом еще раз. А потом еще. Иначе конфуз получится.
Это ты все мериться пытаешься, а я просто поинтересовался есть ли у тебя такой опыт. Так что думай.
L>Имя того критически важного софта не затруднит привести? С ассертами.
Билибинская, Курская, Ростовская АЭС. Но ассерты не для тех кто свято верит, что комментарии спасут от всего. Я же предпочитаю, что программа аварийно завершиться в случае нарушения контракта, чем будет функционировать непредсказуемо.
MTD>>Покажи свой код, дабы не быть голословным.
L>Коммерческая тайна.
Быстро слился.
MTD>>Доксигеновские описания — это никак не комментарии в коде, кроме того, для адекватной документации их мало.
L>Для начала потрудись доказать, что "красиво офомленный код" — это "нечего показать", что ли.
Будь добр, читай что пишешь, ничего не понятно.
MTD>>И не уходи от вопроса с которого началась дискуссия — адекватно функции открытия файла диск форматировать?
L>Не знаю, может быть для вашего свех-критического софта и адекватно
Здравствуйте, landerhigh, Вы писали:
L>Я же сразу отметил бестолковость комментариев, присутствующих в коде, приведенном ТС и полное отсутствие комментариев там, где надо.
Кстати да, я как раз о подобном и говорил — "CSewingDlg(CWnd* pParent = NULL); // стандартный конструктор".
L>такая же история, кстати, случается и с юнит-тестами.
Да, там тоже карго-культ цветёт и пахнет в виде каких-то формальных отписок.
L>(хотя я сам комментирую объявление pure virtual destructor. На разный лад, а с определенной поры и на разных языках. Это моя фирменная фишка)
Это, естественно, другого рода ситуация.
L>Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
Скажем так, мне трудно представить чем может быть вреден подобный комментарий для нетривиальных классов. Часто такой комментарий действительно оправдан, и даже необходим.
L>Очень хорошо написать, какие паттерны используются.
Смотря какого уровня класс, и что за паттерн. Например, комментировать что FooVisitor это "Visitor pattern" — избыточно, а вот сказать что обработка событий реализована как Proactor — уж точно ничем не повредит.
L>Хороший комментарий в заголовке класса убирает необходимость заглядывать в определение методов от слова совсем.
Кстати, немного offtopic — предпочитаю данные класса помещать в начало описания. Зачастую полезно знать не просто что делает класс, а представлять как именно он реализован, например чтобы оценить производительность/потребление ресурсов. Часто, краем глаза взглянув на структуру данных класса, сразу становится понятно что именно за птица и как она летает.
Здравствуйте, landerhigh, Вы писали:
L>Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
Имя класса должно быть выбрано удачно, тогда будет очевидно зачем он нужен.
Здравствуйте, MTD, Вы писали:
MTD>Это ты все мериться пытаешься, а я просто поинтересовался есть ли у тебя такой опыт. Так что думай.
L>>Имя того критически важного софта не затруднит привести? С ассертами.
MTD>Билибинская, Курская, Ростовская АЭС. Но ассерты не для тех кто свято верит, что комментарии спасут от всего. Я же предпочитаю, что программа аварийно завершиться в случае нарушения контракта, чем будет функционировать непредсказуемо.
Здравствуйте, Evgeny.Panasyuk, Вы писали:
L>>Я же сразу отметил бестолковость комментариев, присутствующих в коде, приведенном ТС и полное отсутствие комментариев там, где надо. EP>Кстати да, я как раз о подобном и говорил — "CSewingDlg(CWnd* pParent = NULL); // стандартный конструктор".
+
L>>такая же история, кстати, случается и с юнит-тестами. EP>Да, там тоже карго-культ цветёт и пахнет в виде каких-то формальных отписок.
и метрик вроде "покрытия кода тестами".
L>>Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
EP>Скажем так, мне трудно представить чем может быть вреден подобный комментарий для нетривиальных классов. Часто такой комментарий действительно оправдан, и даже необходим.
Есть еще важные вещи, которые зачастую не замечают или не придают им значения. Например, берет ли класс владение неким объектом или нет, а если нет, то должно ли гарантироваться время жизни этого объекта и как. Еще пример — случается, что в конструктор передается некий умный указатель, но он используется исключительно в конструкторе и не сохраняется в данных класса. Подобные вещи лучше всего добавлять в комментарии, чтобы минимизировать количество сюрпризов при использовании.
L>>Очень хорошо написать, какие паттерны используются.
L>>Хороший комментарий в заголовке класса убирает необходимость заглядывать в определение методов от слова совсем.
EP>Кстати, немного offtopic — предпочитаю данные класса помещать в начало описания. Зачастую полезно знать не просто что делает класс, а представлять как именно он реализован, например чтобы оценить производительность/потребление ресурсов. Часто, краем глаза взглянув на структуру данных класса, сразу становится понятно что именно за птица и как она летает.
ну, это уже дело привычки, и иногда может идти в разрез с принятыми в компании code conventions.
Здравствуйте, landerhigh, Вы писали:
L>Есть еще важные вещи, которые зачастую не замечают или не придают им значения. Например, берет ли класс владение неким объектом или нет, а если нет, то должно ли гарантироваться время жизни этого объекта и как.
Есть ряд типичных случаев, для которых есть общепринятые соглашения. В таких случаях комментарии не нужны.
А вот, например, если конструктор/метод берёт const Foo&, и сохраняет указатель в поле класса, то это нужно как-то обязательно обозначить.
L>Еще пример — случается, что в конструктор передается некий умный указатель, но он используется исключительно в конструкторе и не сохраняется в данных класса.
В таком случае нужно брать не умный указатель, а ссылку. Во-первых это бесплатно расширит область применимости, а во-вторых сделает комментарий необязательным (хотя и вреда он не принесёт).
L>Подобные вещи лучше всего добавлять в комментарии, чтобы минимизировать количество сюрпризов при использовании.
А вообще, да, нужно комментировать всякие неявные места, а иногда и явные, но не всем очевидные.
Здравствуйте, MTD, Вы писали:
L>>Ну вот надо классу одновременно быть и обсервером и посетителем. Выбирай имя. Только чтобы удачно.
MTD>Зачем? Я инженер, опиши проблему, попробую помочь.
Есть класс, который должен следить за обновлениями кеша. Для этого он подписывается на соответствующее событие, т.е. является обсервером.
Задача класса — когда скажут, обновить значения в некоей таблице, где он посетит каждую ячейку.
Придумывай название.
L>>Когда же уже сделают кнопочку "фейспалм"?
MTD>Конструктивно.
Здравствуйте, landerhigh, Вы писали:
L>Есть класс, который должен следить за обновлениями кеша. Для этого он подписывается на соответствующее событие, т.е. является обсервером. L>Задача класса — когда скажут, обновить значения в некоей таблице, где он посетит каждую ячейку.
L>Придумывай название.
Зачем дополнительная сущность, зачем нарушение инкапсуляции? Почему бы не подписать таблицу на событие и она сама об этом не сообщит своим ячейкам в простом цикле?
L>>Придумывай название.
MTD>Зачем дополнительная сущность, зачем нарушение инкапсуляции? Почему бы не подписать таблицу на событие и она сама об этом не сообщит своим ячейкам в простом цикле?
Считай, что так нужно. Может, в целях оптимизации производительности, может, потому что сама таблица не может обработать это событие. т.к. требуются еще другие данные.
Здравствуйте, landerhigh, Вы писали:
L>Считай, что так нужно.
Увы, не аргумент.
L>Может, в целях оптимизации производительности
Не вижу в приведенном примере простора для оптимизации, если только этот класс не производит фильтрацию лишних событий об обновлении, тогда название становиться очевидным и да, в этом случаее ему все равно не нужно знание внутреннего устройства таблицы.
L>может, потому что сама таблица не может обработать это событие. т.к. требуются еще другие данные.
Аргумент. Фантазия-то присутствет вообще? Например, класс таблицы неизменяем и не может сам подписаться. Живет в отдельном ActiveX. Или требуется недетская фильтрация, как ты уже догадался. Или требуется собирать данные из нескольких источников. Да мало ли что еще.
L>>Может, в целях оптимизации производительности
MTD>Не вижу в приведенном примере простора для оптимизации, если только этот класс не производит фильтрацию лишних событий об обновлении, тогда название становиться очевидным и да, в этом случаее ему все равно не нужно знание внутреннего устройства таблицы.
L>>может, потому что сама таблица не может обработать это событие. т.к. требуются еще другие данные.
MTD>Значит надо передавать данные вместе с событием.
Здравствуйте, landerhigh, Вы писали:
L>Аргумент. Фантазия-то присутствет вообще?
Я не фантазер, оперирую строго фактами.
L>Например, класс таблицы неизменяем и не может сам подписаться. Живет в отдельном ActiveX.
Adapter
L>Или требуется недетская фильтрация, как ты уже догадался.
Filter
L>Или требуется собирать данные из нескольких источников.
Agregator
L>Да мало ли что еще.
Это разные классы с разной ответственностью. Немного отвлекись от документирования сферических коней и порефактори, в результате у тебя получится несколько классов с названиями отражающими то, чем они занимаются и ты сможешь из них выстроить цепочку, код станет понятней, более удобным в поддержке и менее подверженным багам. ProactorVisitorObserverStateCacheManipulator — это просто ужас, не надо так делать.
MTD>>Значит надо передавать данные вместе с событием.
L>Это не всегда возможно/оправдано.
Надо смотреть, но ProactorVisitorObserverStateCacheManipulator однозначно ни в какие ворота.
L>Название-то придумал?
Здравствуйте, Basil2, Вы писали:
B>Обычно именно сишники выдают такой С++ код. Или драйверисты.
Это не сишник.
Судя по коду, его автор долго писал числодробилки на Фортране.
Думаю, что если бы обсуждаемый код был написан на Фортране 77 или даже Фортране 4, вопросов было бы гораздо меньше.
ХГД>> Я, например, всегда комментирую "проваливающиеся" (без break) ветки в switch, или места, где важен порядок объявления членов класса.
bnk>Ага, классный принцип — закладывать мины и обставлять их флажками
Ну а какие варианты, если в классе вдруг нужны 3-5 зависимых друг от друга членов? Двухстадийная инициализация еще хуже, запихивать их в unique_ptr — ваще отстой. Так что это не мины, а вполне легитимные фичи языка.
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, MTD, Вы писали:
MTD>Это разные классы с разной ответственностью. Немного отвлекись от документирования сферических коней и порефактори, в результате у тебя получится несколько классов с названиями отражающими то, чем они занимаются и ты сможешь из них выстроить цепочку, код станет понятней, более удобным в поддержке и менее подверженным багам. ProactorVisitorObserverStateCacheManipulator — это просто ужас, не надо так делать.
Теоретег. А что, если этот класс, котороый одновременно и визитор, и обсервер и еще кое-что, и есть результат рефакторинга? Просто потому, что так оказалось удобнее и логичнее?
L>>Название-то придумал?
MTD>Зачем? Мой вердикт — рефакторить.
То есть не смог придумать нормальное название — рефакторить?
Здравствуйте, MTD, Вы писали:
ХГД>>Есть куча ситуаций, где недостаточно внимательный разработчик может нарваться на неприятности. Я, например, всегда комментирую "проваливающиеся" (без break) ветки в switch, или места, где важен порядок объявления членов класса.
MTD>Перепиши код проще, чтобы недостаточно внимательный разработчик не нарвался — гарантии, что он прочтет твой комментарий нет.
Эээ, ты правда считаешь, что код, полагающийся на гарантированный стандартом языка порядок инициализации членов класса, обязательно сложный?
ХГД>>Со сторонними библиотеками тоже всяких приколов хватает. Вообще, нарвался на неожиданность — прокомментируй.
MTD>Бывает, но мы же о хорошем коде фантазируем.
О хорошем, а не о нереально совершенном. Вот, например, библиотека boost::asio, по-твоему, достаточно хороша? Да и что нам asio, как на счет стандартных стримов? Заслуживают ли комментария их мелкие отличия на различных платформах?
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, Хон Гиль Дон, Вы писали:
MTD>>Перепиши код проще, чтобы недостаточно внимательный разработчик не нарвался — гарантии, что он прочтет твой комментарий нет.
ХГД>Эээ, ты правда считаешь, что код, полагающийся на гарантированный стандартом языка порядок инициализации членов класса, обязательно сложный?
Если инициализация зависит от порядка членов в классе — это плохо, сломать это может кто угодно, да даже ты сам, через некоторое время. Явное всегда лучше неявного, используй фабрику.
ХГД>О хорошем, а не о нереально совершенном. Вот, например, библиотека boost::asio, по-твоему, достаточно хороша?
Не знаю, не сталкивался.
ХГД>Да и что нам asio, как на счет стандартных стримов? Заслуживают ли комментария их мелкие отличия на различных платформах?
Как бы и сами разработчики не скрывают, что дизайн стримов неудачный.
Ребята, поймите — быть практиком, не значит писать говнокод. Быть практиком — это решать проблему приемлемые сроки минимизируя затраты с адекватным качеством. В этом очень помогают проверенные практики, часть из которых я затронул — проверка контрактов, осмысленное именование, один класс — одна ответственность, инкапсуляция.
L>А что, если
Слишком много если, для практика, ну видно же что пытаешься изобрести проблему.
L>этот класс, котороый одновременно и визитор, и обсервер и еще кое-что, и есть результат рефакторинга?
Рефактори дальше — результат неудовлетворительный.
MTD>>Зачем? Мой вердикт — рефакторить.
L>То есть не смог придумать нормальное название — рефакторить?
Как профессионал я не могу маскировать проблему, я ее решаю.
Здравствуйте, MTD, Вы писали:
AWW>>мало коментариев.
MTD>В хорошем коде комментариев быть не должно.
Вот это срачь то развели на пустом месте. Ога!
Коментарии это не только коментарии, это —
— советы наследнику например
— себе самому типа "что-то тут иногда не то пролетает"
— или заметки что надо поправить если будет медленно работать.
Да триллион ситуаций.
Коментарии которые коментируют очевидное, никому не нужны, а вот коментарии которые рассказывают почему выделяется память кусками не по размеру элемента а по какие-то магические 1024 байта. То есть отсуствие комментариев говорит, об отношении работника к своей работе. То есть попросу — ему насрать. ))
Здравствуйте, MTD, Вы писали:
MTD>Здравствуйте, landerhigh, Вы писали:
L>>Теоретег.
MTD>Ребята, поймите — быть практиком, не значит писать говнокод. Быть практиком — это решать проблему приемлемые сроки минимизируя затраты с адекватным качеством. В этом очень помогают проверенные практики, часть из которых я затронул — проверка контрактов, осмысленное именование, один класс — одна ответственность, инкапсуляция.
Очень хорошие фразы.
L>>А что, если
MTD>Слишком много если, для практика, ну видно же что пытаешься изобрести проблему.
L>>этот класс, котороый одновременно и визитор, и обсервер и еще кое-что, и есть результат рефакторинга?
MTD>Рефактори дальше — результат неудовлетворительный.
Сказал кто? Ты видел код?
А вообще, я слегка покривил душой тут. Было время С++03, где не было ни лямбд, ни прочих вкусностей, а пользоваться std::bind_1st можно было только после пол-литры. И тогда, если твой класс хотел быть обсервером, самым простым способом было унаследовать интерфейс IObserver. Весело было, когда требовалось реализовывать два подобных интерфейса.
Вот тогда, бывало, смотришь на объявление класса и видишь, что он — кавалер ордена Визиторов дважды Обсервер Состояний. Даже если его имя этого не отражает.
Сейчас всю эту непубличную подноготную гораздо проще прятать под капот.
Здравствуйте, aik, Вы писали:
aik>struct super_struct *xxx = malloc(sizeof(*xxx) * MAX_SUPER_OBJECTS_NUM);
aik>и никаких комментариев. И почему именно malloc — надо писать если это c++ или glibc (а лучше заменять на new/g_malloc), а если голый си + libc — то зачем это комментировать?
Здравствуйте, landerhigh, Вы писали:
L>А подумать, что означает "потеря истории изменений", как она происходит и как и когда обнаруживается, и каким боком тут бекапы, подумать не пробовали, прежде чем написать?
Я представляю, какой срач поднимут некоторые увидев что-то такое :
#if 0
.... куча кода )))
#endif
Так обычно закрываются куски кода, которые выкинуты но пока еще нет 100% уверенности что это сделано правильно! ))
Здравствуйте, MTD, Вы писали:
MTD>Ребята, поймите — быть практиком, не значит писать говнокод.
быть практиком — это иногда писать говнокод
MTD> Быть практиком — это решать проблему приемлемые сроки минимизируя затраты с адекватным качеством. В этом очень помогают проверенные практики, часть из которых я затронул — проверка контрактов, осмысленное именование, один класс — одна ответственность, инкапсуляция.
практики — это хорошо, только не только они есть. есть еще сроки, нервы и сложность решения и сопровождения
одна ответственность возможна только на определенном уровне абстракции: на одно уровне у тебя одна ответственность, а на другом класс делает двадцать разных действий
L>>этот класс, котороый одновременно и визитор, и обсервер и еще кое-что, и есть результат рефакторинга?
MTD>Рефактори дальше — результат неудовлетворительный.
нет предела совершенству, надо уметь остановиться в рефакторинге
почему нельзя всегда давать хорошие имена — да потому что мы не писатели, мы программисты. если давать хорошие имена, то получится жава с длинными названями и эпические поэмы. ты, например, любишь поэмы, а я — хокку
программист борется с двумя сложностями :
1) сложность прикладной задачи
2) сложность решения прикладной задачи, т.к. попутно из-за "бест практик" ему приходится вводить сущности не из доменной области. а так, взятые с потолка
в общем, ты слишком категоричен и перфекционист Ж)
ХГД>>Эээ, ты правда считаешь, что код, полагающийся на гарантированный стандартом языка порядок инициализации членов класса, обязательно сложный?
MTD>Если инициализация зависит от порядка членов в классе — это плохо, сломать это может кто угодно, да даже ты сам, через некоторое время.
Вот чтобы не сломали, я и пишу комментарии. Пока помогает.
MTD>Явное всегда лучше неявного, используй фабрику.
Фабрика-то тут при чем? Посмотри, например, rationale к boost::base_from_member — там описана похожая ситуация, только еще хуже, когда для обеспечения правильного порядка инициализации приходится выносить член в базовый класс.
ХГД>>О хорошем, а не о нереально совершенном. Вот, например, библиотека boost::asio, по-твоему, достаточно хороша?
MTD>Не знаю, не сталкивался.
Ну а с какой-нибудь хорошей кроссплатформенной плюсовой сетевой библиотекой сталкивался? Или таких нет в природе?
ХГД>>Да и что нам asio, как на счет стандартных стримов? Заслуживают ли комментария их мелкие отличия на различных платформах?
MTD>Как бы и сами разработчики не скрывают, что дизайн стримов неудачный.
Тем не менее, с ними приходится иметь дело. Значит и без комментариев не обойтись. Да не в дизайне там дело, а в особенностях реализиции.
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, uzhas, Вы писали:
U>быть практиком — это иногда писать говнокод
Даже в таком случае практик локализует его и обернет красиво, сразу и не поймешь.
U>практики — это хорошо, только не только они есть. есть еще сроки, нервы и сложность решения и сопровождения
По моему опыту попытка сэкономить время оборачивается серьезными проблемами потом, если это только не проект — написали и выбросили. Опять же я говорю о новом коде, на поддержке часто видишь, что плохо, а сделать ничего не можешь — нет ни времени, ни сил.
U>нет предела совершенству, надо уметь остановиться в рефакторинге
Золотые слова.
U>почему нельзя всегда давать хорошие имена — да потому что мы не писатели, мы программисты. если давать хорошие имена, то получится жава с длинными названями и эпические поэмы. ты, например, любишь поэмы, а я — хокку
Я тоже сторонник лаконичности, а в яве длинные имена от увлечения паттернами и прочей хренью, когда вместо терминов задачи вылезают термины решения — ProactorVisitorObserverStateCacheManipulator
U>в общем, ты слишком категоричен и перфекционист Ж)
Здравствуйте, Хон Гиль Дон, Вы писали:
ХГД>Ну а с какой-нибудь хорошей кроссплатформенной плюсовой сетевой библиотекой сталкивался? Или таких нет в природе?
Сокеты Беркли Писал сам над ними кросплатформенную обертку которая так и прижилась в компании.
Здравствуйте, Хон Гиль Дон, Вы писали:
ХГД>>>Двухстадийная инициализация еще хуже, запихивать их в unique_ptr — ваще отстой.
MTD>>Почему?
ХГД>А незачем без нужды хип теребить.
Стек тоже не резиновый А двухстадийная инициализация чем не угодила?
Здравствуйте, MTD, Вы писали:
ХГД>>Ну а с какой-нибудь хорошей кроссплатформенной плюсовой сетевой библиотекой сталкивался? Или таких нет в природе?
MTD>Сокеты Беркли Писал сам над ними кросплатформенную обертку которая так и прижилась в компании.
Отличный подход — все писать самому с нуля. Вот только цену софта увеличивает в разы, зато комментариев минимум
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, MTD, Вы писали:
ХГД>>>>Двухстадийная инициализация еще хуже, запихивать их в unique_ptr — ваще отстой.
MTD>>>Почему?
ХГД>>А незачем без нужды хип теребить.
MTD>Стек тоже не резиновый А двухстадийная инициализация чем не угодила?
А мне удобно, например, в одном классе держать дивайс (в терминологии boost::iostreams), фильтр (оттуда же), стримбуфер, стрим и архив (из boost::serialization). А архив не умеет двухстадийно инициализироваться. Да и стрим тоже хочет буфер прямо в конструктор. И вообще все эти open/close усложняют код так, что связываться с ними без действительно веской причины я не желаю.
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Здравствуйте, landerhigh, Вы писали:
L>Топик-то про C++
Неа, топик уже про то кто лучше кодер и нужны ли комментарии — один спорщик тянет в сторону что они не нужны вообще (детсад), другой — что нужны везде (зануда). Как то так
Здравствуйте, aik, Вы писали:
aik>Здравствуйте, landerhigh, Вы писали:
L>>Топик-то про C++
aik>Неа, топик уже про то кто лучше кодер и нужны ли комментарии — один спорщик тянет в сторону что они не нужны вообще (детсад), другой — что нужны везде (зануда). Как то так
Если чо, то я за то, что комментировать надо интрефейсную часть юнитов, т.е. относиться к комментариям, как к документации API.
Потроха же лучше устраивать так, чтобы комментировать их было незачем.
Здравствуйте, landerhigh, Вы писали:
L>Если чо, то я за то, что комментировать надо интрефейсную часть юнитов, т.е. относиться к комментариям, как к документации API.
Поправка — не комментировать, а документировать, по-нормальному — с ссылками, описанием исключений и прочего, доксиген здесь кстати.
L>Потроха же лучше устраивать так, чтобы комментировать их было незачем.
У тебя что раздвоение личности? Когда я об этом говорил ты со мной спорил с пеной у рта
Здравствуйте, MTD, Вы писали:
L>>Если чо, то я за то, что комментировать надо интрефейсную часть юнитов, т.е. относиться к комментариям, как к документации API.
MTD>Поправка — не комментировать, а документировать, по-нормальному — с ссылками, описанием исключений и прочего, доксиген здесь кстати.
Как хочешь называй. Ни того, ни другого в куске кода ТС и близко нет.
L>>Потроха же лучше устраивать так, чтобы комментировать их было незачем.
MTD>У тебя что раздвоение личности? Когда я об этом говорил ты со мной спорил с пеной у рта
Учу читать. Дорого:
Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно). Очень хорошо написать, какие паттерны используются. Хороший комментарий в заголовке класса убирает необходимость заглядывать в определение методов от слова совсем.
А теперь, краткий курс логики для планирующих сдавать IELTS!
Вышеприведенная цитата говорит о том, что:
1. Комментирование потрохов реализации обязательно.
2. Комментирование потрохов реализации излишне.
3. Not Given
L>Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
L>А теперь, краткий курс логики для планирующих сдавать IELTS!
L>Вышеприведенная цитата говорит о том, что: L>1. Комментирование потрохов реализации обязательно. L>2. Комментирование потрохов реализации излишне. L>3. Not Given
Очевидно, пункт 1, так как далеко не все классы являются частью публичного интерфейса. А потом, ты вдруг выдаешь пункт 2:
Потроха же лучше устраивать так, чтобы комментировать их было незачем.
Здравствуйте, MTD, Вы писали:
L>>Вышеприведенная цитата говорит о том, что: L>>1. Комментирование потрохов реализации обязательно. L>>2. Комментирование потрохов реализации излишне. L>>3. Not Given
MTD>Очевидно, пункт 1, так как далеко не все классы являются частью публичного интерфейса. А потом, ты вдруг выдаешь пункт 2:
MTD>
MTD>Потроха же лучше устраивать так, чтобы комментировать их было незачем.
MTD>Раздвоение личности?
Все гораздо проще — ты бы завалил IELTS и схожие тесты.
Правильный ответ — 3.
Здравствуйте, MTD, Вы писали:
MTD>Здравствуйте, landerhigh, Вы писали:
L>>Все гораздо проще — ты бы завалил IELTS и схожие тесты. L>>Правильный ответ — 3.
MTD>Уверен, сейчас ты приведешь доказательство где у меня ошибка, не балабол же ты в самом деле.
Брать на слабо — это для детского сада. Поумерь пыл, если хочешь, чтобы с тобой продолжали разговор.
У тебе в элементарной внимательности ошибка.
Приведенная мной цитата ни слова не говорит о том, как нужно комментировать потроха и нужно ли вообще. Перечитай ее еще раз. А потом еще раз.
Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно). Очень хорошо написать, какие паттерны используются. Хороший комментарий в заголовке класса убирает необходимость заглядывать в определение методов от слова совсем.
Кстати, распространенная проблема среди населения вообще, не только погромиздов. Читают спеки и делают далеко идущие выводы на основе того, что им показалось, когда они это читали.
L>Заголовок с копирайтом позволяет сразу понять, что это, откуда оно взялось и кто в этом виноват. Даже когда работаешь из движущегося поезда без подключения к системе.
Здравствуйте, aik, Вы писали:
aik>лютый трындец. write-only.
Ничего подобного. Бывает много, много хуже. В коде, прошу заметить, всего одна глобальная переменная и числа с плавающей точкой сравниваются через == только в ассёртах.
Здравствуйте, Skorodum, Вы писали:
L>>Заголовок с копирайтом позволяет сразу понять, что это, откуда оно взялось и кто в этом виноват. Даже когда работаешь из движущегося поезда без подключения к системе.
S>Git — everything is local.
Сбросил тебе по шаре тимлид прототип, который он набросал за 15 минут на коленке в кафе из надерганных отовсюду файлов. Никакой гит тебе не поможет узнать, откуда что взялось и кто вообще это поддерживает.
Здравствуйте, landerhigh, Вы писали:
MTD>>Уверен, сейчас ты приведешь доказательство где у меня ошибка, не балабол же ты в самом деле.
L>Брать на слабо — это для детского сада.
Как можно! Попросили слова аргументировать — все обида.
L>Поумерь пыл, если хочешь, чтобы с тобой продолжали разговор.
А что ты в принципе можешь рассказать заслуживающего внимание?
L>У тебе в элементарной внимательности ошибка. L>Приведенная мной цитата ни слова не говорит о том, как нужно комментировать потроха и нужно ли вообще. Перечитай ее еще раз. А потом еще раз.
Ну давай как для маленьких на пальцах.
Тезис: Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
Вывод: Нужно комментировать классы.
Факт: Не все классы являются частью публичного интерфейса.
Вывод: Нужно комментировать классы не являющиеся частью публичного интерфейса (потроха).
Вывод: Как минимум классы в потрохах комментировать нужно.
Где ошибка?
L>Кстати, распространенная проблема среди населения вообще, не только погромиздов. Читают спеки и делают далеко идущие выводы на основе того, что им показалось, когда они это читали.
Опять бла-бла-бла, становится скучно, что нибудь конкретное в тему есть что сказать?
Здравствуйте, MTD, Вы писали:
L>>Поумерь пыл, если хочешь, чтобы с тобой продолжали разговор.
MTD>А что ты в принципе можешь рассказать заслуживающего внимание?
А ты?
L>>У тебе в элементарной внимательности ошибка. L>>Приведенная мной цитата ни слова не говорит о том, как нужно комментировать потроха и нужно ли вообще. Перечитай ее еще раз. А потом еще раз.
MTD>Ну давай как для маленьких на пальцах.
MTD>Тезис: Комментарий должен помогать в использовании кода. Кратко сообщать о том, что это за класс, зачем он нужен и как и в каких случаях используется, какие задачи решает, а какие-нет (в случае, если это неочевидно).
MTD>Вывод: Нужно комментировать классы.
MTD>Факт: Не все классы являются частью публичного интерфейса.
Факт: переиспользуются не только классы, являющиеся частью публичного интерфейса. Публичного интерфейса как такового может и не быть. Потенциально вообще все юниты могут быть переиспользованы.
Факт: для эффективного переиспользования любых классов требуется наличие адекватной документации
MTD>Вывод: Нужно комментировать классы не являющиеся частью публичного интерфейса (потроха).
Налицо путаница в терминологии. Потроха — это детали реализации этих классов, которые как раз желательно писать так, чтобы туда нужды заглядывать вообще не было.
MTD>Вывод: Как минимум классы в потрохах комментировать нужно.
Комментировать нужно все юниты. Обычно это классы. Потроха этих классов, напротив, в комментировании нуждаются редко.
MTD>Где ошибка?
В терминологии.
L>>Кстати, распространенная проблема среди населения вообще, не только погромиздов. Читают спеки и делают далеко идущие выводы на основе того, что им показалось, когда они это читали.
MTD>Опять бла-бла-бла, становится скучно, что нибудь конкретное в тему есть что сказать?
Здравствуйте, landerhigh, Вы писали:
MTD>>А что ты в принципе можешь рассказать заслуживающего внимание?
L>А ты?
Тебе например, рассказал про проверки контрактов, инкапсуляцию, грамотное именование классов и разграничении ответственности, не моя беда, что ты не захотел слушать. А теперь ответь на вопрос.
L>>>У тебе в элементарной внимательности ошибка.
L>Факт: переиспользуются не только классы, являющиеся частью публичного интерфейса. Публичного интерфейса как такового может и не быть. Потенциально вообще все юниты могут быть переиспользованы.
Еще раз, где у меня ошибка? Снова в лужу сел и начал досочинять?
L>Факт: для эффективного переиспользования любых классов требуется наличие адекватной документации
Не обязательно, код сам по себе зачастую является документацией, которая к тому же всегда в актуальном состоянии.
L>Налицо путаница в терминологии. Потроха — это детали реализации этих классов, которые как раз желательно писать так, чтобы туда нужды заглядывать вообще не было.
Деталями реализации классов могут быть другие классы, о необходимости документирования которых ты твердишь, будь последователен.
MTD>>Где ошибка?
L>В терминологии.
Как уж на сковородке
L>Все четко и по теме.
Бла-бла-бла, тут ты мастер, видимо и по работе тебя знают как мастера объемной но бесполезной документации, а не производителя надежного кода
Здравствуйте, MTD, Вы писали:
L>>Все четко и по теме.
MTD>Бла-бла-бла, тут ты мастер, видимо и по работе тебя знают как мастера объемной но бесполезной документации, а не производителя надежного кода
aik>>лютый трындец. write-only. BFE>Ничего подобного. Бывает много, много хуже. В коде, прошу заметить, всего одна глобальная переменная и числа с плавающей точкой сравниваются через == только в ассёртах.
Угу, код конечно с первого взгляда выглядит пугающе, но если вчитаться — все это без труда можно отрефакторить. То что код не отрефакторил изначальный его писатель, негативно характеризует этого писателя, как черезчур упертого, а то что код не смогли отрефакторить все остальные члены команды — не лучшим образом характеризует уже их, как не очень умных любителей с нуля переписывать любой код, написанный без их участия. Как охарактеризовать менеджера этой команды, плачущегося в форуме и кидающегося кусками проприетарного кода и личными данными своих работников даже не знаю.. Но наверно они все созданы друг для друга...
Как много веселых ребят, и все делают велосипед...
Мою ветку зачем то закрыли. Будто там не было принципиальных вопросов.
Это
Здравствуйте, ononim, Вы писали:
aik>>>лютый трындец. write-only. BFE>>Ничего подобного. Бывает много, много хуже. В коде, прошу заметить, всего одна глобальная переменная и числа с плавающей точкой сравниваются через == только в ассёртах. O>Угу, код конечно с первого взгляда выглядит пугающе, но если вчитаться — все это без труда можно отрефакторить. То что код не отрефакторил изначальный его писатель, негативно характеризует этого писателя, как черезчур упертого, а то что код не смогли отрефакторить все остальные члены команды — не лучшим образом характеризует уже их, как не очень умных любителей с нуля переписывать любой код, написанный без их участия. Как охарактеризовать менеджера этой команды, плачущегося в форуме и кидающегося кусками проприетарного кода и личными данными своих работников даже не знаю.. Но наверно они все созданы друг для друга...
Можно, но надо думать как. А я еще раз повторяю, алгоритм делался с нуля, отлаживался и дописывался, и времени на его приведение в "красивый вид" не было. Больше того, если бы заниматься красивостями попутно с написанием алгоритма, это было бы чревато и для алгоритма, и времени, но пользы бы не принесло особой. ПОСЛЕ написания и отладки, согласен, код надо рефакторить, если есть хорошие идеи. В противном случае получаем искусственно фрагментированный алгоритм, который вряд ли станет понятнее от множества "небольших функций".
На это есть возражения? А между тем болтовней о красивостях меня еще и отвлекали от разработки, отнимая полезное время.
Религия, догматизм в программировании далеко не лучший союзник. Когда под эту религию вынуждены прогибаться сотрудники, это не работа, а молельный дом.
Только дятел стал бы выкладывать сюда код. Но раз уж выложен, я спрашиваю, есть ли запрет на написание длинных функций? Не специально длинных, заметьте, а потому что алгоритм таков по природе. Я объяснял постановку задачи и споосб ее решения, так вот весь этот "длинный" код однороден и целостен, разбивать при желании его можно, но будет выглядеть неоправданным насилием.
Здесь уже согласились, что в работе с гитом возможны исключительные случаи, когда слияния бессмысленны. Почему же тут так упираются признать, что возможны и исключительные ситуации и с длиной кода, когда длинно не значит обязательно плохо?
Наконец, пара контрольных вопросов
1) Правильно ли, когда разработка алгоритма оценивается манагером вдвое дешевле ничего не значащего "сопровождения"?
2) Как вам ситуация, когда разработчик поставлен в условия самоокупаемости при разработке в основном разовых заказов от отечественных заказчиков?
Если вы посмотрите на длинный код внимательно, вы увидите массу комментов, начинающихся у левого края. Это и были мои юнит-тесты, помеченные словом PARSE и пройденные и давно закрытые по причине завершения отладки каждого из предыдущих блоков текста.
Понимаю, до некоторых такое тяжело доходит. На то они и каста манагеров.
"Глупость это такой ум" (А.Лебедь)
От прошлой ветки у меня сохранилось только это.
Отвечу.
Здравствуйте, dalmal, Вы писали:
D>Ох и портянок ты написал тут по треду, сколько надменности. Не будь таким, это же фу-фу-фу.
С волками жить. Не у вас ли тут в кодексе форума указано, что вы готовы презирать всех, кто ниже вас.
D>Если ты внимательно читал Макконнелла[] D> отказ от притязаний на роль эксперта, если вы им не являетесь;
Ну а если являюсь?
D> охотное признание собственных ошибок;
Я всегда готов учиться на своих и чужих ошибках, и не считаю это "ужасным грехом", напротив. В отличие от (манагеров), этих упертых дятлов, которые их каждой ошибки делают проблему.
D>33.5. Общение и сотрудничество D>По настоящему отличные программисты учатся эффективно сотрудничать, что всегда подразумевает написание удобочитаемого кода. Вероятно, компьютер читает вашу программу так же часто, как другие люди, но он читает плохой код гораздо лучше, чем люди. Работая над кодом, не забывайте про людей, которым придется изменять его в будущем. Программирование — это в первую очередь общение с другим программистом и только во вторую — с компьютером. D>[/q] D>Понимаешь? Общение важнее кода, так что нужно поумерить своё эго, не считать себя умнее других и уважать людей, с которыми ты работаешь. Независимо от их должности, а также от того, умнее ты их в самом деле или нет. D>Тогда и вопросов у тебя таких не будет ко всем, а у начальства и коллектива к тебе.
Я объясню вам, как я это понимаю. Я принимаю всех людей равными себе, и потому неприемлю ни собственного уничижения, ни возвеличивания других без всякгого на то основания. Равенство есть залог нормальных отношений в коллективе, и когда я писал о настоящих своих коллегах, я писал о равных себе людях (хотя среди них были и профессора).
Уровень ума также не значит многого, если гарантировано равенство. Было бы интересно с человеком.
Но равенство предполагает ведь и равенство в оплате, верно?
Какое же уважение может заслуживать манагер, произвольно устанавливающий зряплату себе и остальным бездельникам?
"О чём поёшь, степняк?"
"О том что вижу о том и пою".
И дальше идет музыка тоскливая как свист ветра в коляючках саксаулов и аксакалов...
Код [похоже] делает две достаточно простые вещи собирает параметры диалога и трансформирует некий DOM из одной формы в другую на их основе.
Никакого заявленного Draw*** там и близко нет ...
Ну чего бы эти две функции по разным классам хотя бы не разнести ...
Фиг уже с этими model view controllers, не каждому эти слова даются, но здравый смысл-то должен же быть в концу концов...
Здравствуйте, c-smile, Вы писали:
CS>Здравствуйте, GhostCoders, Вы писали:
Уважаемый c-smile, я — автор кода , GhostCoders никакого отношения к нему не имеет.
Все вопросы ко мне.
Напишите конкретнее, что за здравый смысл вам привиделся?
Здравствуйте, ononim, Вы писали:
aik>>>лютый трындец. write-only. BFE>>Ничего подобного. Бывает много, много хуже. В коде, прошу заметить, всего одна глобальная переменная и числа с плавающей точкой сравниваются через == только в ассёртах. O>Угу, код конечно с первого взгляда выглядит пугающе, но если вчитаться — все это без труда можно отрефакторить.
Кстати, ловлю на слове. Давайте покажите, как этот код рефакторится, да еще без труда.
Докажите, что вы не трепло, в отличие от большинства здесь кидающихся бандерлогов.
Люблю учиться у умных людей.
От ваших тем очень несет надменностью.
Вполне возможно, что этот ваш бывший начальник действительно не шибко умеет руководить. И то что он выложил сюда код, не сумев вам самостоятельно пояснить в чем проблема кода, характеризует его не с лучшей стороны.
Но вы гребете под одну гребенку всех кто выше вас по должности, говоря про неравенство в зарплате.
Видимо сказывается, что начальство уже давно моложе вам и это вас нереально бесит. Оттого и считаете всех за тупое говно.
Да, есть в IT такая проблема, что старики оказываются в джуниорах.
Но это не дает им право быть говном
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Я конечно особо не вникал в логику работы кода, но помоему тут целым namespace попахивает (вынести из этого dlg в классы логику) и таким образом "разгрузить" диалог.
Здравствуйте, alexb1980, Вы писали:
A>Здравствуйте, GhostCoders, Вы писали:
GC>>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
A>Я конечно особо не вникал в логику работы кода, но помоему тут целым namespace попахивает (вынести из этого dlg в классы логику) и таким образом "разгрузить" диалог.
Да то, что диалог совмещен с алгоритмом, пусть вас не смущает. Это всегда можно разделить, но сейчас это несущественно. Разговор о самой длинной функции — как ее рефакторить? Говорят, без труда. Дайте просто идею, с чего начать. Без полной переработки — она не нужна, поезд уже ушел.
(Манагеру надо было выкладывать только эту одну функцию, а не весь почти проект. Если уж цепляться ко всему проекту в целом, то надо заметить, что отдельные моменты — это установки фирмы, а не мои изобретения. И я не хочу их защищать или поносить, мне они безразличны).
Здравствуйте, Varavva, Вы писали:
V>От ваших тем очень несет надменностью.
Да, чувство равенства дает ощущение превосходства над теми бандерлогами, которые равенства не признают и всю жизнь живут по шакальи.
V>Видимо сказывается, что начальство уже давно моложе вам и это вас нереально бесит. Оттого и считаете всех за тупое говно.
Вот тут вы глубоко заблуждаетесь. Причина денег не может быть для меня определяющей. Это рассуждение для бандерлога, а не человека.
V>Да, есть в IT такая проблема, что старики оказываются в джуниорах. V>Но это не дает им право быть говном
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Уважаемый c-smile, я — автор кода , GhostCoders никакого отношения к нему не имеет. ЮЛ>Все вопросы ко мне. ЮЛ>Напишите конкретнее, что за здравый смысл вам привиделся?
aik>>>>лютый трындец. write-only. BFE>>>Ничего подобного. Бывает много, много хуже. В коде, прошу заметить, всего одна глобальная переменная и числа с плавающей точкой сравниваются через == только в ассёртах. O>>Угу, код конечно с первого взгляда выглядит пугающе, но если вчитаться — все это без труда можно отрефакторить. ЮЛ>Кстати, ловлю на слове. Давайте покажите, как этот код рефакторится, да еще без труда. ЮЛ>Докажите, что вы не трепло, в отличие от большинства здесь кидающихся бандерлогов.
Переводя на вашу лексику одну известную поговорку — если выйдя на улицу вы встретили бандерлога — значит вы встретили бандерлога. Если же вокруг вас все бандерлоги — значит вы бандерлог.
Весь код рефакторить не буду, мне еще есть чем заняться, но могу показать как я бы начал делать декомпозицию, даже не вникая пока в весь алгоритм. К примеру берем одну из самых длинных ф-ий — CSewingDlg::PseudoProcessAcceptedData. ВИдим цикл в самом начале. Видим коммент //PARSING TEST. Выносим весь этот цикл в ф-ю с название ParsingTest().
Видим следующий цикл, откоменченный как optimizing vertices (sorting). С ним сложнее — потому посмотрим что у него внутри. Натыкаемся на for (size_t j = 0; j < LineEnds.size(); j++). Смотрим что делает. А делает он подготовку концов отрезков, или чтото типа того. Потому пишем такую функцию
...код под if (ii == 0 && ii0==1) ниче не делает — потому его тут нету. Это тоже не идеальная декомпозиция, но для первой итерации рефакторинга сойдет. Далее по тексту видим комент PARSING SORT — видимо последующий код можно выделить в ParsingSort(), но как я уже писал — я не подрядился перелопачивать весь ваш код, тут работы на несколько дней, а не на 10 минут. Вообще всю эту гиперфункцию PseudoProcessAcceptedData лучше вынести в отдельную сучность-класс, назвать ее AcceptrdDataPseudoProcessor а все что я написал выше, и что не написал — сделать ее методами. Про мелочи типа именования переменных и т.п. я уж не говорю (ii0 — это жесть). Ваш код для proof-of-concept вполне годный, но для продакшена его нужно переделать. К счастью его и _можно_ переделать, что бывает далеко не всегда.
Как много веселых ребят, и все делают велосипед...
Здравствуйте, MasterZiv, Вы писали:
MZ>Здравствуйте, niXman, Вы писали:
X>>в общем, довольно не плохо с первого взгляда, видал и похуже %) X>>"порадовало" использование запятой:
MZ>Это плохо, однако!
Так, а чем плохо использование запятой или вложенных тернарных операций, можете ясно ответить? Может, вам стоит написать Страустрапу, чтобы запретить запятую?
Здравствуйте, ononim, Вы писали:
O>Весь код рефакторить не буду, мне еще есть чем заняться, но могу показать как я бы начал делать декомпозицию, даже не вникая пока в весь алгоритм. К примеру берем одну из самых длинных ф-ий — CSewingDlg::PseudoProcessAcceptedData. ВИдим цикл в самом начале. [] назвать ее AcceptrdDataPseudoProcessor а все что я написал выше, и что не написал — сделать ее методами. Про мелочи типа именования переменных и т.п. я уж не говорю (ii0 — это жесть). Ваш код для proof-of-concept вполне годный, но для продакшена его нужно переделать. К счастью его и _можно_ переделать, что бывает далеко не всегда.
Вообще, да, это первое что приходит в голову. Открою вам страшную тайну — примерно так я и сделал перед вторым этапом работы, именно, разбил код этой гиперфункции на 8 последовательных вызовов, с более-менее замкнутым содержанием в каждой. Но они все пользуются общими переменными, которые в класс тащить не очень хочется. Потому что это рабочие переменные, потому что их определение и время жизни уходит из-под контроля, они становятся для алгоритма в некоем смысле "глобальными", что конечно, не смертельно, но и не помогает в разработке. Поэтому я поступил проще — передаю их все как ссылки через параметры. Это временная мера, и также я продолжаю искать лучшее решение. Несмотря на то, что выглядят вызовы "ужасно", зато я сразу вижу все зависимости, где переменные возникают, где они используются.
Но в любом случае это насилие над кодом, реально не приносящее особых выгод. Слишком искусственный подход, еще немного, и получится обертка каждой строки. Я ищу семантический (осмысленный) способ рефакторинга. Скажем, если бы можно было придумать определенную алгебру узлов, я взял бы ее за основу класса безоговорочно.
Теперь с точки зрения разработки. Допустим, я пишу локальный цикл, еще не зная, насколько он будет полезен в окончательном алгоритме. Вы рекомендуете мне сразу оформлять его в виде метода класса, или подождать, пока код устоится? Если я его оформляю в метод, то 1) он отрывается визуально от контекста (а именно из контекста я решаю о его полезности), мне надо проделать лишнюю техническую работу по заведению метода, разрешить вопросы с переменными и т.д. В результате я выбрасываю этот метод.
Или же просто я пишу цикл по месту алгоритма, оцениваю его и при желании правлю или удаляю, никуда не перемещаясь. Результат получается тот же, но без лишних хлопот.
Поскольку логика алгоритма в основном последовательная, то нет большой беды в раздувании функции, — при исследовании каждого блока необходим лишь ближайший участок кода. Кажется, и у МакКонелла говорится об том же, что подобная последовательность кода не обязательно требует формального ограничения числа строк.
Если вернуться к аналогии с Войной и Миром, то заведение отдельных методов для мелких частей алгоритма подобно простановке названий параграфов в оглавлении длинной главы. Насколько это будет полезно? Кто читает главы по параграфам?
К слову сказать, те 8 подфункций, к которым я прибег, в этой аналогии приобретают смысл страниц — чисто технический прием для преодоления иэбыточного объема.
ЮЛ>Но в любом случае это насилие над кодом, реально не приносящее особых выгод. Слишком искусственный подход, еще немного, и получится обертка каждой строки. Я ищу семантический (осмысленный) способ рефакторинга. Скажем, если бы можно было придумать определенную алгебру узлов, я взял бы ее за основу класса безоговорочно.
Это не насилие над кодом, это дело тренировки восприятия + "дефолтовые настройки" этих особенностей у большинства людей. Большинство людей не могут держать в активной обработке большое количество сущностей без разделения их на иерархии. Да, можно ценой многолетней практики научится это делать, расширив количество одновременно обрабатываемых мозгом сущностей. И даже для такого человека работа с большим количеством однаранговых entities может быть эффективнее нежели работа над иерархией с "подгрузкой" в сознание каждого уровня по мере необходимости. Но надо отдавать себе отчет что код вы пишете не для себя, а для дяди, который вам платит за него денежку. Потому мнение этого дяди как минимум нужно принимать в расчет, иначе дядя вас может уволить, как видимо и произошло.
А дальше вопрос плавно перетекает в плоскость как вас можно было замотивировать сделать нужные изменения. Я думаю это тоже достаточно несложно можно было сделать, и то что этого не смог сделать менеджер — признак его непрофессионализма. Но методы мотивации вас лежат в не-технической плоскости, потому они скучны и вообще офтопны в этом разделе
Как много веселых ребят, и все делают велосипед...
Здравствуйте, ononim, Вы писали:
ЮЛ>>Но в любом случае это насилие над кодом, реально не приносящее особых выгод. Слишком искусственный подход, еще немного, и получится обертка каждой строки. Я ищу семантический (осмысленный) способ рефакторинга. Скажем, если бы можно было придумать определенную алгебру узлов, я взял бы ее за основу класса безоговорочно. O>Это не насилие над кодом, это дело тренировки восприятия + "дефолтовые настройки" этих особенностей у большинства людей. Большинство людей не могут держать в активной обработке большое количество сущностей без разделения их на иерархии. Да, можно ценой многолетней практики научится это делать, расширив количество одновременно обрабатываемых мозгом сущностей. И даже для такого человека работа с большим количеством однаранговых entities может быть эффективнее нежели работа над иерархией с "подгрузкой" в сознание каждого уровня по мере необходимости. Но надо отдавать себе отчет что код вы пишете не для себя, а для дяди, который вам платит за него денежку. Потому мнение этого дяди как минимум нужно принимать в расчет, иначе дядя вас может уволить, как видимо и произошло. O>А дальше вопрос плавно перетекает в плоскость как вас можно было замотивировать сделать нужные изменения. Я думаю это тоже достаточно несложно можно было сделать, и то что этого не смог сделать менеджер — признак его непрофессионализма. Но методы мотивации вас лежат в не-технической плоскости, потому они скучны и вообще офтопны в этом разделе
Ну хорошо если +, а если -? Если "дефолтные настройки" не помогают в разработке? Вы такое исключаете?
Откуда берутся эти настройки? Не догматические ли это сущности?
Вы пытаетесь меня (себя) убедить в том, что бездумное заискивание благосклонности этого дяди (денюшки, какой аргумент!) важнее цели разработки?
Так что все же вы скажете, 200 строк на функцию — это табу, или признаете возможные исключения из правил (которые я вовсе не подвергаю сомнению с общей точки зрения разумности, вообще — но не в частности).
ЮЛ>>>Но в любом случае это насилие над кодом, реально не приносящее особых выгод. Слишком искусственный подход, еще немного, и получится обертка каждой строки. Я ищу семантический (осмысленный) способ рефакторинга. Скажем, если бы можно было придумать определенную алгебру узлов, я взял бы ее за основу класса безоговорочно. O>>Это не насилие над кодом, это дело тренировки восприятия + "дефолтовые настройки" этих особенностей у большинства людей. Большинство людей не могут держать в активной обработке большое количество сущностей без разделения их на иерархии. Да, можно ценой многолетней практики научится это делать, расширив количество одновременно обрабатываемых мозгом сущностей. И даже для такого человека работа с большим количеством однаранговых entities может быть эффективнее нежели работа над иерархией с "подгрузкой" в сознание каждого уровня по мере необходимости. Но надо отдавать себе отчет что код вы пишете не для себя, а для дяди, который вам платит за него денежку. Потому мнение этого дяди как минимум нужно принимать в расчет, иначе дядя вас может уволить, как видимо и произошло. O>>А дальше вопрос плавно перетекает в плоскость как вас можно было замотивировать сделать нужные изменения. Я думаю это тоже достаточно несложно можно было сделать, и то что этого не смог сделать менеджер — признак его непрофессионализма. Но методы мотивации вас лежат в не-технической плоскости, потому они скучны и вообще офтопны в этом разделе
ЮЛ>Ну хорошо если +, а если -? Если "дефолтные настройки" не помогают в разработке? Вы такое исключаете? ЮЛ>Откуда берутся эти настройки? Не догматические ли это сущности?
Все люди разные. Дефолтовые настройки большинства мешают лично вашей разработке. Но код вы пишете не для себя, а для того самого большинства. Потому хотябы из соображений вежливости — перед передачей своего кода большинству — приведите его в тот вид, который оно сможет прожевать.
ЮЛ>Вы пытаетесь меня (себя) убедить в том, что бездумное заискивание благосклонности этого дяди (денюшки, какой аргумент!) важнее цели разработки?
Я не пытаюсь убеждать, я констатирую реальность Если ваша цель разработка — разрабатывайте как вам хочется, если ваша цель заработать денег дяде, чтобы он часть этих денег потом отдал вам — придется учитывать желания других. Кроме говоря про разработку — не забывайте что разработку эту ведете не вы один. Ну если вы ее собираетесь вести один — это ваше желание может не совпасть с желанием дяди. А дядя это не убеждения, дядя — это реальность. Иначе велком в шаровару — пишите софт сами, и сами его и продавайте.
ЮЛ>Так что все же вы скажете, 200 строк на функцию — это табу, или признаете возможные исключения из правил (которые я вовсе не подвергаю сомнению с общей точки зрения разумности, вообще — но не в частности).
У меня есть одно правило в жизни — не иметь правил в жизни Нужно смотреть каждую ситуацию в отдельности исходя из общих соображений оптимизации трудозатрат для разработки и поддержки продукта. В данном случае код можно и нужно побить на кусочки. Кстати можете если не лень провести длительный эксперимент — через пару месяцев (а лучше дольше) отсутствия работы с этим кодом отдайте его кому нить, и попросите внести какой нить мелкий баг. Ну или попробуйте модифицировать условия задачи. Потом приведите код в нужный вид.
Как много веселых ребят, и все делают велосипед...
Здравствуйте, ononim, Вы писали:
O>Все люди разные. Дефолтовые настройки большинства мешают лично вашей разработке. Но код вы пишете не для себя, а для того самого большинства. Потому хотябы из соображений вежливости — перед передачей своего кода большинству — приведите его в тот вид, который оно сможет прожевать. O>Я не пытаюсь убеждать, я констатирую реальность Если ваша цель разработка — разрабатывайте как вам хочется, если ваша цель заработать денег дяде, чтобы он часть этих денег потом отдал вам — придется учитывать желания других. Кроме говоря про разработку — не забывайте что разработку эту ведете не вы один.
Видимо, вы не поняли обстоятельств. Код этот находился еще в разработке. Разработка прерывалась из-за резких поворотов манагера — он то хотел обслуживать заказчиков, то не хотел. Меня он оставил на самоокупаемость. Вы пробовали такой способ работы, да еще с отечественным заказчиком? Разумеется, никакой команды не было. Да если бы и была, с этой командой я потерял бы только больше времени и возможно не смог выполнить даже сделанного.
O>Ну если вы ее собираетесь вести один — это ваше желание может не совпасть с желанием дяди. А дядя это не убеждения, дядя — это реальность. Иначе велком в шаровару — пишите софт сами, и сами его и продавайте.
Кто вам сказал, что я собираюсь / собирался работать один? Вы всегда пользуетесь домыслами?
O>У меня есть одно правило в жизни — не иметь правил в жизни Нужно смотреть каждую ситуацию в отдельности исходя из общих соображений оптимизации трудозатрат для разработки и поддержки продукта. В данном случае код можно и нужно побить на кусочки.
На любые кусочки, или все же есть стратегия? Или как дядя скажет?
O>Кстати можете если не лень провести длительный эксперимент — через пару месяцев (а лучше дольше) отсутствия работы с этим кодом отдайте его кому нить, и попросите внести какой нить мелкий баг. Ну или попробуйте модифицировать условия задачи. Потом приведите код в нужный вид.
Давайте лучше я внесу баг в ваш самый лучший код. И вы его моментально найдете, через 2 месяца. Пустые заклинания.
Кстати, я такой опыт фактически уже провел — тот исходный код, что вы невзлюбили, уже существует в стадии второго и успешного релиза, с новым алгоритмом внешнего контура, который я даже не надеялся написать.
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Здравствуйте, alexb1980, Вы писали:
A>>Здравствуйте, GhostCoders, Вы писали:
GC>>>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
A>>Я конечно особо не вникал в логику работы кода, но помоему тут целым namespace попахивает (вынести из этого dlg в классы логику) и таким образом "разгрузить" диалог.
ЮЛ>Да то, что диалог совмещен с алгоритмом, пусть вас не смущает. Это всегда можно разделить, но сейчас это несущественно. Разговор о самой длинной функции — как ее рефакторить? Говорят, без труда. Дайте просто идею, с чего начать. Без полной переработки — она не нужна, поезд уже ушел.
ЮЛ>(Манагеру надо было выкладывать только эту одну функцию, а не весь почти проект. Если уж цепляться ко всему проекту в целом, то надо заметить, что отдельные моменты — это установки фирмы, а не мои изобретения. И я не хочу их защищать или поносить, мне они безразличны).
Ну я вижу что самая длинная функция:
void CSewingDlg::PrepareContoursMakeEquidistants(...)
поробуйте разбить задачу на подзадачи ещё раз (разрабатывая рисование графических примитивов вы это уже сделали). Можно выделетить подзадачи в этом методе на основе private или protected методов.
Но кое-что ещё, я невооружённым глазом вижу что код не пройдёт коде ревью (если на проекте оно есть, но у вас я так понял манагер не доволен), просто видно что код messy т.е. очень много lines of code просто комменты // /**/
Здравствуйте, alexb1980, Вы писали: A>Ну я вижу что самая длинная функция: A> void CSewingDlg::PrepareContoursMakeEquidistants(...) A> поробуйте разбить задачу на подзадачи ещё раз (разрабатывая рисование графических примитивов вы это уже сделали). Можно выделетить подзадачи в этом методе на основе private или protected методов.
A> Но кое-что ещё, я невооружённым глазом вижу что код не пройдёт коде ревью (если на проекте оно есть, но у вас я так понял манагер не доволен), просто видно что код messy т.е. очень много lines of code просто комменты // /**/
Вернее самая длинная функция скорее.
int CSewingDlg::PseudoProcessAcceptedData(...)
О, даже когда фунция кочается не видно в MFC вроде можно разделять так:
////////////////////////////////////////////////////////////////////////
Здравствуйте, alexb1980, Вы писали:
A> поробуйте разбить задачу на подзадачи ещё раз (разрабатывая рисование графических примитивов вы это уже сделали). Можно выделетить подзадачи в этом методе на основе private или protected методов.
A> Но кое-что ещё, я невооружённым глазом вижу что код не пройдёт коде ревью (если на проекте оно есть, но у вас я так понял манагер не доволен), просто видно что код messy т.е. очень много lines of code просто комменты // /**/
Я уже разбил на 8 подзадач. Но это все равно слишком формально. Теперь представим, что мы принимаем этот формальный подход и дальше. Нам рекомендуют разбить код на методы длиной не больше 20 строк. Это где то 80 методов. И что, такая каша малообусловленных методов (а к ним и названия придумать будет сложно) будет лучше , чем цельный алгоритм?
Как тяжело признаться себе в своих предрассудках!
Комменты — я уже писал, что это не просто комменты, а код тестов, уже завершенных. Все в комментах сдвинуто влево намеренно, чтобы была видна временность назначения этих лесов. По вашему, я стал бы упираться убрать эти комменты, которые и написаны были специально так, чтобы их легко было почистить?
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Здравствуйте, alexb1980, Вы писали:
A>> Но кое-что ещё, я невооружённым глазом вижу что код не пройдёт коде ревью (если на проекте оно есть, но у вас я так понял манагер не доволен), просто видно что код messy т.е. очень много lines of code просто комменты // /**/
ЮЛ>Я уже разбил на 8 подзадач. Но это все равно слишком формально. Теперь представим, что мы принимаем этот формальный подход и дальше. Нам рекомендуют разбить код на методы длиной не больше 20 строк. Это где то 80 методов. И что, такая каша малообусловленных методов (а к ним и названия придумать будет сложно) будет лучше , чем цельный алгоритм?
ЮЛ>Как тяжело признаться себе в своих предрассудках!
ЮЛ>Комменты — я уже писал, что это не просто комменты, а код тестов, уже завершенных. Все в комментах сдвинуто влево намеренно, чтобы была видна временность назначения этих лесов. По вашему, я стал бы упираться убрать эти комменты, которые и написаны были специально так, чтобы их легко было почистить?
Сразу тоже ж ничего не получается, есть ещё один подход, которой редко используется в MFC (чаще в cross platform наверное). Можно использовать множественное наследование, т.е : public CDialog, protected BaseClass1, protected BaseClass2 {
таким образом эти методы (которых малопонятных у вас получается 80) хотябы не будут идти вперешку ...
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Но в любом случае это насилие над кодом, реально не приносящее особых выгод. Слишком искусственный подход, еще немного, и получится обертка каждой строки. Я ищу семантический (осмысленный) способ рефакторинга. Скажем, если бы можно было придумать определенную алгебру узлов, я взял бы ее за основу класса безоговорочно.
вот! именно поэтому рефакторинг у твоего менеджера и стоит вдвое дороже написания, что это не просто механическое разбиение, а продумывание "алгебры" задачи, в которой её реализация будет смореться просто и изящно. это целое искусство, позволяющее превратить код, который просто работает, в "совершенный", который дальше можно развивать и сопровождать, даже если тебя трактор переедет
а ты для начала ответь на один вопрос — хочешь ли ты, чтобы твои коллеги смогли сопровождать твой код после твоего ухода? да/нет
ЮЛ>Теперь с точки зрения разработки. Допустим, я пишу локальный цикл, еще не зная, насколько он будет полезен в окончательном алгоритме.
скажу сразу — твой код я считаю отличным для прототипа [1], т.е. той стадии, когда код в дальнейшем может быть либо выкинут, либо зарефакторен. противоречия с Злым Менеджером у вас возникли именно из-за рефакторинга, верно?
[1] плюс-минус всякие детали — например, константы надо делать сразу символическими, иначе потом их хрен найдёшь, а проблемные места в коде я предпочитаю помечать словами "to do: ..." в комментах, так что grep может их все найти
ЮЛ>Если вернуться к аналогии с Войной и Миром, то заведение отдельных методов для мелких частей алгоритма подобно простановке названий параграфов в оглавлении длинной главы. Насколько это будет полезно? Кто читает главы по параграфам?
аналог книги, которую ты читаешь — это exe-файл. что же касается технологий написания книги, то даже у переводчиков есть специальные средства для организации работы, а уж у писателей — и подавно
что касается Галуа, то напомню, что при жизни никто его идей не понял вообще и он помер в нищете в приюте. ты также хочешь?
я тебе могу подкинуть код на 2000 строк, который никто в мире кроме автора не понимает. но при этом он его поддерживает и развивает в течении 15 лет, и отдаёт под BSD лицензией. это код лучшего в мире алгоритма сжатия — lzma. и вот тут никто особых притензий автору не предъявляет, хочешь — разбирайся, не хочешь — отвали
если ты будешь разрабатывать свои решения, продавать их заказчику и обеспечивать сопровождение, то это будет твоё личное дело — как обеспечить их сопровождаемость (и проблемы твоего заказчика, если он переоценил твою надёжность). если же ты работаешь за зарплату, то это задача менеджера — обеспечить их сопровождаемость и независимость от первоначального разработчика. поэтому я могу только посочувствовать твоемук ЗМ, который потерял два месяца, плюс твою з/п, соцотчисления и прочие расходы совершенно зазря: если тебе самому выдать подобный кусок кода, написанный без структуры и комментов другим программистом, то ты его выкинешьб нафиг со словами "что за криворукий ламер написал!" и предложишь переписать всё заново
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Если вы посмотрите на длинный код внимательно, вы увидите массу комментов, начинающихся у левого края. Это и были мои юнит-тесты, помеченные словом PARSE и пройденные и давно закрытые по причине завершения отладки каждого из предыдущих блоков текста.
Это очень странные тесты. Обычно тесты находятся рядом с кодом и живут вместе с ним. Тесты можно прогнать при изменении и убедиться, что ничего не сломалось. Тесты можно дописать при появлении бага, на который они не срабатывали
ЮЛ>Понимаю, до некоторых такое тяжело доходит. На то они и каста манагеров. ЮЛ>"Глупость это такой ум" (А.Лебедь)
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>С волками жить. Не у вас ли тут в кодексе форума указано, что вы готовы презирать всех, кто ниже вас.
щито?
ЮЛ>Ну а если являюсь?
А если нет? Скромность украшает человека...
ЮЛ>Но равенство предполагает ведь и равенство в оплате, верно? ЮЛ>Какое же уважение может заслуживать манагер, произвольно устанавливающий зряплату себе и остальным бездельникам?
Сколько проблем оказывается исчезает, когда народ не знает о зп друг друга
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Так, а чем плохо использование запятой или вложенных тернарных операций, можете ясно ответить? Может, вам стоит написать Страустрапу, чтобы запретить запятую?
а зачем тебе запятая? Ты экономишь на скобках?
ИМХО, обычно операции разделяются точкой с запятой. Запятая применяется лишь там, где она необходима — в выражениях, блоках for и т.п.
Тут просто принцип наименьшего удивления должен работать. Если можно сделать как обычно, то зачем запутывать читающего?
Здравствуйте, BulatZiganshin, Вы писали:
BZ>вот! именно поэтому рефакторинг у твоего менеджера и стоит вдвое дороже написания, что это не просто механическое разбиение, а продумывание "алгебры" задачи, в которой её реализация будет смореться просто и изящно. это целое искусство, позволяющее превратить код, который просто работает, в "совершенный", который дальше можно развивать и сопровождать, даже если тебя трактор переедет
Вы переоцениваете способности моего "менеджера". Если бы он ценил "продумывание "алгебры" задачи", то прекрасно понимал бы, что рефакторинг лучше проводить по уже готовому и работающему коду, а не лезть со своими установками в каждый неподходящий момент, и не учить со своим молитвенником, как писать код. Причина придирок была банальна — он не мог выложить такой длинный код в SVN, поскольку всеми силами старается угодить забугорному боссу. И тут любые здравые аргументы бессильны.
Для него лучше вообще ничего не писать, чем сделать что-то работающее, но не укладывающееся в мессу о чечевичной похлебке.
BZ>а ты для начала ответь на один вопрос — хочешь ли ты, чтобы твои коллеги смогли сопровождать твой код после твоего ухода? да/нет
Во первых, никаких коллег у маня там нет и не было. Была пара хлюстов с раздутым ЧСВ и сидящих на одном "сопровождении", на которое босс из-за бугра отстегивал, и уж конечно ничего полезного с этими явными подхалимами манагера у меня быть не могло. Умные коллеги (если бы таковые были) сумели бы сопровождать мой код, но тогда они не были бы подхалимами и не напевали бы всякую благоглупость из молитвенника начальника. В коде они никогда не хотели и не разбирались (даже в стандартном), семантика их не занимала, своих алгоритмов они мне так и не представили, вся их "проверка" сводилась к формальным правилам. Что может быть интересного в общении с роботами?
BZ>скажу сразу — твой код я считаю отличным для прототипа [1], т.е. той стадии, когда код в дальнейшем может быть либо выкинут, либо зарефакторен. противоречия с Злым Менеджером у вас возникли именно из-за рефакторинга, верно?
Не только. Здесь это только явный повод указать мне на дверь. Но подноготная была — недостаток финансирования. Начиная с первого уже моего проекта, который удался (хлюст, напортачивший с гитом, тихо сопел в стену и что то там шипел), с которым манагер выезжал на Компасовскую конференцию, он тут же пошел на попятную. Проект плохо покупают, потребителей в России нет, а потому "скрипач не нужен". Это к вопросу о том, кто за что отвечает. За продажи отвечает манагер. Так почему бы ему было не урезать себе любимому зарплату за собственную недоработку?
BZ>[1] плюс-минус всякие детали — например, константы надо делать сразу символическими, иначе потом их хрен найдёшь, а проблемные места в коде я предпочитаю помечать словами "to do: ..." в комментах, так что grep может их все найти
grep вообще не применялся на фирме. Проблемные места я выделял для себя сам, и рассказал уже как. Если бы были какие то правила насчет этого, я не имел бы ничего против.
BZ>что касается Галуа, то напомню, что при жизни никто его идей не понял вообще и он помер в нищете в приюте. ты также хочешь?
Что значит, я так хочу? Так хочет руководство, хотите вы. А вы считаете, за идеи свои не стоит бороться?
BZ>если ты будешь разрабатывать свои решения, продавать их заказчику и обеспечивать сопровождение, то это будет твоё личное дело — как обеспечить их сопровождаемость (и проблемы твоего заказчика, если он переоценил твою надёжность). если же ты работаешь за зарплату, то это задача менеджера — обеспечить их сопровождаемость и независимость от первоначального разработчика. поэтому я могу только посочувствовать твоемук ЗМ, который потерял два месяца, плюс твою з/п, соцотчисления и прочие расходы совершенно зазря: если тебе самому выдать подобный кусок кода, написанный без структуры и комментов другим программистом, то ты его выкинешьб нафиг со словами "что за криворукий ламер написал!" и предложишь переписать всё заново
Да никто на фирме не стал бы писать этот проект вообще — мозги прокомпассированы инструкциями. Их обычный коронный ответ "Это невозможно", или "Это невыгодно", и эти ответы обожает манагер. Едва я только пришел на фирму, я удивился, почему окошко плагина сделано немодальным, очень некрасиво. Мне заявили на чистом глазу, "Это невозможно", потому что так сказал какой то их авторитет. Через два часа изучения их кода я сделал первую правку, которая показала, что это более чем возможно. Они даже не догадывались , что окнами в Windows можно управлять каким угодно образом.
Если вы полагаете, что именно на мне ЗМ понес самые большие убытки, то как быть с теми хлюстами, что ничего не делая, огребали вдвое больше?
Я не имею ничего против, если на фирме финансовые затруднения, и приходится урезать з/п. Но так если получать помалу, так всем помалу. Или одни выживают за счет других?
Здравствуйте, alexb1980, Вы писали:
A>Сразу тоже ж ничего не получается, есть ещё один подход, которой редко используется в MFC (чаще в cross platform наверное). Можно использовать множественное наследование, т.е : public CDialog, protected BaseClass1, protected BaseClass2 { A>таким образом эти методы (которых малопонятных у вас получается 80) хотябы не будут идти вперешку ...
Безусловно, надо (было) искать, но надо было и писать алгоритм. В сжатые сроки. Ну вот это ваше предложение, чем оно в принципе улучшило бы читаемость кода? Не говорю о трудоемкости написания этого множественного наследования, которое потом придется и переписывать в другой какой нибудь вид.
Чем оно было бы лучше цельного алгоритма, в котором все отношения под рукой, никакой фрагментации кода?
Я всегда за хорошую идею, но против фрагментации. В данном случае идея фрагментацию не перевешивает.
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Здравствуйте, alexb1980, Вы писали:
ЮЛ>Безусловно, надо (было) искать, но надо было и писать алгоритм. В сжатые сроки. Ну вот это ваше предложение, чем оно в принципе улучшило бы читаемость кода? Не говорю о трудоемкости написания этого множественного наследования, которое потом придется и переписывать в другой какой нибудь вид. ЮЛ>Чем оно было бы лучше цельного алгоритма, в котором все отношения под рукой, никакой фрагментации кода? ЮЛ>Я всегда за хорошую идею, но против фрагментации. В данном случае идея фрагментацию не перевешивает.
Тут возможно вы не правы, вот например в C# есть например sealed classes. Вот и в C++ можно что-то такое изобрести (это точно можно сделать).
Ну тогда нужно оптимизировать алгоритм, если вас не устраеват фрагментация. Тут я пока ничего предложить немогу, потомучто это трудоёмко.
Здравствуйте, alexb1980, Вы писали:
A>Ну тогда нужно оптимизировать алгоритм, если вас не устраеват фрагментация. Тут я пока ничего предложить немогу, потомучто это трудоёмко.
Именно что трудоемко. Так можете вы прямо сказать, чем так неугодна программа с цельным алгоритмом и что исключения на длину кода все же возможны? Или превышения числа строк должны караться увольнением?
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Здравствуйте, alexb1980, Вы писали:
A>>Ну тогда нужно оптимизировать алгоритм, если вас не устраеват фрагментация. Тут я пока ничего предложить немогу, потомучто это трудоёмко.
ЮЛ>Именно что трудоемко. Так можете вы прямо сказать, чем так неугодна программа с цельным алгоритмом и что исключения на длину кода все же возможны? Или превышения числа строк должны караться увольнением?
Я сам несколько раз попадал под "горячую" руку (довольны менеджеры — недовольны заказчики, не довольны менеджеры — довольны заказчики, все довольны — разработчик не доволен), скорее это просто предлог для очередного сокращения...
Здравствуйте, alexb1980, Вы писали:
ЮЛ>>Именно что трудоемко. Так можете вы прямо сказать, чем так неугодна программа с цельным алгоритмом и что исключения на длину кода все же возможны? Или превышения числа строк должны караться увольнением?
A>Я сам несколько раз попадал под "горячую" руку (довольны менеджеры — недовольны заказчики, не довольны менеджеры — довольны заказчики, все довольны — разработчик не доволен), скорее это просто предлог для очередного сокращения...
Скорее всего. Начальник с сокращениями не церемонится. При мне за год сократил двоих. Просто так — работу сделали, больше не нужны. На самом деле не нужны менеджеры — посредники, общение исполнителя с заказчиком напрямую много эффективнее.
Так как с принципиальным вопросом — возможна ли ситуация, когда природа алгоритма требует длинного кода? Или надо непременно убивать время на (далеко не всегда оправданный) поиск улучшения, свертки кода?
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Так как с принципиальным вопросом — возможна ли ситуация, когда природа алгоритма требует длинного кода? Или надо непременно убивать время на (далеко не всегда оправданный) поиск улучшения, свертки кода?
Хороший разработчик благодаря тому, что сразу не допускает длинного кода — не убивает время, а наоборот его экономит. Лично я конечно в лоб говнокод напишу да, быстрее. Процентов так на 10. Но на отладку говнокода я потрачу в 10 раз больше времени, чем если я потружусь, и не буду допускать спагетти. Да, я в курсе, что очень многие называющие себя программистами настолько стали круты в написании говнокода, что в состоянии терять на отладку спагетти кода не в 10 раз больше времени, а в 2 раза больше времени. Вот только нахрена этот навык развивать? Когда если выработать привычку сразу писать максимально нормально, то прирост производительности работы будет гораздо больше. Да, я в курсе, что таким навыком мало кто обладает. Да, я в курсе, что в российских институтах нормально писать не учат. Да, я в курсе, что очень тяжело попасть на проект с нормальным кодом, где можно этому учиться. Я в курсе, что даже чтение Макконела не выработает эту привычку. И я в курсе что это очень тяжело, этому можно учиться всю жизнь, и все равно не достигнешь совершенства.
Но также я в курсе, что умение сразу писать код дает больший прирост производительности труда, чем любые знания деталей реализации множественного наследования, умение разворачивать списки на бумажке, умение компилировать код в уме и заучивание деталей реализации всяких I = I++ + ++I. А раз это дает повышение производительности труда — именно это и следует в первую очередь совершенствовать, причем совершенствовать постоянно. Да, умение писать код нормально никак не поможет пройти 95 процентов собеседований, а зачастую будет наоборот зачтено в минус. Вот только это поможет потом в реальной работе, в умении всегда без проблем выдерживать сроки, или хотя бы выдавать хоть что то приемлемое в сроки.
Здравствуйте, elmal, Вы писали:
E>Хороший разработчик благодаря тому, что сразу не допускает длинного кода ...
Совершенно с вами согласен, так вот и я не пишу длинного кода. Почти никогда. Но это не избавляет от случаев, когда такое приходится писать. О том и был вопрос —
Так как с принципиальным вопросом — возможна ли ситуация, когда природа алгоритма требует длинного кода? Или надо непременно убивать время на (далеко не всегда оправданный) поиск улучшения, свертки кода?
Значит ли то, что ваш хороший разработчик "сразу не допускает длинного кода" то, что он его вообще не пишет? Никакого. И в чем тогда выражается экономия?
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Так как с принципиальным вопросом — возможна ли ситуация, когда природа алгоритма требует длинного кода? Или надо непременно убивать время на (далеко не всегда оправданный) поиск улучшения, свертки кода?
Я не алгоритмист. Но я не знаю ситуаций, когда природа алгоритма требует длинного кода. Когда был студентом, я считал, что таких алгоритмов, которые требуют длинного кода — до хрена и больше. С опытом я уже не очень могу представить таких ситуаций.
ЮЛ>Значит ли то, что ваш хороший разработчик "сразу не допускает длинного кода" то, что он его вообще не пишет? Никакого. И в чем тогда выражается экономия?
Хороший разработчик не пишет длинный код. Он автоматом его разбивает в тот момент, когда он начинает казаться длинным. Просто на рефлексах это делает, не задумываясь и практически не тратя времени.
Здравствуйте, elmal, Вы писали:
E>Я не алгоритмист. Но я не знаю ситуаций, когда природа алгоритма требует длинного кода. Когда был студентом, я считал, что таких алгоритмов, которые требуют длинного кода — до хрена и больше. С опытом я уже не очень могу представить таких ситуаций.
Ну вот вам такой алгоритм. Не понял, с каким таким опытом, если вы не алгоритмист?
E>Хороший разработчик не пишет длинный код. Он автоматом его разбивает в тот момент, когда он начинает казаться длинным. Просто на рефлексах это делает, не задумываясь и практически не тратя времени.
Надеюсь не автоматом Калашникова? Просто на рефлексах... — обезьяна какая то вырисовывается.
Я так понимаю, вы говорите о кодах уже заранее отрепетированных в голове. Здесь алгоритм писался с нуля и до последнего момента был не определен окончательно.
ЮЛ>Надеюсь не автоматом Калашникова? Просто на рефлексах... — обезьяна какая то вырисовывается.
Хамство какое-то вырисовывается.
ЮЛ>Я так понимаю, вы говорите о кодах уже заранее отрепетированных в голове. Здесь алгоритм писался с нуля и до последнего момента был не определен окончательно.
Алгоритмист-Юрий, вы можете сказать, например, какая сложность у вашего алгоритма?
Нормальный подход в данном случае:
1. спецификация или статья, описывающая алгоритм, чтобы другие люди могли ее прочитать и покритиковать
2. (опционально) эскизный проект на питоне или чем-нибудь подобном (хотя иногда это невозможно и приходится использовать C++ для эскизного проекта из за специфики задачи), чтобы можно было отладить алгоритм (внести изменения в спеку, если нужно)
3. реализация алгоритма (не в классе CSewingDlg а отдельно), чтобы можно было потестить, например подготовить набор верифицируемых кейсов в текстовом файле и прогнать тест на нем
4. заюзать полученый код в реальном приложении
Так выглядит работа нормального алгоритмиста/ресерчера в IT компании, JFYI.
Получается быстрее (так как задачи решаются по отдельности, сначала дизайн алгоритма, потом кодинг) и качественней (покрытие тестами, естественным образом получается отдельная сущьность в коде, отвечающая за алгоритм).
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Скажите плз, на какую зарплату был взят сотрудник? Москва?
Т.е. для 40 тыс. в месяц наверно неплох, для 80 уже плох.
Здравствуйте, antonio_banderas, Вы писали:
_>Здравствуйте, GhostCoders, Вы писали:
GC>>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
_>Скажите плз, на какую зарплату был взят сотрудник? Москва? _>Т.е. для 40 тыс. в месяц наверно неплох, для 80 уже плох.
Я думаю, ТС вам не ответит. Это провинция, делите все на 2. Последние месяцы жаба душила манагера платить и 10 тыс., но выхода не было — других мест не найти.
Тут еще момент. Дело в том, что в середине лета я сломал ногу, вынужден был потерять месяц и, само собой, за свой счет. Полностью потратив на аренду все свои сбережения, по выходе на работу я вскоре был осчастливлен новостью о моем скором сокращении. При том что заказчик был извещен о задержке с разработкой и согласен был ждать, ждать не изволил манагер. Он несколько раз разрывал договор с заказчиком, несмотря на то, что первый этап я вопреки всему успешно завершил; он меня уволил (забавные подробности есть в др.ветке), и второй этап, также пройденный без замечаний, я выполнил с заказчиком уже после увольнения, в прямом с ним общении — сейчас как раз неделя тестов у заказчика.
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Я думаю, ТС вам не ответит. Это провинция, делите все на 2. Последние месяцы жаба душила манагера платить и 10 тыс., но выхода не было — других мест не найти.
Ты не преувеличиваешь? У нас кассир в супере получает рублей 12 минимум. Город 150 тыс примерно
ЮЛ>Тут еще момент. Дело в том, что в середине лета я сломал ногу, вынужден был потерять месяц и, само собой, за свой счет.
Почему не по больничному?
Здравствуйте, Юрий Лазарев, Вы писали:
ЮЛ>Я думаю, ТС вам не ответит. Это провинция, делите все на 2. Последние месяцы жаба душила манагера платить и 10 тыс., но выхода не было — других мест не найти.
Ууу, если провинция, то понятно. Полное отсутствие даже начальной культуры разработки сплошняком, учиться не у кого зачастую, только по книжкам. С увольнением могу поздравить, как раз хороший случай что то поменять в жизни. Рекомендую валить в Москву на юниорскую позицию если возраст позволяет, года через полтора код будешь писать приемлемый. Да, придется на дошираке посидеть, возможно снимать комнату и тому подобное. Но затем, как будет опыт, уже по провинциальным меркам будешь мегакрут, так что можешь попробовать вернуться (я правда вернувшихся не знаю, Москва это такое место, откуда не возвращаются, либо там остаются — либо в буржундию валят), либо уже на съем однушки хватать будет, и будут перспективы роста, если напрячься и оставить провинциальный уровень потребления, то можно и квартиру купить. В любом случае — через полтора года поймешь что был не прав относительно своего текущего кода, материться на текущий код будешь еще круче, чем здесь матерятся, научишься писать его быстрее, и в то же время понятнее.
Здравствуйте, Юрий Лазарев, Вы писали:
GC>>>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
_>>Скажите плз, на какую зарплату был взят сотрудник? Москва?
ЮЛ><skipped> Последние месяцы жаба душила манагера платить и 10 тыс., но <skipped>
Несмотря на всю свою говёность, имхо, для цены 10 тыс. рублей в месяц — код неплох.
Не силён в плюсах.
Подскажи, пожалуйста, есть ли в приведенном коде паттерны?
На мой взгляд, вышеприведенный код замусорен комментариями и сбит в один файл. Мне думается, чтобы код был бы читабельнее, если код можно было бы разнести по разным файликам, обрамить его в регионы, вынести в какие-нибудь header (или как там у вас в С++)?
Также я не увидел ни одного try...catch... finally, что очень и очень ужасно.
Здравствуйте, wety, Вы писали:
W>Здравствуйте, Кодт, Вы писали:
W>Не силён в плюсах. W>Подскажи, пожалуйста, есть ли в приведенном коде паттерны?
Там и паттерны, и маттерны. Маттернов больше
W>На мой взгляд, вышеприведенный код замусорен комментариями и сбит в один файл. Мне думается, чтобы код был бы читабельнее, если код можно было бы разнести по разным файликам, обрамить его в регионы, вынести в какие-нибудь header (или как там у вас в С++)?
Хотя бы разбить на много функций.
W>Также я не увидел ни одного try...catch... finally, что очень и очень ужасно.
Это, как раз, не ужасно.
Во-первых, RAII избавляет от большинства finally. Это же С++.
Во-вторых, штатное исключение там — это исчерпание памяти. Но эта ситуация сама по себе настолько кровава, что нет существенной разницы между просто завершением программы и её вылетом с треском по terminate.
А все остальные исключительные ситуации — нештатные (например, отсутствие контрола с нужным идентификатором, или нулевой указатель из ком-объекта), и должны, максимум, покрываться ассертами и/или тестами.
Только если не стоит задача сделать программу устойчивой ко внутренним сбоям.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох. GC>...
Доказывать бесполезно, введите code review. 2 месяца и свой код от чужого перестанете отличать. Испытано на себе.
Здравствуйте, Кодт, Вы писали:
К>Здравствуйте, GhostCoders, Вы писали:
GC>>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
в описаниях функции не должно быть тела если это только не одна строка.
— иначе не оттестируешь
Ну и судя по стилю юнит тесты вообше не писались.
блоки ифоф без скобок — за такое бить надо железной линейкой по руках — как товаришь собирается ставить точку остановки при дебаге? Значить приличные системы не поддерживал.
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Посмотрел небольшой кусочек.
Если оценивать по внешнему виду — да, не фонтан (смешение гуи и логики,
длинные методы, местами магические числа, ...).
Но если смотреть на суть (сложная вычислительная геометрия), то код весьма и весьма неплох
и производит впечатление реально работающего. Бывает намного-намного хуже, и дело даже
не в "лапше" и однобуквенных переменных, а в потенциальных багах, вызванных непониманием
особенностей вещественных чисел, логических ошибках, не учтением краевых случаев, делений на 0 и т.п.
В целом, возникло ощущение, что крутого математика-алгоритмиста зачем-то посадили писать продакшн-код.
На мой взгляд, его дело — придумывать и описывать (может быть, в виде прототипа) алгоритмы,
которые 99% "стильно пишущих" программистов даже не поймут, не говоря уже о том, чтобы придумать
это самим (к RSDN эта статистика не относится ).
Это и не был продакшн-код, в смысле, моему заказчику нужен был работающий бинарник, а не код. Другое дело, — начальству нужен был "красивый" код, чтобы показать боссу. Я не против писать "совершенный" код, только это делается вовсе не быстро, и не всегда способствует быстроте разработки. Ну и потворствовать лентяям, которым надо "с первого взгляда" понять, где баг, тоже нет резона. Во всяком случае, этот код в сравнении с , например, boost::Graph при поиске бага будет ничем не хуже (а Graph ничем не лучше).
Здравствуйте, GhostCoders, Вы писали:
GC>Данный код был написан моим сотрудником. Мне этот код не нравится, но он мне отвечает что я субъективен и его код неплох.
Сколько по времени заняло написание этого кода? Очень важный вопрос. Был ли написан на скорую руку за 1 месяц -- то неплохо. Если же месяца 2-3 ваял -- то мог бы и отрефакторить.
Перекуём баги на фичи!
(в смысле — всё плохо!)
).
