bogdanov-d-a / oop_lab2

OOP lab 2
0 stars 0 forks source link

Замечания по ParseUrl #5

Open alexey-malov opened 9 years ago

alexey-malov commented 9 years ago
namespace ParseURL
{
    enum class Protocol
    {
        HTTP,
        HTTPS,
        FTP
    };

    bool ParseURL(std::string const& url, Protocol &protocol,
        int &port, std::string &host, std::string &document);
    bool SplitURLProtocol(std::string const& url,
        Protocol &protocol, std::string &remains);
    bool StringToProtocol(std::string protocolStr, Protocol &protocol);
    bool ParseURLRemains(std::string const& remains, Protocol protocol,
        int &port, std::string &host, std::string &document);
    int GetDefaultPort(Protocol protocol);
    bool StringToPort(std::string const& portStr, int &port);
    size_t Min(size_t a, size_t b);
}

Если вставить пустые строки между функциями, то это улучшит читаемость кода и ни капли не повлияет на производительность приложения.

В заголовочном файле следует оставлять только те функции, которые требуются извне. Все остальное можно спрятать в .cpp файле

alexey-malov commented 9 years ago
    size_t Min(size_t a, size_t b);

чем не удовлетворил std::min?

alexey-malov commented 9 years ago
bool ParseURL::StringToProtocol(std::string protocolStr, Protocol &protocol)
{
    std::transform(protocolStr.begin(), protocolStr.end(), protocolStr.begin(), tolower);

    if (protocolStr == "http")
    {
        protocol = Protocol::HTTP;
        return true;
    }

    if (protocolStr == "https")
    {
        protocol = Protocol::HTTPS;
        return true;
    }

    if (protocolStr == "ftp")
    {
        protocol = Protocol::FTP;
        return true;
    }

    return false;
}

можно объявить статичекий map или вообще массив пар, в котором задать соответствие между строковым представлением протокола и его перечислимым типом. Искать можно при помощи map.find либо по вектору при помощи find_if. Получится не длинее, а вот добавить новый протокол без увеличения объема кода будет проще

alexey-malov commented 9 years ago
    document = remains.substr(slashIndex + 1, std::string::npos);

второй аргумент метода substr опционален и равен по умолчанию npos

alexey-malov commented 9 years ago

Чтобы внутри файла реализации не писать каждый раз std:: можно написать в после #include-ов внутри .cpp файла using namespace std;

alexey-malov commented 9 years ago

Следующее замечание не обязательно для реализации, но рекомендуемо для изучения: Есть в стандартной библиотеке файл regex, в котором можно регулярные выражения использовать для разбора урла по частям. Может получиться короче. Но нужно потратить время на изучение

alexey-malov commented 9 years ago

В интерфейсе взаимодействия с пользователем при вводе невалидного урла желательно сообщать об этом пользователю

alexey-malov commented 9 years ago

k=0,75 за исполнение 1,0 за срок