Замечания по программе Radix

[alexey-malov]

    for_each(value.begin(), value.end(), [=](char sign)
    {
        if (sign != '-')
        {
            if (!(sign >= 'A' && sign < 'Z'))
            {
                if (!(sign >= '0' && sign <= '9'))
                {
                    throw invalid_argument("Invalid value. Value can only include chars [0, 9], [A, Z)");
                }
            }
            if (sign >= 'A')
            {
                if (sign - AsciiNumDifference >= sourceNotation)
                {
                    throw invalid_argument("Each symbol of value must be lower than source notation. Example: 1F in 16 : 1 < 16 and F < 16");
                }
            }
            else if (sign <= '9')
            {
                if (sign - '0' >= sourceNotation)
                {
                    throw invalid_argument("Each symbol of value must be lower than source notation. Example: 10 in 2 : 1 < 2 and 0 < 2");
                }
            }
        }
    });

1. В алгоритме for_each необходимости практически не возникает, т.к. range-based for проще
2. Здесь более уместен алгоритм all_of (все символы являются допустимыми), либо none_of (ни один из символов не является недопустимым)
3. Если символ '-' считается допустимым, то почему '+' не считается?
4. Почему-то забыты строчные символы a-z
5. Что за константа AsciiNumDifference, как она появилась? Лучше выразить ее значения через символы 'A', 'Z', '0', '9', 'a', 'z'
6. 36-ричная система счисления требует 10 цифр и 26 букв. У тебя же символ Z считается недопустимым

[alexey-malov]

long long ToDecimalNotation(const std::string & value, int sourceNotation)
{
    ValidationOfValue(value, sourceNotation);
    if (sourceNotation == 10)
    {
        return stoi(value);
    }
    std::vector<char> digits(value.begin(), value.end());
    long long result = 0;
    bool isNegative = false;
    for (size_t i = 0; i < digits.size(); i++)
    {
        if (digits[i] == '-')
        {
            isNegative = true;
        }
        else if (digits[i] >= 'A')
        {
            result += (digits[i] - AsciiNumDifference) * (long long)pow(sourceNotation, digits.size() - 1 - i);
        }
        else
        {
            result += (digits[i] - '0') * (long long)pow(sourceNotation, digits.size() - 1 - i);
        }
    }
    if (isNegative)
    {
        result *= -1;
    }
    return result;
}

от использования pow ты так и не избавился.

[alexey-malov]

в тестах не проверяется работа функций на недопустимых данных (выход основания системы счисления за пределы 2...36, переполнение из-за невозможности вместить в тип int переданное число используй BOOST_CHECK_THROW

[alexey-malov]

std::string Transfer(int sourceNotation, int destNotaion, const std::string & value)
{
    if (value == "0")
    {
        return "0";
    }
    try
    {
        auto decimalVal = ToDecimalNotation(value, sourceNotation);
        string result = FromDecimalToDestination(decimalVal, destNotaion);
        return result;
    }
    catch(invalid_argument err)
    {
        return "Invalid value. Value must be a number";
    }
}

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

[alexey-malov]

    std::reverse(result.begin(), result.end());
    return result;
}

можно просто написать: return {result.rbegin(), result.rend()};

[alexey-malov]

k=0,6

[Dzzirt]

Исправил

[alexey-malov]

Сборка должна проходить без предупреждений

1>d:\teaching\2016\kuzin\oop\lab1\radix\radix\main.cpp(22): warning C4101: 'err' : unreferenced local variable

например, исправить можно так:

    catch (invalid_argument const& /*err*/)
    {
        cout << "Notation must be in range [2, 36)" << endl;
    }

или так:

    catch (invalid_argument const& err)
    {
        std::ignore = err;
        cout << "Notation must be in range [2, 36)" << endl;
    }

или просто использовать err (например, его метод what())

[alexey-malov]

нет поддержки 36-чной системы счисления, использующей символы 0..9,A...Z включительно

[alexey-malov]

            if (i == 6)
            {
                cout << "kek";
            }

wtf?

[alexey-malov]

            if (isNegative)
            {
                MultiplicationWithOverflowCheck(sourceNotation, result * -1);
                AddWithOverflowCheck(CharToNumber(value[i]), result * sourceNotation * -1);
            }
            else
            {
                MultiplicationWithOverflowCheck(sourceNotation, result);
                AddWithOverflowCheck(CharToNumber(value[i]), result * sourceNotation);
            }

            result = result * sourceNotation + CharToNumber(value[i]);

Для устранения дублирования кода и соответствия своему имени функциям MultiplicationWithOverflowCheck и AddWithOverflowCheck следует выполнять вычисления, а не просто проверять

[alexey-malov]

Попытка преобразовать число INT_MIN, равное -2147483648, в шестнадцатиричную систему счисления, программа выводит странное:

-8463847412-

[alexey-malov]

k=0,75

[Dzzirt]

Исправил