Skip to content

Commit

Permalink
Using Result instead of tuple where applicable (#408)
Browse files Browse the repository at this point in the history
* Using Result instead of tuple where applicable

* fixed code smell

* removed result as return for orderrequestservice
  • Loading branch information
acn-sbuad authored Feb 5, 2024
1 parent f11016b commit 1c61702
Show file tree
Hide file tree
Showing 18 changed files with 235 additions and 170 deletions.
18 changes: 9 additions & 9 deletions src/Altinn.Notifications.Core/Services/GetOrderService.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using Altinn.Notifications.Core.Enums;
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Persistence;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;

namespace Altinn.Notifications.Core.Services;

Expand All @@ -28,38 +28,38 @@ public GetOrderService(IOrderRepository repo)
}

/// <inheritdoc/>
public async Task<(NotificationOrder? Order, ServiceError? Error)> GetOrderById(Guid id, string creator)
public async Task<Result<NotificationOrder, ServiceError>> GetOrderById(Guid id, string creator)
{
NotificationOrder? order = await _repo.GetOrderById(id, creator);

if (order == null)
{
return (null, new ServiceError(404));
return new ServiceError(404);
}

return (order, null);
return order;
}

/// <inheritdoc/>
public async Task<(List<NotificationOrder>? Orders, ServiceError? Error)> GetOrdersBySendersReference(string senderRef, string creator)
public async Task<Result<List<NotificationOrder>, ServiceError>> GetOrdersBySendersReference(string senderRef, string creator)
{
List<NotificationOrder> orders = await _repo.GetOrdersBySendersReference(senderRef, creator);

return (orders, null);
return orders;
}

/// <inheritdoc/>
public async Task<(NotificationOrderWithStatus? Order, ServiceError? Error)> GetOrderWithStatuById(Guid id, string creator)
public async Task<Result<NotificationOrderWithStatus, ServiceError>> GetOrderWithStatuById(Guid id, string creator)
{
NotificationOrderWithStatus? order = await _repo.GetOrderWithStatusById(id, creator);

if (order == null)
{
return (null, new ServiceError(404));
return new ServiceError(404);
}

order.ProcessingStatus.StatusDescription = GetStatusDescription(order.ProcessingStatus.Status);
return (order, null);
return order;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Shared;

namespace Altinn.Notifications.Core.Services.Interfaces;

Expand All @@ -13,19 +13,19 @@ public interface IGetOrderService
/// </summary>
/// <param name="id">The order id</param>
/// <param name="creator">The creator of the orders</param>
public Task<(NotificationOrder? Order, ServiceError? Error)> GetOrderById(Guid id, string creator);
public Task<Result<NotificationOrder, ServiceError>> GetOrderById(Guid id, string creator);

/// <summary>
/// Retrieves a notification order by senders reference
/// </summary>
/// <param name="senderRef">The senders reference</param>
/// <param name="creator">The creator of the orders</param>
public Task<(List<NotificationOrder>? Orders, ServiceError? Error)> GetOrdersBySendersReference(string senderRef, string creator);
public Task<Result<List<NotificationOrder>, ServiceError>> GetOrdersBySendersReference(string senderRef, string creator);

/// <summary>
/// Retrieves a notification order with process and notification status by id
/// </summary>
/// <param name="id">The order id</param>
/// <param name="creator">The creator of the orders</param>
public Task<(NotificationOrderWithStatus? Order, ServiceError? Error)> GetOrderWithStatuById(Guid id, string creator);
public Task<Result<NotificationOrderWithStatus, ServiceError>> GetOrderWithStatuById(Guid id, string creator);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Notification;
using Altinn.Notifications.Core.Models.Notification;
using Altinn.Notifications.Core.Shared;

namespace Altinn.Notifications.Core.Services.Interfaces
{
Expand All @@ -13,6 +13,6 @@ public interface INotificationSummaryService
/// </summary>
/// <param name="orderId">The order id to find notifications for</param>
/// <param name="creator">The creator of the order</param>
public Task<(EmailNotificationSummary? Summary, ServiceError? Error)> GetEmailSummary(Guid orderId, string creator);
public Task<Result<EmailNotificationSummary, ServiceError>> GetEmailSummary(Guid orderId, string creator);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Shared;

namespace Altinn.Notifications.Core.Services.Interfaces;

Expand All @@ -13,5 +13,5 @@ public interface IOrderRequestService
/// </summary>
/// <param name="orderRequest">The notification order request</param>
/// <returns>The registered notification order</returns>
public Task<(NotificationOrder? Order, ServiceError? Error)> RegisterNotificationOrder(NotificationOrderRequest orderRequest);
public Task<NotificationOrder> RegisterNotificationOrder(NotificationOrderRequest orderRequest);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using Altinn.Notifications.Core.Enums;
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Notification;
using Altinn.Notifications.Core.Persistence;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;

namespace Altinn.Notifications.Core.Services
{
Expand Down Expand Up @@ -40,21 +40,21 @@ public NotificationSummaryService(INotificationSummaryRepository summaryReposito
}

/// <inheritdoc/>
public async Task<(EmailNotificationSummary? Summary, ServiceError? Error)> GetEmailSummary(Guid orderId, string creator)
public async Task<Result<EmailNotificationSummary, ServiceError>> GetEmailSummary(Guid orderId, string creator)
{
EmailNotificationSummary? summary = await _summaryRepository.GetEmailSummary(orderId, creator);

if (summary == null)
{
return (null, new ServiceError(404));
return new ServiceError(404);
}

if (summary.Notifications.Count != 0)
{
ProcessNotificationResults(summary);
}

return (summary, null);
return summary;
}

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/Altinn.Notifications.Core/Services/OrderRequestService.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using Altinn.Notifications.Core.Configuration;
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.NotificationTemplate;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Persistence;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;

using Microsoft.Extensions.Options;

Expand Down Expand Up @@ -33,7 +33,7 @@ public OrderRequestService(IOrderRepository repository, IGuidService guid, IDate
}

/// <inheritdoc/>
public async Task<(NotificationOrder? Order, ServiceError? Error)> RegisterNotificationOrder(NotificationOrderRequest orderRequest)
public async Task<NotificationOrder> RegisterNotificationOrder(NotificationOrderRequest orderRequest)
{
Guid orderId = _guid.NewGuid();
DateTime created = _dateTime.UtcNow();
Expand All @@ -52,7 +52,7 @@ public OrderRequestService(IOrderRepository repository, IGuidService guid, IDate

NotificationOrder savedOrder = await _repository.Create(order);

return (savedOrder, null);
return savedOrder;
}

private List<INotificationTemplate> SetSenderIfNotDefined(List<INotificationTemplate> templates)
Expand Down
61 changes: 61 additions & 0 deletions src/Altinn.Notifications.Core/Shared/Result.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
namespace Altinn.Notifications.Core.Shared;

/// <summary>
/// A simple implementation of the Result class to handle success XOR failure as separate return
/// values in a type safe way.
/// </summary>
/// <typeparam name="TValue">The type to be assigned to indicate success.</typeparam>
/// <typeparam name="TError">The type to be assigned to indicate failure.</typeparam>
public readonly struct Result<TValue, TError>
{
private readonly TValue? _value;
private readonly TError? _error;

private Result(TValue value)
{
IsError = false;
_value = value;
_error = default;
}

private Result(TError error)
{
IsError = true;
_value = default;
_error = error;
}

/// <summary>
/// Gets a value indicating whether the Result contains an error value.
/// </summary>
public bool IsError { get; }

/// <summary>
/// Gets a value indicating whether the Result contains a success value.
/// </summary>
public bool IsSuccess => !IsError;

/// <summary>
/// Implicit operator used when creating an instance of Result when assigning a success value.
/// </summary>
/// <param name="value">An object of the type indicating success.</param>
public static implicit operator Result<TValue, TError>(TValue value) => new(value);

/// <summary>
/// Implicit operator used when creating an instance of Result when assigning an error value.
/// </summary>
/// <param name="error">An object of the type indicating failure.</param>
public static implicit operator Result<TValue, TError>(TError error) => new(error);

/// <summary>
/// This method will call either the success OR the failure function based on it's error state.
/// </summary>
/// <typeparam name="TResult">The type to be returned by the given functions.</typeparam>
/// <param name="success">The function to call if Result holds a success value.</param>
/// <param name="failure">The function to call if Result holds an error value.</param>
/// <returns>An instance of the defined type.</returns>
public TResult Match<TResult>(
Func<TValue, TResult> success,
Func<TError, TResult> failure) =>
!IsError ? success(_value!) : failure(_error!);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace Altinn.Notifications.Core.Models;
namespace Altinn.Notifications.Core.Shared;

/// <summary>
/// A class representing a service error object used to transfere error information from service to controller.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using Altinn.Notifications.Configuration;
using Altinn.Notifications.Core.Models;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;
using Altinn.Notifications.Extensions;
using Altinn.Notifications.Mappers;
using Altinn.Notifications.Models;
Expand Down Expand Up @@ -71,14 +71,14 @@ public async Task<ActionResult<OrderIdExt>> Post(EmailNotificationOrderRequestEx
}

var orderRequest = emailNotificationOrderRequest.MapToOrderRequest(creator);
(NotificationOrder? registeredOrder, ServiceError? error) = await _orderRequestService.RegisterNotificationOrder(orderRequest);
Result<NotificationOrder, ServiceError> result = await _orderRequestService.RegisterNotificationOrder(orderRequest);

if (error != null)
{
return StatusCode(error.ErrorCode, error.ErrorMessage);
}

string selfLink = registeredOrder!.GetSelfLink();
return Accepted(selfLink, new OrderIdExt(registeredOrder!.Id));
return result.Match(
order =>
{
string selfLink = order.GetSelfLink();
return Accepted(selfLink, new OrderIdExt(order.Id));
},
error => StatusCode(error.ErrorCode, error.ErrorMessage));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Altinn.Notifications.Configuration;
using Altinn.Notifications.Core.Models.Notification;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;
using Altinn.Notifications.Extensions;
using Altinn.Notifications.Mappers;

Expand Down Expand Up @@ -50,14 +51,11 @@ public async Task<ActionResult<EmailNotificationSummaryExt>> Get([FromRoute] Gui
return Forbid();
}

var (emailSummary, error) = await _summaryService.GetEmailSummary(id, expectedCreator);
Result<EmailNotificationSummary, ServiceError> result = await _summaryService.GetEmailSummary(id, expectedCreator);

if (error != null)
{
return StatusCode(error.ErrorCode, error.ErrorMessage);
}

return Ok(emailSummary?.MapToEmailNotificationSummaryExt());
return result.Match(
summary => Ok(summary.MapToEmailNotificationSummaryExt()),
error => StatusCode(error.ErrorCode, error.ErrorMessage));
}
}
}
44 changes: 23 additions & 21 deletions src/Altinn.Notifications/Controllers/OrdersController.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using Altinn.Notifications.Configuration;
using Altinn.Notifications.Core.Models.Orders;
using Altinn.Notifications.Core.Services.Interfaces;
using Altinn.Notifications.Core.Shared;
using Altinn.Notifications.Extensions;
using Altinn.Notifications.Mappers;
using Altinn.Notifications.Models;
Expand Down Expand Up @@ -51,14 +53,14 @@ public async Task<ActionResult<NotificationOrderExt>> GetById([FromRoute] Guid i
return Forbid();
}

var (order, error) = await _getOrderService.GetOrderById(id, expectedCreator);
Result<NotificationOrder, ServiceError> result = await _getOrderService.GetOrderById(id, expectedCreator);

if (error != null)
{
return StatusCode(error.ErrorCode, error.ErrorMessage);
}

return order!.MapToNotificationOrderExt();
return result.Match<ActionResult<NotificationOrderExt>>(
order =>
{
return order.MapToNotificationOrderExt();
},
error => StatusCode(error.ErrorCode, error.ErrorMessage));
}

/// <summary>
Expand All @@ -77,14 +79,14 @@ public async Task<ActionResult<NotificationOrderListExt>> GetBySendersRef([FromQ
return Forbid();
}

var (orders, error) = await _getOrderService.GetOrdersBySendersReference(sendersReference, expectedCreator);
Result<List<NotificationOrder>, ServiceError> result = await _getOrderService.GetOrdersBySendersReference(sendersReference, expectedCreator);

if (error != null)
{
return StatusCode(error.ErrorCode, error.ErrorMessage);
}

return orders!.MapToNotificationOrderListExt();
return result.Match<ActionResult<NotificationOrderListExt>>(
orders =>
{
return orders.MapToNotificationOrderListExt();
},
error => StatusCode(error.ErrorCode, error.ErrorMessage));
}

/// <summary>
Expand All @@ -105,13 +107,13 @@ public async Task<ActionResult<NotificationOrderWithStatusExt>> GetWithStatusByI
return Forbid();
}

var (order, error) = await _getOrderService.GetOrderWithStatuById(id, expectedCreator);

if (error != null)
{
return StatusCode(error.ErrorCode, error.ErrorMessage);
}
Result<NotificationOrderWithStatus, ServiceError> result = await _getOrderService.GetOrderWithStatuById(id, expectedCreator);

return order!.MapToNotificationOrderWithStatusExt();
return result.Match<ActionResult<NotificationOrderWithStatusExt>>(
order =>
{
return order.MapToNotificationOrderWithStatusExt();
},
error => StatusCode(error.ErrorCode, error.ErrorMessage));
}
}
Loading

0 comments on commit 1c61702

Please sign in to comment.