Re[3]: Просьба от новичка.
От: _Dreamer Россия  
Дата: 30.01.08 02:12
Оценка:
Здравствуйте, DiKeert, Вы писали:

DK>Выкладываю.


вопрос первый.
почему везде бросается std::string ?
мало того, что этого будет мало кто ожидать при использовании Вашего кода, так еще и собственно информации для приложения никакой нет.
тоесть, я бы предложил — создать наследника std::exception, возможно с некоторым контекстом исключения, хранящем информацию о произошедшем.

далее, зачем постоянно ловить по значению ?
в случае std::string это запросто может привести к аллокации памяти, которая в свою очередь, может вызвать тот же std::bad_alloc, который Вы сами же ловите и сразу же создаете еще один объект std::string в одном из методов.

std::string Socket::getIp()
{
    char ipaddr[INET_ADDRSTRLEN];
    const char *ipAddr = inet_ntop(AF_INET, &(factAddr->sin_addr), ipaddr, sizeof(ipaddr));

    std::string *result = new std::string(ipAddr); /* можно спросить, зачем так ? да еще и нет проверки на (NULL == ipAddr) */
    ip = *result;
    return ip; /* в этом месте, как Вы понимаете, никто за Вас result освобождать не будет, поэтому тут утечка памяти */    
}


к тому же, функция не совсем логична. getIp() есть функция от factAddr, более она ни отчего не зависит. поэтому логичнее было бы формировать строку ip в момент изменения factAddr, это у Вас один только метод bind(). далее просто возвращать сформированную строку.
const std::string& Socket::getIp() const { return this->ip; }


почему некоторые методы объявлены как 'int methodName()', а в теле идет 'return true /* false */ ;' ?
если кодов возврата всего 2, так и объявить соответственно, если же больше, то выделить их скажем в набор констант.

многие параметры, вроде INADDR_ANY, AF_INET, а так же волшебные цифры 8008 и 16 забиты прямо в коде.
это как минимум неудобно.

зачем память для 'sockaddr_in *factAddr;' выделять динамически ?
храните структуру как 'sockaddr_in factAddr;', не нужно будет следить за выделением\освобождением памяти.

не очень красиво, что класс завязан на std::cout.
перегрузить operator << (std::ostream&, const Socket&), или как то еще.
но метод void Socket::showIp() не нужен.

что бегло заметил, то спросил.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.