Как стать автором
Обновить

Комментарии 26

Ну ё-моё, select ведь прост как не знаю что. Почему нельзя сразу рассказывать правильно, а не эту ересь со sleep?
select не подходит для очень большого количества соединений…
Я select использую в винде, а в Linux epoll. В кроссплатформенном коде надо это учесть. В данной статье я счел нецелесообразным на это отвлекаться.
Тогда уж в Windows следует использовать IOCP (Input/output completion port).
select действительно плохо масштабируется, однако общая идея работы с ним мало отличается от оной в epoll/kqueue. Про IOCP не скажу.
Количество обрабатываемых сокетов в виндовом select зависит от размера FD_SET, этот размер меняется специальным дефайном. Подробности внизу этой страницы msdn.microsoft.com/en-us/library/windows/desktop/ms740141(v=vs.85).aspx
В винде начиная с висты есть WSAPoll, вроде как аналог линуксового pool.
Скажите пожалуйста, а почему класс сервера написан так, что в конструкторе находится бесконечный цикл, и внутри вызываются методы этого же класса, когда он фактически является «недоинициализированным»?

Может быть стоило добавить какой-нибудь метод, что-то вроде Start?
Думаете, после заявления: «пишу и буду писать код в заголовочных файлах если мне это удобно», стоит читать код? :-)
Ну, я подумал, вдруг у человека на то есть необоснованные причины, равно как и смешивать два разных класса в одном заголовке.

А насчёт конструктора, может он руководствовался чем-нибудь…
Сто раз так делал и ни разу не возникало проблем с «недоинициированностью». А если нет разницы, то зачем платить больше? (ц)
Чисто для ржаки отнаследуйся и вызови виртуальную функцию наследника. Разница есть…
Разница есть, и она значительна. То что вы лично до сих пор ее не обнаружили ничего не доказывает.
Как вам уже сказали будут проблемы с вызовами виртуальных функций. То что сегодня вам они не нужны не значит что не понадобятся завтра. Но что еще хуже — раз уж вы выложили свой код как учебный, то будьте так добры напишите код так чтобы он приносил больше добра чем зла.
А ваш проект, извините конечно, можно использовать как пособие по антипаттернам создания проектов.

И кстати вы пробовали собрать ваш кроссплатформенный проект под другими платформами? Я вот попробовал. Ошибок было две:
стока 81: добавить const к указателю.
строка 401: тут неправильный тип передается в функцию accept(). Дело в том что я собирал ваш код на 64 битной версии линукса, и здесь size_t имеют разную размерность socklen_t

Впрочем это были мелочи. Попробую перечислить более серьезные недостатки:

1. Конструктор Сервера это простите жопа. Да в си++ можно и не так писать, можно вообще значения из функций возвращать через throw-catch ( и тоже вам никто не запретит и может даже не докажет что так нельзя ). Но программа должна быть прозрачна и предсказуема. Если вы в коробке из-под перца с надписью чай лежит сахар — значит что-то у вас не так в консерватории.
Конструктор создает объект. Так уж повелось. Другие программисты не всегда смогут досконально изучить вашу программу — в идеале человеку нужно взглянуть на заголовки хорошо спроектированных классов чтобы представить себе как ими пользоваться. Вот поэтому нужны общие для всех понятия. Компилятору то на них наплевать.

Кстати в Кт3 была такая неприятная особенность у класса сокет — при создании объекта этого класса — сам сокет не создавался. А создавался он лишь при вызове функций сокет.акцепт или сокет.коннект.
Вроде бы логично ( эдакая оптимизация от разработчиков библиотеки ), но эта долбаная оптимизация забрала кучу нашего времени на попытки понять, почему дескриптр созданного объекта всегда невалидный. Это банальный пример неожиданного поведения чужой библиотеки. Так вот — поведение должно быть ожидаемым и логичным.

Идем дальше.
memcpy(&vTemp[0], &m_vSendBuffer[err], m_vSendBuffer.size()-err);
Так делать нельзя — memcpy не умеет копировать данные из пересекающихся участков памяти, тут нужен memmove ( а еще лучше уйти от сишных фукнций на std алгоритмы вроде copy )

Это из того на что упал взгляд. Вообще же мешанина стилей и подходов. Нет четкого разграничения уровней http, ssl и собственно сервера.

И попробую ответить на вопрос чем библиотеки лучше вашего собственного кода. Библиотеки хороши тем что решают огромное количество проблем о существовании которых вы не подозреваете. Кроме того они уже протестированы и уже работают.
Написание же своего кода — хорошо тем что вы лучше понимаете как и что работает ( что прекрасно подходит для обучения ). А также тем, что вы можете создать код для оптимального решения именно своей задачи. Впрочем это удел очень хороших специалистов.

зы. После статьи и комментариев остались смешанные чувства. С одной стороны — мне бы не хотелось чтобы кто-то начинал учить сетевое программирование ( или упаси Страуструп ) Си++ по этой статье. Слишком много тут дыр и ошибок.
С другой — мне не нравится когда старожилы хабра уводят в минуса людей с желанием писать программы и статьи. Пусть даже уровень этих статей далеко не дотягивает до уровня желаемого. Предлагаю автору попробовать переосмыслить свой подход, и хм надеюсь что он сумеет допустить что то что ему тут советуют имеет смысл.
А мой взгляд вот за это зацепился:

while (it != m_mapClients.end()) //Перечисляем всех клиентов
{
    if (!it->second->Continue()) //Делаем что-нибудь с клиентом
        m_mapClients.erase(it++); //Если клиент вернул false, то удаляем клиента
    else
        it++;
}


Должно быть вот так:

it = m_mapClients.erase(it);


С другой — мне не нравится когда старожилы хабра уводят в минуса людей с желанием писать программы и статьи. Пусть даже уровень этих статей далеко не дотягивает до уровня желаемого.


Я не минусовал но подозреваю, что причина минусов в том, что автор категорически не хочет прислушиваться к мнению более опытных разработчиков (особенно это заметно по комментариям к первой статье). А с таким подходом далеко не продвинуться.
По-моему этот код как раз у автора написан правильно. Именно так и делается удаление элементов из мэпа, во время движения по нему, чтобы не сломать итератор. Ваш вариант по-моему не будет работать, во-первых map.erase возвращает void или size_t ( если передать ключ а не итератор ). Может у вас какая-то другая реализация мэпа, которая возвращает следующий за удаленным итератор?

>подозреваю, что причина минусов в том, что автор категорически не хочет прислушиваться к мнению более опытных разработчиков
Полностью с вами согласен. Автор сжав зубы отбивается от гадких людишек, посягнувших на его код и право писать так как он хочет. Жаль что он не допускает что ему тоже пытаются помочь. С другой стороны это и правда сложно — признать себя неправым и попытаться разобраться. Впрочем я в него верю =)
По-моему этот код как раз у автора написан правильно.


Вы правы, что-то я уже забываю C++03 и полагаюсь на C++11 :-)
Во-первых, разрешите Вас поблагодарить за столь подробный и в общем доброжелательный комментарий.
Теперь, несмотря на то, что мои ответы всегда притягивают только минусы, я все таки попробую прокомментировать ваш комментарий )))

1. Конструктор сервера
Вообще, у меня появилась мысль, что про это можно написать отдельную статью, но пока попробую тут описать идею.Вы пишите «конструктор создает объект». По моему, правильнее так:: «конструктор инициирует объект».
В чем отличие? В том, что инициация это задание начального состояния или, другими словами, задание начальных значений для внутренних переменных объекта. Согласны? Если нет, то поправьте меня, но я вижу роль конструкторов именно так.
Если же Вы со мной тут согласны, то дальше все логично: мой класс CServer имеет лишь одну внутреннюю переменную и весь смысл его существования — инициирование этой переменной.
Что качается вызовов виртуальных функций: опять же, следуя своему пониманию роли конструктора, у меня даже в мыслях не возникнет вызвать из конструктора не только виртуальную, но и любую другую не приватную функцию.

2. Пример с классом сокета
Честно говоря не понял — что вы им хотели сказать, на мой взгляд он лишь подтверждает правильность моего подхода: после конструктора все переменные класса должны иметь предсказуемое, валидное и желательно константное значение. В противном случае поведение класса может вызвать недоумение у сторонних разработчиков.

3. memcpy(&vTemp[0], &m_vSendBuffer[err], m_vSendBuffer.size()-err);
Я правда наверное очень плохо знаю с++ в целом и stl в частности, но убейте — не вижу здесь пересекающихся областей памяти (((
vTemp — временный локальный буфер, выделенный в функции, а m_vSendBuffer — переменная класса, память для которой выделяется в конструкторе. Как они могут пересекаться???

4. "… библиотеки лучше вашего собственного кода."
Полностью согласен с тем, что Вы пишите. Кроме пункта:
Впрочем это удел очень хороших специалистов.

Я лично убежден, что прежде чем использовать библиотеки, надо сначала научиться программировать без них. Если меня не забанят за рекордное количество минусов, то после статей про сервер на голых сокетах я планирую написать такую же серию про сервер на boost::asio. Но именно в таком порядке, а не наоборот. Уж простите…
1. Про конструктор. Попробую представить это иначе. Используя ООП мы подразумеваем что мы оперируем объектами. Объекты имеют время жизни, состояние, набор событий которые с ними могут происходить. Так вначале они создаются, затем получают различные сигналы ( вызовы функций) меняют свое состояние и в конце уничтожаются. Объект — модель некой другой ментальной модели в голове разработчика. В случае сервера у объекта может быть функция выключающая его, которая корректно завершит все соединения. Может быть функция вызываются переинциалазацию каких-то данных, или функция приостанавливающая прием новых соединений.
В вашем случае не было никакого смыла городить целый класс ради того чтобы написать в нем код лишь конструктора — обошлись бы простой функций — никто бы вам слова не сказал. Просто использование классов подразумевает некую общепринятую модель, который вы не последовали.

2. Пример с классом сокета
Если бы я был пользоватем библиотеки в который реализован такой подход к серверу — я бы долго не мог понять как использовать ваш сервер. Скорее всего я бы решил что здесь какая-то ошибка, и что этот код не тут. Поэтому я говорю про схожую неожиданность поведения кода.

3. memcpy
Да тут я кажется ошибся. Видимо показлось что там копирование в ту же самую переменную. Кстати рекомендую вам посмотреть на std::copy. Если честно ваше использование memcpy вкупе с std::vector вызвало у меня легкий разрыв шаблона. Хоть я и узнал что это вполне допустимая операция с ним.

4. "… библиотеки лучше вашего собственного кода."
полностью согласен — учится лучше на своем коде. Может вы просто не написали что этот проект для вас учебный, я счел что вы хотите им научить чему-то других, менее опытных программистов.
Небольшие замечания по коду, не более того…

Первой строкой в server.h было бы здорово написать
#pragma once

Коллеги по цеху Вас не поймут за такое:
using namespace std;

И в конце хотелось бы отметить, что заголовочные файлы C++ как-то принято именовать *.hh или *.hpp для обозначения намерений, а *.h — слишком общий вариант (это уже imho).
В статье, со словами «кроссплатформенный» и «неблокирующие сокеты» в заголовке, должно быть как минимум упоминание о libev/libevent, а вообще странно что вы их не захотели использовать в реализации.
Зачем о них упоминать?
Кто-то считает, что без libevent неблокирующих сокетов не бывает, кто-то считает что без boost::asio…
Я вот считаю, что с голыми сокетами вполне можно комфортно жить.
Сервер из статьи умещается в 425 строк кода. Сколько строк будет занимать точно такой же сервер как у меня, но реализованный на ваших любимых библиотеках?
Столько же, но при этом он будет действительно кроссплатформенным (с автоматическим выбором лучшего асинхронного механизма для данной платформы, коих побольше, чем просто «линукс» и «виндоус»).
Словари считают, что кроссплатформенность это «работа на нескольких платформах», а не «работа на любой платформе».
Жесть какая… книг почитайте что ли по С++ и вообще по программированию, прежде чем других учить.
Чтобы скомпилировать в Linux надо скопировать в одну директорию файлы: serv.cpp, server.h, ca-cert.pem и в командной строке набрать: «g++ -std=c++0x -L/usr/lib -lssl -lcrypto serv.cpp»
Запомните: если хотите избавиться от дурацких ошибок ещё на этапе компиляции, всегда компилируйте код с параметром -Wall, а ещё лучше с -Wextra иногда прогонять. В MSVS после создания проекта нужно привыкнуть сразу же устанавливать ворнинги с дефолтного /W3 на /W4. Сразу же выбросятся примитивнейшие ошибки сравнения signed и unsigned, которые у вас на 284-й и 335-й строках.

Ещё как можно чаще нужно заглядывать в man\msdn:
sa_serv.sin_addr.s_addr = INADDR_ANY;
И можно узнать, что адрес в эту структуру передаётся так же как и порт — в сетевом порядке байт. Вам просто тут повезло с константой INADDR_ANY равной нулю. Ну ладно, спихнём на невнимательность.

Также, у вас тут не С++, а солянка какая-то получается из С99, С++03 и С++11. Не тащите за собой libc, раз уж используете libstdc++. Используйте что-то одно, все эти Си'шные printf и memcpy вполне имеют замену в С++.

И уж, если хотите писать сетевые приложения, почитайте Стивенса что ли. Научитесь использовать всякие SO_REUSEADDR, без которого ваш сервер невозможно будет перезапустить по скрипту.

А писать код в хидерах это жесть =). Как вы планируете распространять библиотеки с таким «оригинальным» способом написания кода? Потратьте недельку на изучение стиля и приёмов из библиотеки Qt. Хидеры, при хорошем стиле программирования, должны заменять документацию и не содержать ничего лишнего. Достаточно заглянуть в него, и всё становится понятно о библиотеке.
Согласен со всеми Вашими замечаниями, кроме
А писать код в хидерах это жесть

Насколько я в курсе, библиотека boost распространяется именно в хедерах? Поправьте, если ошибаюсь.
За это буст большинство и не любит. Адское мета-программирование на шаблонах, постоянные перекомпиляции всего кода целиком, если нет возможности делать precompiled-header. У них свой стиль просто, выработанный годами, и многими за это ненавидится =).

К тому же, это опенсорс. Стоит делать что-то проприетарное и коммерческое, тут уже header-only не прокатит, либо придётся снова извращаться сидеть, вместо того, чтобы просто писать код.

На том же бусте что-то написать, даже в юниксах приходится тащить за собой все исходники. Вместо того, чтобы установить одиночную .so-шку в систему, и к ней 1-2 хидера на 50кб. А тут же, хидеров на 150 метров и рантайма на 20. Ну да ладно, это моя личная неприязнь к данной библиотеке =).

На вкус и цвет, короче. Писать то можно и на брейнфаке, и юникод использовать в именах переменных, и ещё много чего, что не является критически неправильным, но вот кто после вас будет поддерживать такой код?
Код и темплейты это не одно и то же. Шаблоны пишутся в хидерах, а код — .cpp. И буст не полностью в хедерах, у него есть компилируемые части (хоть и не всех библиотеках).
Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории