Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Order Domain Objects - Cart, CartItem, CartItemOption (#1) #2

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
47 changes: 47 additions & 0 deletions src/AUSG.Eats.Order.Domain/Cart.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
namespace AUSG.Eats.Order.Domain;

public class Cart
{
private readonly List<CartItem> _items = new();

private long? _userId;

public Cart(long? userId = null)
{
_userId = userId;
}

public IEnumerable<CartItem> Items => _items.AsReadOnly();

public void AddToCart(CartItem newItem)
{
_items.Add(newItem);
}

public void RemoveFromCart(CartItem itemToRemove)
{
_items.Remove(itemToRemove);
}

public void AlterCartItem(CartItem itemToChange)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public UpdateCartItem(CartItem.Id id, Action<CartItem> action) {
    action(_items.First(item => item.Id == id)
}

사실 요것도 좀 걸리긴하지만 이전보단 낫지 않을까요?

도메인적인 요구사항을 외부로 노출시키는 용도라면 UpdateQuantity(Id id, int num)이 더 나아보이긴 해요.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. AlterCartItem의 구현은 best practice를 참고했거나 이론적 근거가 있지는 않았습니다.
  2. 올려주신 코드처럼 CartItem => void도 상태 갱신의 유연성 측면에선 가장 좋을 것 같습니다.
  3. 지금처럼 특정 상태 갱신에 대한 애그리거트 수준의 불변식 요구사항이 없을 때는 가장 좋은 선택일 것 같습니다.

한 가지 의견이 있다면

  • CartItem#Id보다 CartItem만 참조해도 비교가 가능해서 엔터티로 받아도 충분할 것 같은데 그렇게 해도 될까요?

아래는 말씀하신 UpdateQuantity를 보고 든 생각인데요, Cart 애그리거트 차원에서 불변식을 만족시켜야 할 때는 Update~~~가 유의미할 것 같습니다.

다만 현재의 도메인 분석/요구 사항에는 해당 내부 엔터티(CartItem)의 상태를 넘어서는 불변식이 아직 없어서, 지금처럼 상태를 바꾸는 정도로 충분하지 않을까요? (ex: 아이템 수량은 모든 장바구니 아이템들을 통틀어 최대 10개까지만 담기 가능)

  • 애그리거트 차원에서 불변식 강제가 필요 없는 상황에서 내부 엔터티의 상태 변경에 대응되는 상태 갱신 메소드를 만든다면 애그리거트 <=> 내부 엔터티간 결합은 커지는데 이익은 모호한 것 같습니다.

그래서 제 생각은 (위에서처럼) 도메인 요구사항이 딱히 없는 경우 상태 갱신은 replace 혹은 콜백 형태로 구현하되, 일부 불변식 조건이 필요한 경우 애그리거트 차원에서 불변식 강제를 위해 애그리거트에서 public API를 제공하는 게 좋을 것 같습니다.

(그리고 지금은 CartItem 엔터티의 내부 필드가 없어서 지금 updateQuantity를 구현할 순 없을 것 같습니다...)


만약 콜백 함수 방식으로 처리한다면, 상태 갱신을 하는 메소드를 노출해야 하는지 궁금합니다.

{
if (!_items.Remove(itemToChange)) // Id 비교이므로 제거된다.
throw new ArgumentException("없는 CartItem을 변경하려고 한다.");
_items.Add(itemToChange);
}

public override bool Equals(object? obj)
{
// use null propagation (잘 이해 못하고 있습니다.)
// see TC: CSharpTests#compare_with_null_instance_returns_false
if (obj is not Cart other)
return false;
return _userId == other._userId;
}

public override int GetHashCode()
{
// Non-readonly property referenced in 'GetHashCode()' ??
return _userId.GetHashCode();
}
}
26 changes: 26 additions & 0 deletions src/AUSG.Eats.Order.Domain/CartItem.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
namespace AUSG.Eats.Order.Domain;

public class CartItem
{
private long? _id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Id VO 따로 만들어주시면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Id VO가 어떤 것을 말씀하신 건지 잘 모르겠습니다 =(


public CartItem(long? id)
{
_id = id;
}

public override bool Equals(object? obj)
{
// use null propagation (잘 이해 못하고 있습니다.)
// see TC: CSharpTests#compare_with_null_instance_returns_false
if (obj is not CartItem other)
return false;
return _id == other._id;
}

public override int GetHashCode()
{
// Non-readonly property referenced in 'GetHashCode()' ??
return _id.GetHashCode();
}
}
61 changes: 61 additions & 0 deletions src/AUSG.Eats.Order.Test/CSharpTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using Xunit;

namespace AUSG.Eats.Order.Test;

public class CSharpTests
{
[Fact(DisplayName = "Integer fields are initialized to 0")]
public void int_init_value_is_0_in_class()
{
var sut = new IntInit();

Assert.Equal(0, sut.id);
}

[Fact(DisplayName = "Nullable integer fields are initialized to 0")]
public void nullable_int_init_value_is_null_in_class()
{
var sut = new IntInit();

// Assert.Null은 사용할 수 없다.
// Boxing allocation: conversion from 'int?' to 'object' requires boxing of value type
// Assert.Null(sut.nullableId);

// Nullable<int?>의 null 여부를 체크할 수 있는 메소드가 없다.
Assert.True(sut.nullableId == null);
}

[Fact(DisplayName = "Comparing with null instance returns false using null propagation")]
public void compare_with_null_instance_returns_false()
{
var NonNullInstance = new EqualsImplEx(1L);
var NullInstance = new EqualsImplEx();

Assert.True(NullInstance.Id == null);
Assert.NotEqual(NonNullInstance, NullInstance);
}

private class IntInit
{
public int id { get; set; }
public int? nullableId { get; set; }
}

private class EqualsImplEx
{
public EqualsImplEx(long? id = null)
{
Id = id;
}

public long? Id { get; }

public override bool Equals(object? obj)
{
// use null propagation
if (obj is not EqualsImplEx other)
return false;
return Id == other.Id;
}
}
}
104 changes: 104 additions & 0 deletions src/AUSG.Eats.Order.Test/OrderTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
using AUSG.Eats.Order.Domain;
using Xunit;

namespace AUSG.Eats.Order.Test;

public class OrderTests
{
private CartItem MakeNewCartItem(long? id = null)
{
return new CartItem(id);
}

[Fact(DisplayName = "CartItem 객체는 Id로 식별할 수 있다.")]
public void CartItem_Can_Be_Identified_By_Id()
{
// given
const long pid = 1L;
var cartItem1 = MakeNewCartItem(pid);
var cartItem2 = MakeNewCartItem(pid);

// then
Assert.Equal(cartItem1, cartItem2);
Assert.Equal(cartItem1.GetHashCode(), cartItem2.GetHashCode());
}

[Fact(DisplayName = "Cart 객체는 UserId로 식별할 수 있다.")]
public void Cart_Shares_UserId_and_Can_be_Identified_Using_UserId()
{
// given
var userId = 1L;
var cart1 = new Cart(userId);
var cart2 = new Cart(userId);

// then
Assert.Equal(cart1, cart2);
Assert.Equal(cart1.GetHashCode(), cart2.GetHashCode());
}

[Fact(DisplayName = "Cart 객체는 CartItem을 추가할 수 있다.")]
public void Cart_can_Add_CartItem()
{
// given
var cart = new Cart();
var cartItem1 = MakeNewCartItem();

// when
cart.AddToCart(cartItem1);

// then
Assert.Equal(cart.Items.ElementAt(0), cartItem1);
}

[Fact(DisplayName = "Cart 객체는 CartItem을 제거할 수 있다.")]
public void Cart_can_Remove_CartItem()
{
// given
var cart = new Cart();
var cartItem1 = MakeNewCartItem();
cart.AddToCart(cartItem1);
Assert.Equal(cart.Items.ElementAt(0), cartItem1);

// when
cart.RemoveFromCart(cartItem1);

// then
Assert.Empty(cart.Items);
}

[Fact(DisplayName = "Cart 객체는 CartItem을 변경할 수 있다.")]
public void Cart_can_Alter_CartItem()
{
// given
const long cartItemId = 1L;
var cart = new Cart();
var oldItem = MakeNewCartItem(cartItemId);
cart.AddToCart(oldItem);

// when
var newItem = MakeNewCartItem(cartItemId);
cart.AlterCartItem(newItem);

// then
var alteredItem = cart.Items.ElementAt(0);
Assert.Equal(alteredItem, oldItem);
}

[Fact(DisplayName = "Cart 객체는 CartItem을 변경할 때 해당 Item이 이미 List에 없다면 예외를 발생시킨다.")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Cart에 담기지 않은 CartItem을 수정하려고 할 수 없다." 요로케 하면 좋지 않을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네. 예외는 기술적인 명세다 보니 말씀하신 이름이 도메인적으로 더 좋은 네이밍일 것 같습니다.

public void Cart_throw_ArgumentException_when_Alter_CartItem()
{
// given
var cart = new Cart();
const long cartItemIdWhichDoesNotExist = 1L;
var oldItem = MakeNewCartItem(cartItemIdWhichDoesNotExist);

// when
void AlterItem()
{
cart.AlterCartItem(oldItem);
}

// then
Assert.Throws<ArgumentException>((Action) AlterItem);
}
}