Re[8]: Низкая квалификация работников
От: Ulin США  
Дата: 18.11.11 07:01
Оценка:
Здравствуйте, grosborn, Вы писали:

>> То есть нечто, больше относящееся к форматированию. Имена классов типа "BinRes", "a", "b", "ptr" и тп. это в моем понимании просто кривой код, подлежащий рефакторингу.

>> Использование хедера #include <string> и при этом параметры типа wchar_t скорее всего тоже кривость. Но тут уже надо разбираться с конкретным случаем.

G>Оппа. Рефакторинг это оказывается переименование идентификаторов. Чудненько, а то я думаю чего это народ все обсуждает, митингует. Теперь я в теме тоже могу обсуждать рефакторинг


А что вас удивляет? Да, рефакторинг, это в том числе и переименование идентификаторов
Re[7]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 07:02
Оценка: -1 :)
Здравствуйте, Ulin, Вы писали:
U>То есть нечто, больше относящееся к форматированию. Имена классов типа "BinRes", "a", "b", "ptr" и тп. это в моем понимании просто кривой код, подлежащий рефакторингу.
рефакторинг это пустая трата времени. это лишь адаптация одного стиля под другой. если программа работает, значит она выполняет свои ф-ии и не надо в нее лезть своими ручками. если настолько кривая, что не работает, то ее и не существует.
Re[8]: Низкая квалификация работников
От: Ulin США  
Дата: 18.11.11 07:20
Оценка:
Здравствуйте, __kot2, Вы писали:

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


Я бы, все таки, различал понятия другого стиля и кривого кода. Другой стиль рефакторить, конечно же, смысла нет совершенно никакого. А вот кривой код резко увеличивает стоимость внесения изменений. И его рефакторить часто просто приходится. Особенно когда добавление фичи все равно может много чего поломать, тут уж грех не воспользоваться и не отрефакторить изменяемый модуль.
Re[8]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 07:23
Оценка:
Здравствуйте, __kot2, Вы писали:
__>Здравствуйте, Ulin, Вы писали:
U>>То есть нечто, больше относящееся к форматированию. Имена классов типа "BinRes", "a", "b", "ptr" и тп. это в моем понимании просто кривой код, подлежащий рефакторингу.
__>рефакторинг это пустая трата времени. это лишь адаптация одного стиля под другой. если программа работает, значит она выполняет свои ф-ии и не надо в нее лезть своими ручками. если настолько кривая, что не работает, то ее и не существует.
добавлю, чтобы правильно поняли
в программировании есть сущности, которые взаимодействуют по определенным интерфейсам. сущности группируются в другие сущности, взаимодействующих с другими группами по другим интерфейсам.
если у нас нет какого-то нужного нам интерфейса, значит нам нужно его сделать, введя новые обьекты и использовав интерфейсы к существующим обьектам
если какой-то интерфейс не работает, значит у нас нет рабочего интерфейса — мы либо правим уже существующий код, либо пишем свой код, смотря что быстрее.
если интерфейс работает, то какая разница как он работает?
расширение интерфейса — дело обычно гиблое, так как обьекты должны быть атомарными. и если кто-то навалил все в одну кучу, то нам не стоит заниматься тем же.
Re[9]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 07:25
Оценка:
Здравствуйте, Ulin, Вы писали:
U>Я бы, все таки, различал понятия другого стиля и кривого кода. Другой стиль рефакторить, конечно же, смысла нет совершенно никакого. А вот кривой код резко увеличивает стоимость внесения изменений. И его рефакторить часто просто приходится. Особенно когда добавление фичи все равно может много чего поломать, тут уж грех не воспользоваться и не отрефакторить изменяемый модуль.
давайте вы приведете пример "некривого кода", а я скажу почему он по-моему мнению кривой и как его нужно изменить? мне аж самому интересно, что из этого получится.
Re[10]: Низкая квалификация работников
От: Ulin США  
Дата: 18.11.11 07:37
Оценка:
Здравствуйте, __kot2, Вы писали:

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


Ну вот, например, вполне ровный, на мой взгляд, код из либы JRTPLIB (http://research.edm.uhasselt.be/~jori/page/index.php?n=CS.Jrtplib):


class JRTPLIB_IMPORTEXPORT RTCPSenderReportInfo
{
public:
    RTCPSenderReportInfo():ntptimestamp(0,0),receivetime(0,0)        { hasinfo = false; rtptimestamp = 0; packetcount = 0; bytecount = 0; }
    void Set(const RTPNTPTime &ntptime,uint32_t rtptime,uint32_t pcount,
             uint32_t bcount,const RTPTime &rcvtime)            { ntptimestamp = ntptime; rtptimestamp = rtptime; packetcount = pcount; bytecount = bcount; receivetime = rcvtime; hasinfo = true; }
    
    bool HasInfo() const                            { return hasinfo; }
    RTPNTPTime GetNTPTimestamp() const                    { return ntptimestamp; }
    uint32_t GetRTPTimestamp() const                    { return rtptimestamp; }
    uint32_t GetPacketCount() const                        { return packetcount; }
    uint32_t GetByteCount() const                        { return bytecount; }
    RTPTime GetReceiveTime() const                        { return receivetime; }
private:
    bool hasinfo;
    RTPNTPTime ntptimestamp;
    uint32_t rtptimestamp;
    uint32_t packetcount;
    uint32_t bytecount;
    RTPTime receivetime;
};


То, что в моем понимании является стилем — тело функций в хедере, нет пробелов между параметрами функций — не совпадает с тем, как пишу, например, я. Тем не менее рефакторить тут в моем понимании нечего — это действительно просто другой стиль. И если девелопер приходит на проект, где все сорцы написаны так — он просто должен писать так же, хоть это и может противоречить его стилю.
Re[11]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 08:17
Оценка: -1
Здравствуйте, Ulin, Вы писали:
U>Ну вот, например, вполне ровный, на мой взгляд, код из либы JRTPLIB (http://research.edm.uhasselt.be/~jori/page/index.php?n=CS.Jrtplib):
пример, конечно, мал, я бы хотел некую реализацию какого-то класса,
поэтому давайте возьму лучше rtcpcompoundpacket
начнем с rtcpcompoundpacket.h

>>/** Creates an RTCPCompoundPacket instance from the data in \c rawpack, installing a memory manager if specified. */

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

>>RTCPCompoundPacket(uint8_t *packet, size_t len, bool deletedata = true, RTPMemoryManager *memmgr = 0);

так это С++ или С? почему packed голым указателем? почему указатель = 0? хотя бы NULL нужно писать
составные имена переменных стоило бы для лучшей читабельности разделять подчеркиваниями deletedata -> delete_data

>> int GetCreationError() { return error; }

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

что это за паскалевское именование?

>> /** Starts the iteration over the individual RTCP packets in the RTCP compound packet. */

>> void GotoFirstPacket() { rtcppackit = rtcppacklist.begin(); }
>> RTCPPacket *GetNextPacket() { if (rtcppackit == rtcppacklist.end()) return 0; RTCPPacket *p = *rtcppackit; rtcppackit++; return p; }

сделали свой интерфейс для итератора. а зачем?

>> size_t GetCompoundPacketLength() { return compoundpacketlength; }

почему метод неконстантный?

>> void ClearPacketList(); — этот метод вообще не нужен, если сделать контейнер не std::list<RTCPPacket *>, а std::list<SmartPtr<RTCPPacket> >


судя по коду реализации RTCPCompoundPacket можно было бы сделать константным классом — он инициализируется данными в конструкторе и всегда остается неизменным. везде забыты константные идентификаторы, соотв-но вместо GotoFirstPacket(), GetNextPacket() можно было просто выдавать константный референс на контейнер

>>#ifdef RTPDEBUG

>> void Dump();
>>#endif // RTPDEBUG <- коммент для дебилов?

>>std::list<RTCPPacket *>::const_iterator

слабо было обявить сразу typedef'ом, чтобы по 100 раз не писать std::list<RTCPPacket *> ?

переходим на С++ файл
>>RTCPCompoundPacket::RTCPCompoundPacket(RTPRawPacket &rawpack, RTPMemoryManager *mgr) : RTPMemoryObject(mgr)
>>{
>> compoundpacket = 0;
>> compoundpacketlength = 0;
>> error = 0;
народу бы почитать про списки инициализации


>> error = ParseData(data,datalen);

>> if (error < 0)
>> return;
суперработа с исключительными ситуациями

rawpack.ZeroData();
он в конструкторе изменяет состояние передаваемого обьекта?????


>> compoundpacket = packet;

>> compoundpacketlength = packetlen;
>> deletepacket = deletedata;
такие повторяющиеся блоки иногда имеет смысл выделить в структурку. вообще, там три конструктора с дублированием инициализации этой структурки. если кто-нить добавить туда еще что-то, то надо будет не забыть подписать инициализацию о всех конструкторах — неудобно


>> switch (rtcphdr->packettype)

>> {
>> case RTP_RTCPTYPE_SR:
>> p = RTPNew(GetMemoryManager(),RTPMEM_TYPE_CLASS_RTCPSRPACKET) RTCPSRPacket(data,length);
>> break;
>> case RTP_RTCPTYPE_RR:
>> p = RTPNew(GetMemoryManager(),RTPMEM_TYPE_CLASS_RTCPRRPACKET) RTCPRRPacket(data,length);
>> .........
это стоило бы выделить в отдельный метод

+ по дурацки построена работа с менеджером памяти. что-то мне подсказывает что в С++ его не нужно тащить в каждый класс
даже простое удаление жутко выглядит
>> RTPDelete(*it,GetMemoryManager());
вот забудешь так, удалишь как обычный обьект — и никто ведь не предупредит даже! или выделишь просто через new. потенциальное море граблей.

вывод — код — полное гавно. но оно (скорее всего) рабочее! а, значит, код годный и все замечательно. ну писал его сишник, что ж теперь. главное, чтобы все работало
Re[12]: Низкая квалификация работников
От: Ulin США  
Дата: 18.11.11 08:40
Оценка:
Здравствуйте, __kot2, Вы писали:

__>вывод — код — полное гавно. но оно (скорее всего) рабочее! а, значит, код годный и все замечательно. ну писал его сишник, что ж теперь. главное, чтобы все работало


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

В целом могу подытожить так: любому рефакторингу есть свое время и место.
1. Если на проекте затишье — почему-бы не замутить рефакторинг по, например, вашим замечаниям, в отдельном бранче, стабилизировать его и подлить в транк?
2. Если рынок требует фичи или завтра, или никогда — лепим копипасты, всячески портим себе карму, но успеваем в срок, в надежде на п.1
3. Если новая фича никак не лезет в существующую реализацию — делаем минимально возможный рефакторинг. Тут просто и выхода другого может не быть. Альтернатива — заплатки на заплатки, и, как следствие, время стабилизации умножаем на 3, хрупкость системы еще больше увеличена — следующие фичи уже умножат время дебага на 10. Если, конечно, не работает п.2
Re[13]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 08:54
Оценка:
Здравствуйте, Ulin, Вы писали:
U>Но, вот, например, повторяющихся куска, которых было изначально два, резко понадобилось двадцать. Что будем делать? Продолжать копипаст, или, все-таки, вынесем этот код в одно место? То есть, таки сделаем рефакторинг?
я стараюсь вообще не делать повторов ни кода ни данных. соотв-но в моей части повторов не будет. у других — да хоть 20.

U>1. Если на проекте затишье — почему-бы не замутить рефакторинг по, например, вашим замечаниям, в отдельном бранче, стабилизировать его и подлить в транк?

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

U>2. Если рынок требует фичи или завтра, или никогда — лепим копипасты, всячески портим себе карму, но успеваем в срок, в надежде на п.1

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

U>3. Если новая фича никак не лезет в существующую реализацию — делаем минимально возможный рефакторинг. Тут просто и выхода другого может не быть. Альтернатива — заплатки на заплатки, и, как следствие, время стабилизации умножаем на 3, хрупкость системы еще больше увеличена — следующие фичи уже умножат время дебага на 10. Если, конечно, не работает п.2

я делал рефакторинг. тогда, когда не понимал, как проект вообще делать. попробовал одним способом, не получилось, понял, что нужно с другого угла зайти. рефакторинг говорит о том, что проект делают недостаточно компетентные люди. периодический или хронический рефакторинг — проект не будет доделан до нормального состояния никогда.
Re[13]: Низкая квалификация работников
От: grosborn  
Дата: 18.11.11 09:00
Оценка:
> 3. Если новая фича никак не лезет в существующую реализацию — делаем минимально возможный рефакторинг.

То есть максимально-возможно переименовываем идентификаторы.
бу-га-га-га....
Posted via RSDN NNTP Server 2.1 beta
Забанен на рсдн за применение слова "Маргинал"
Re[11]: Низкая квалификация работников
От: alzt  
Дата: 18.11.11 10:59
Оценка:
Здравствуйте, alexsoff, Вы писали:

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


Ну а как ты хотел? "О работе" это же не совсем профильный форум, тут не надо быть экспертом, чтобы отвечать. Так что не стоит удивляться ответам в стиле "у тебя кривые руки", хорошо хоть мудаком не назвали. Вон Octothorp-у в недавней теме про МММ повезло меньше. см. тут
Автор: pvirk
Дата: 17.11.11
.
Re[5]: Низкая квалификация работников
От: alpha21264 СССР  
Дата: 18.11.11 11:09
Оценка:
Здравствуйте, iiice, Вы писали:

I>Товарищ Маузер, очевидно Вы не в полной мере осознвёте все последствия использования глобальных переменных. Размер программы тут не при чём. А вот портабельность кода убивается на раз — на некоторых платформах глобальность, а порой и синглтоны, технологически недопустимы. Пример — android. И пожалуйста, поосторожнее с поучениями молодёжи.


Если мне придется писать на Андроид, то глобальные переменные не будут самой большой проблеммой.
Там вообще Ява, а я пишу на С++

I>PS. О себе. Вырос, набрался опыта, осознал, ужаснулся, весь опыт выкинул нахрен как вредный, и больше накапливать не буду. Темпы сейчас такие...


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

Течёт вода Кубань-реки куда велят большевики.
Re[13]: Низкая квалификация работников
От: Aviator  
Дата: 18.11.11 11:32
Оценка: :)
Здравствуйте, Erop, Вы писали:

E>Здравствуйте, MxMsk, Вы писали:


MM>>Мне-то как раз ясно, что ты хочешь сделать хорошо.


E>Мне, например, тоже ясно, что он хочет сделать хорошо. Но чем боьше alexsoff описывает своё видинее ситуации, тем боьше я сомневаюсь, что у него получится сделать лучше, а не хуже...

Имея в качестве мотивации усложнение проекта для увеличения порога вхождения можно улучшить проект? Тут изначально позиция ухудшить проект и за счёт этого стать "незаменимым".
Re[12]: Низкая квалификация работников
От: AndrewJD США  
Дата: 18.11.11 11:52
Оценка:
Здравствуйте, __kot2, Вы писали:

>>>RTCPCompoundPacket(uint8_t *packet, size_t len, bool deletedata = true, RTPMemoryManager *memmgr = 0);

__>так это С++ или С? почему packed голым указателем?
А что уже в С++ отменили голые указатели?

>>почему указатель = 0? хотя бы NULL нужно писать

Хотя бы потому что это по стандарту

>>> int GetCreationError() { return error; }

__>о, ужас, они про исключения слышали?
Расскжите это Гуглу

__>что это за паскалевское именование?

CamelCase ничем не хуже других конвенций и человек может писать так как это принято в код-снадарте той конторы где он работает.


__>судя по коду реализации RTCPCompoundPacket можно было бы сделать константным классом — он инициализируется данными в конструкторе и всегда остается неизменным. везде забыты константные идентификаторы, соотв-но вместо GotoFirstPacket(), GetNextPacket() можно было просто выдавать константный референс на контейнер


>>>#ifdef RTPDEBUG

>>> void Dump();
>>>#endif // RTPDEBUG <- коммент для дебилов?

Если будут куча вложенных #ifdef, то "дебилами" уже можно назвать кто так не делает.
"For every complex problem, there is a solution that is simple, neat,
and wrong."
Re[13]: Низкая квалификация работников
От: __kot2  
Дата: 18.11.11 12:05
Оценка:
Здравствуйте, AndrewJD, Вы писали:
AJD>Здравствуйте, __kot2, Вы писали:
>>>>RTCPCompoundPacket(uint8_t *packet, size_t len, bool deletedata = true, RTPMemoryManager *memmgr = 0);
__>>так это С++ или С? почему packed голым указателем?
AJD>А что уже в С++ отменили голые указатели?
вот видите, у вас свое мнение насчет правильного кода. и так со всеми. в чем отличие моего мнения от вашего?
я считаю, что в С++ необходимости в голых указателях, тем более на uchar* нету вообще. для этого есть контейнеры — там внутри и len и проверка индексов (если ее дописать ).

>>>почему указатель = 0? хотя бы NULL нужно писать

AJD>Хотя бы потому что это по стандарту
NULL лучше глазами выделяется, сразу видно что это указатель. к тому же проще будет привести к новому стандарту просто заменив все NULL на nullptr, а = 0 придется глазами выглядывать везде

>>>> int GetCreationError() { return error; }

__>>о, ужас, они про исключения слышали?
AJD>Расскжите это Гуглу
зачем? тем более гугл большой. каждому рассказывать? если обьект может принимать невалидное состояние это всегда гемор. тогда иногда обусловленный чем-то, а иногда так сделали, потому что не понимают, что это гемор.

__>>что это за паскалевское именование?

AJD>CamelCase ничем не хуже других конвенций и человек может писать так как это принято в код-снадарте той конторы где он работает.
удобно бывает по именованию идентификатора сразу понимать что это такое. внтурениий парсер в голове так быстрее работает. мое именование — классы — с большой буквы, методы и данные с маленькой.

AJD>Если будут куча вложенных #ifdef, то "дебилами" уже можно назвать кто так не делает.

я думаю коммент написал какой-нить вижуал ассист, потому что человеку бы он в голову никогда не пришел — соотв-ующий ему ifdef всего строчкой выше.
Re[8]: Низкая квалификация работников
От: Aviator  
Дата: 18.11.11 12:58
Оценка:
Здравствуйте, Undying, Вы писали:

U>Здравствуйте, alexsoff, Вы писали:


A>>Разве применение паттернов, может превратить мешанину кода в еще большее гОвно,


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


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

A>>неужели ты ставишь мнение таких метров программирования как Фаулер, Эванс,Бэк по структурированию и паттерированию программ в грош?

U>Паттерны это инструмент. Инструмент можно применить правильно, тогда он упростит код и вхождение в код в том числе. Можно неправильно, тогда код кардинально усложнится.
Чем сильнее инструмент, тем больше последствий от ошибок и неверного применения.
Re[8]: Низкая квалификация работников
От: Aviator  
Дата: 18.11.11 15:13
Оценка: 1 (1) +2
Здравствуйте, Wolverrum, Вы писали:

W>Здравствуйте, alexsoff, Вы писали:


A>>Разве применение паттернов, может превратить мешанину кода в еще большее гОвно


W>ДА!!!

W>Для применения паттернов еще и меру знать надо, которой не имеет большинство девов.
Паттерны сами собой появляются в процессе шлифовки кода. Сама система паттернов как таковая имеет смысл только для структуризации существующих навыков и знаний разработчика, а не целенаправленного "внедрения паттернов".
Re[4]: Низкая квалификация работников
От: Aviator  
Дата: 18.11.11 15:34
Оценка:
Здравствуйте, Klatu, Вы писали:

K>Здравствуйте, Олег К., Вы писали:


K>>>Рефакторить или отказаться с этим работать, других вариантов для уважающего себя профи просто нет


ОК>>Виден совет непрофессионала. Код которому 10-20-30 лет и который писала куча людей которые больше не работают в компании ты тоже начнешь сразу переписывать или отказываться с ним работать?


K>Точно, как это я не сообразил. Настоящие "профи" код никогда не рефакторят

K>Я всегда переписываю код, который крив. Неважно кто его писал и когда.
Ты представляешь объём работ для переписывания кода, который писала команда людей лет этак 10? И не паттерны внедряла, а выполняла бизнес задачи? И когда полное знание о работе ряда компонент потеряны во времени? Ты думаешь майкрософт очень довольна кодом NT и с огромным удовольствием не выкинула бы нафик её на помойку, переписав на новые рельсы с учётом текущих условий?
Re[14]: Низкая квалификация работников
От: Erop Россия  
Дата: 18.11.11 15:55
Оценка:
Здравствуйте, Aviator, Вы писали:


A>Имея в качестве мотивации усложнение проекта для увеличения порога вхождения можно улучшить проект? Тут изначально позиция ухудшить проект и за счёт этого стать "незаменимым".


Ну по всякому бывает. Зависит от того, что чел сможет после "вхождения" делать.
Все эмоциональные формулировки не соотвествуют действительному положению вещей и приведены мной исключительно "ради красного словца". За корректными формулировками и неискажённым изложением идей, следует обращаться к их автором или воспользоваться поиском
Re[12]: Низкая квалификация работников
От: Abyx Россия  
Дата: 18.11.11 18:28
Оценка: +2
Здравствуйте, __kot2, Вы писали:

>>>/** Creates an RTCPCompoundPacket instance from the data in \c rawpack, installing a memory manager if specified. */

__>куски кода с такими комментами неудобно комментить — они всегда рождают дополнительные маты при портировании и отладке, так как обычно при портировании ничего сразу не собирается, надо это делать частями потихоньку

откройте для себя #if 0

>>>RTCPCompoundPacket(uint8_t *packet, size_t len, bool deletedata = true, RTPMemoryManager *memmgr = 0);

__>так это С++ или С? почему packed голым указателем? почему указатель = 0? хотя бы NULL нужно писать
__>составные имена переменных стоило бы для лучшей читабельности разделять подчеркиваниями deletedata -> delete_data

да, в C++ используются raw non-owning указатели

>>> int GetCreationError() { return error; }

__>о, ужас, они про исключения слышали?
__>получается, можно создать класс имеющий невалидное состоние. зачем? это очень неудобно

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

>>> /** Starts the iteration over the individual RTCP packets in the RTCP compound packet. */

>>> void GotoFirstPacket() { rtcppackit = rtcppacklist.begin(); }
>>> RTCPPacket *GetNextPacket() { if (rtcppackit == rtcppacklist.end()) return 0; RTCPPacket *p = *rtcppackit; rtcppackit++; return p; }

__>сделали свой интерфейс для итератора. а зачем?


итератор и "итератор STL" это разные вещи.

>>> size_t GetCompoundPacketLength() { return compoundpacketlength; }

__>почему метод неконстантный?

потому что он завтра он будет менять члены класса

>>> void ClearPacketList(); — этот метод вообще не нужен, если сделать контейнер не std::list<RTCPPacket *>, а std::list<SmartPtr<RTCPPacket> >


SmartPtr — это какой? shared_ptr? так он тяжелый и время жизни объекта будет недетерминированное

>>>#ifdef RTPDEBUG

>>> void Dump();
>>>#endif // RTPDEBUG <- коммент для дебилов?

так в coding style guide написано

>>>std::list<RTCPPacket *>::const_iterator

__>слабо было обявить сразу typedef'ом, чтобы по 100 раз не писать std::list<RTCPPacket *> ?

а что, сложно читать?

__>переходим на С++ файл

>>>RTCPCompoundPacket::RTCPCompoundPacket(RTPRawPacket &rawpack, RTPMemoryManager *mgr) : RTPMemoryObject(mgr)
>>>{
>>> compoundpacket = 0;
>>> compoundpacketlength = 0;
>>> error = 0;
__>народу бы почитать про списки инициализации

вопрос вкуса, в данном случае список инициализации ничего не поменял бы

>>> error = ParseData(data,datalen);

>>> if (error < 0)
>>> return;
__>суперработа с исключительными ситуациями



__> rawpack.ZeroData();

__>он в конструкторе изменяет состояние передаваемого обьекта?????

ну и что? rawpack.GetData() тоже могло бы менять состояние rawpack

>>> compoundpacket = packet;

>>> compoundpacketlength = packetlen;
>>> deletepacket = deletedata;
__>такие повторяющиеся блоки иногда имеет смысл выделить в структурку. вообще, там три конструктора с дублированием инициализации этой структурки. если кто-нить добавить туда еще что-то, то надо будет не забыть подписать инициализацию о всех конструкторах — неудобно

не критично

>>> switch (rtcphdr->packettype)

>>> {
>>> case RTP_RTCPTYPE_SR:
>>> p = RTPNew(GetMemoryManager(),RTPMEM_TYPE_CLASS_RTCPSRPACKET) RTCPSRPacket(data,length);
>>> break;
>>> case RTP_RTCPTYPE_RR:
>>> p = RTPNew(GetMemoryManager(),RTPMEM_TYPE_CLASS_RTCPRRPACKET) RTCPRRPacket(data,length);
>>> .........
__>это стоило бы выделить в отдельный метод

+1

__>+ по дурацки построена работа с менеджером памяти. что-то мне подсказывает что в С++ его не нужно тащить в каждый класс

__>даже простое удаление жутко выглядит
>>> RTPDelete(*it,GetMemoryManager());
__>вот забудешь так, удалишь как обычный обьект — и никто ведь не предупредит даже! или выделишь просто через new. потенциальное море граблей.

delete statement должен быть запрещен вне библиотек, по coding style guide

__>вывод — код — полное гавно. но оно (скорее всего) рабочее! а, значит, код годный и все замечательно. ну писал его сишник, что ж теперь. главное, чтобы все работало


вывод — вы плохо знаете С++
In Zen We Trust
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.