Sobes.tech
Назад к вопросам
Middle
7

public bool BookTour(string userId, DateTime start, DateTime end, int hotelId, string roomType, string flightClass, int airCompanyId, int flightnumber) { try { //flightClass тоже не хотелось бы парсить enum var flightInfo = new FlightInfo(Enum.Parse<FlightClass>(flightClass), airCompanyId, flightnumber); var resultBookFlight = _airWaysService.Book(GetUser(userId), flightInfo); //roomType тоже не хотелось бы парсить enum var rentInfo = new RentInfo(start, end, hotelId, Enum.Parse<RoomType>(roomType)); //GetUserId в отдельная переменная var resultBookHotel = _hotelService.Book(GetUser(userId), rentInfo); //Exception уйдут в inner, не отловит Task.WhenAll(resultBookFlight, resultBookHotel); //Почему статикa reservationIdAir = resultBookFlight.Result; _reservationIdHotel = resultBookHotel.Result; } catch (BookFlightException) { _hotelService.CancelBook(_reservationIdAir); return false; } catch (BookHotelException) { _airWaysService.CancelBook(_reservationIdHotel); return false; } catch (Exception ex) { throw new ApplicationException(ex.Message); } finally { //поля 100% не инициализованы //Почему после exception мы пытаемся что-то сохранить // GetUser уже вынесли unitOfWork.Execute(async () => { await _tourRepository.Save(_reservationIdAir, _reservationIdHotel, GetUser(userId)); }).RunSynchronously(); //Interlock.Increment() _countBooked++; } return true; }

Компании, где спрашивали
OZONOZON

Ответ от нейросети

sobes.tech AI

В представленном коде есть несколько проблем и возможностей для улучшения:

  1. Парсинг enum из строкEnum.Parse<FlightClass>(flightClass) и Enum.Parse<RoomType>(roomType) могут выбросить исключение, если строка невалидна. Лучше использовать Enum.TryParse с обработкой ошибки или предусмотреть более безопасный способ передачи этих данных (например, напрямую enum или отдельный валидатор).

  2. Дублирование вызова GetUser(userId) — лучше вызвать один раз и сохранить результат в переменную.

  3. Асинхронность и Task.WhenAll — вызов Task.WhenAll(resultBookFlight, resultBookHotel); не дожидается завершения, так как не используется await. Это значит, что ошибки могут быть не отловлены, а результат — не получен вовремя.

  4. Обработка исключений и отмена бронирования — в catch-блоках вызывается отмена брони, но параметры для отмены перепутаны (_hotelService.CancelBook(_reservationIdAir); и наоборот). Это ошибка.

  5. Использование статических полей _reservationIdAir и _reservationIdHotel — если метод вызывается параллельно, это приведёт к гонкам данных. Лучше использовать локальные переменные.

  6. В блоке finally вызывается асинхронный код через RunSynchronously() — это плохая практика, может привести к дедлоку. Лучше сделать метод асинхронным и использовать await.

  7. Сохранение данных и инкремент счетчика бронирований происходит даже при исключениях — логично делать это только при успешном бронировании.

Пример исправленного варианта (упрощённо):

public async Task<bool> BookTourAsync(string userId, DateTime start, DateTime end, int hotelId, string roomType, string flightClass, int airCompanyId, int flightNumber)
{
    var user = GetUser(userId);

    if (!Enum.TryParse<FlightClass>(flightClass, out var parsedFlightClass))
        return false; // или выбросить исключение
    if (!Enum.TryParse<RoomType>(roomType, out var parsedRoomType))
        return false;

    var flightInfo = new FlightInfo(parsedFlightClass, airCompanyId, flightNumber);
    var rentInfo = new RentInfo(start, end, hotelId, parsedRoomType);

    string reservationIdAir = null;
    string reservationIdHotel = null;

    try
    {
        var bookFlightTask = _airWaysService.Book(user, flightInfo);
        var bookHotelTask = _hotelService.Book(user, rentInfo);

        await Task.WhenAll(bookFlightTask, bookHotelTask);

        reservationIdAir = bookFlightTask.Result;
        reservationIdHotel = bookHotelTask.Result;

        await unitOfWork.Execute(async () =>
        {
            await _tourRepository.Save(reservationIdAir, reservationIdHotel, user);
        });

        Interlocked.Increment(ref _countBooked);

        return true;
    }
    catch (BookFlightException)
    {
        if (reservationIdHotel != null)
            await _hotelService.CancelBook(reservationIdHotel);
        return false;
    }
    catch (BookHotelException)
    {
        if (reservationIdAir != null)
            await _airWaysService.CancelBook(reservationIdAir);
        return false;
    }
    catch (Exception ex)
    {
        throw new ApplicationException(ex.Message, ex);
    }
}

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