3

Имеется следующий метод:

    public async Task<int> PaymentOrderAsync(int orderId, int userId)
    {
        await using(var db = new TelegramShopDBContext())
        {
            try
            {
                var user = await db.Users.FirstOrDefaultAsync(u => u.UserId == userId) ?? throw new ArgumentNullException("⚠️ Пользователь не найден");
                var order = await db.Orders.FirstOrDefaultAsync(o => o.Id == orderId) ?? throw new ArgumentNullException("⚠️ Заказ не был найден");
                var product = await db.Products.FirstOrDefaultAsync(p => p.NameProduct == order.NameProduct && p.Value >= order.Value) ?? throw new ArgumentNullException("🤷‍♀️ Извините, но товар закончился");
                if (user.Balance >= order.TotalPrice)
                {
                    user.Balance -= order.TotalPrice;
                    order.Status = "Ожидает выполнения";
                    product.Value -= order.Value;
                    return await db.SaveChangesAsync();
                }
                else
                {
                    return 
                }
            }
            catch(DbException)
            {
                return 0;
            }
        }
    }

У меня возник вопрос. Насколько корректно будет создавать исключение если у Юзера будет недостаточно средств на балансе и каким оно должно быть?(Я имею ввиду блок else)

10
  • если ты будешь писать логику в этом методе, то исключение вызывать не имеет смысла
    – Lofectr
    3 мая 2021 в 17:47
  • @Lofectr, тут как раз логика и заканчивается, что мне что-то нужно вернуть из метода) Я вот и подумал создавать исключение
    – pled
    3 мая 2021 в 17:49
  • order.Status = "Недостаточно средств"; return 0; зачем вам исключения здесь?
    – aepot
    3 мая 2021 в 17:57
  • 2
    Гм, по-моему, все эти запросы следует как минимум обернуть в транзакцию. Потому что в нагруженной системе цены и количество товаров могут измениться в промежутке между отдельными запросами. 3 мая 2021 в 18:02
  • 1
    Кстати, ваш try-catch не ловит ошибки создания самого контекста. То есть new TelegramShopDBContext() может выкинуть исключение. 3 мая 2021 в 18:42

2 ответа 2

7

Исключения надо создавать в исключительных ситуациях, например есть такой метод

int Divide(int a, int b)
{
    return a / b;
}

Почему оператор деления создает исключение приделении на ноль? Потому что он не может поместить результат такого деления в переменную типа int. То есть возврат из метода невозможен при делении на ноль, вот тогда и выбрасывается исключение.

А что делать, если мы знаем, что ноль в делителе - нормальное дело, и мы хотим например логически обработать эту исключительную ситуацию, а точнее - не допустить ее возникновения, что вы собственно и делаете в вашем коде - проверяете, хватает ли денег на балансе.

bool TryDivide(int a, int b, out int result)
{
    bool success = b != 0,
    result = success ? a / b : 0;
    return success;
}

Когда логика приложения предупреждает возникновение исключения, выброс его не требуется.

Обратите внимание: защитная логика, это не когда вы поймали исключение внутри метода, а когда вы предупредили его, то есть не допустили его появления. Обёртываение в try-catch не является защитной логикой, а является обработкой уже возникшего исключения.

// ПЛОХОЙ КОД, НЕ ДЕЛАЙТЕ ТАК
bool TryDivide(int a, int b, out int result)
{
    try
    {
        result = a / b;
        return true;
    {
    catch (DivisionByZeroException)
    {
        result = 0;
        return false;
    }
}

Вот если бы ваша база данных следила за целостностью данных, то есть запрещала бы отрицательный баланс при списаниях, а вы бы невзирая на баланс пытались бы выполнить транзакцию, вот тогда бы исключение имело бы место быть.

Есть защитная логика - нет исключения. Нет защитной логики - есть исключение. Но решать в конечном итоге, как оно будет работать только вам.

3
  • Можно я немного не соглашусь? С точки зрения производительности - блок try дешевле блока if в программах с большими таблицами переходов. Хоть пример, с try-catch плохой, но в некоторых ситуациях он может быть лучше.
    – Blackmeser
    8 ноя 2021 в 3:12
  • @Blackmeser блок try дешевле блока if заинтриговали. Let-it-crash?
    – aepot
    8 ноя 2021 в 5:57
  • к сожалению, когда crash - тогда уже не дешевле =( PS отработка блока try без ошибок не сбрасывает конвеер, в отличии от неправильно предсказанного ветвления if, это как минимум. С таблицами - куда интереснее, как-то натыкался на статью с падением производительности при забивании таблицы переходов.
    – Blackmeser
    9 ноя 2021 в 10:22
4

Чем больше я смотрел на этот код, тем больше у меня возникало к нему вопросов. Часть из них я высказал в комментариях, но чувствую, что нужно их собрать воедино и описать подробно.

В одних случаях вы бросаете исключения для сигнализации о некоторых ошибках:

throw new ArgumentNullException("⚠️ Пользователь не найден");
throw new ArgumentNullException("⚠️ Заказ не был найден");
throw new ArgumentNullException("🤷‍♀️ Извините, но товар закончился");

При этом сообщения сразу локализованы на русский (как вы будете переводить приложение на другой язык?) и содержат смайлики (тут нецензурные выражения) - то есть они явно предназначены для вывода конечному пользователю.

В другом случае вы возвращаете код ошибки:

catch(DbException)
{
    return 0;
}

Также, в случае успеха возвращаете количество затронутых строк:

return await db.SaveChangesAsync();

Выше по коду вы, вероятно, проверяете возвращённое значение и в зависимости от него предпринимаете те или иные действия. Подозреваю, что не проверяете...

Даже если не учитывать, что SaveChanges() тоже может вернуть 0, получается сложная система обработки результата.

Кстати, ваш try-catch не ловит ошибки создания самого контекста. То есть new TelegramShopDBContext() может выкинуть исключение. Но это не самая большая проблема, т. к. легко решается.


Резюмируя, как бы можно было решить всё это дело? Не знаю. Потому что неизвестно, как и что у вас сделано выше по коду. Есть ли централизованная обработка ошибок (middleware exception handling), используются ли кастомные исключения и многие прочие факторы.

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge you have read our privacy policy.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.