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

[AniSkyWorker]

• [ ] чтобы избежать варнинга при приведении char к int внутри transform, передавать в transform свою обертку(как ее написать смотри доку http://en.cppreference.com/w/cpp/string/byte/tolower)
• [ ] в тестах стоит завести функцию инкапсулирующую процесс проверки урла, чтобы избежать дублирования объявления переменных, передаваемых в ParseUrl, и проверок.

k = 0.8

[alexey-malov]

• [x] Избавиться от постороннего вывода при работе тестов

[alexey-malov]

    catch (const std::invalid_argument& isError)
    {
        std::cout << isError.what() << "\n";
        return false;
    }
    catch (const std::out_of_range& isError)
    {
        std::cout << isError.what() << "\n";
        return false;
    }

• [x] Чтобы не дублировать код обработки разных исключений, можно перехватывать исключения общего родителя
• [x] имя isError больше подходит для bool, но не для exception

[alexey-malov]

• [ ] Пара веток не покрыта тестами

[alexey-malov]

Protocol CheckProtocol(const std::string& protocolString)
{
    auto ConvertCharTolower = [](char ch) {
        return static_cast<char>(tolower(ch));
    };

    std::string protocolStringInLowerCase;
    std::transform(protocolString.begin(), protocolString.end(), std::back_inserter(protocolStringInLowerCase), ConvertCharTolower);
    if (protocolStringInLowerCase == "http")
    {
        return Protocol::HTTP;
    }
    else if (protocolStringInLowerCase == "https")
    {
        return Protocol::HTTPS;
    }
    else if (protocolStringInLowerCase == "ftp")
    {
        return Protocol::FTP;
    }

    return Protocol::NotValid;
}

• [ ] Переименовать функцию

[alexey-malov]

bool CheckPortNumber(const std::string& portString, int& portNumber)
{
    try
    {
        portNumber = std::stoi(portString);
        if ((portNumber < 1) || (portNumber > 65535))
        {
            return false;
        }
    }
    catch (const std::exception&)
    {
        return false;
    }

    return true;
}

• [ ] Переименовать функцию

[alexey-malov]

    // берет неверный url и возвращает false
    BOOST_AUTO_TEST_CASE(takes_invalid_url_and_returns_false)
    {
        CheckUrlParsing("http:/www.mysite.com/docs", 0, "", "", false);
        CheckUrlParsing("https//www.mysite.com/docs", 0, "", "", false);
        CheckUrlParsing("ftpwww.mysite.com/docs", 0, "", "", false);
    }

• [ ] Из имени теста не ясен критерий, по которому урл считается невалидным. Разбить на несколько тестов с понятными именами.

• [ ] Для проверки невалидных урлов сделать отдельную функцию, ожидающую, что парсинг URL-а завалится.

ExpectUrlParsingFailure("InvalidProtocol://google.com");