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

속성 값을 동반한 개정 모델 인스턴스 생성 #15

Open
gyuwon opened this issue Feb 14, 2016 · 15 comments
Open

속성 값을 동반한 개정 모델 인스턴스 생성 #15

gyuwon opened this issue Feb 14, 2016 · 15 comments
Assignees
Labels

Comments

@gyuwon
Copy link
Contributor

gyuwon commented Feb 14, 2016

모델의 특정 속성 혹은 속성들의 값을 변경한 개정 인스턴스를 생성하는 작업을 도와주는 확장 메서드 집합을 제공합니다. 아래 코드를 참고하세요.

public User
{
    public User(Guid id, string userName, string email, string bio)
    {
        Id = id;
        UserName = userName;
        Email = email;
        Bio = bio;
    }

    public Guid Id { get; }
    public string UserName { get; }
    public string Email { get; }
    public string Bio { get; }
}

var user = new User(
    Guid.NewGuid(),
    "Obiwan Kenobi",
    "obiwan@jedi.com",
    "Jedi master");

User revision1 = user.ReviseWith(x => x.Email, "ben@tatooine.com");

User revision2 = revision1
    .Revise()
    .With(x => x.UserName, "Ben Kenobi")
    .With(x => x.Bio, "The Force will be with you, always.");
@gongdo
Copy link
Collaborator

gongdo commented Mar 5, 2016

위의 예제대로라면 T : class 전체를 확장하는건가요? 아니면 IModel에 대해서만 확장을 하는건가요?

  • IModel이 generic이라 확장 메서드를 깔끔하게 쓰기가 상당히 어렵군요.
    IModel에 대해 확장을 한다면 이런 형태가 되겠죠.
public static TModel ReviseWith<TModel, TId, TProperty>(
            this IModel<TId> model,
            Expression<Func<TModel, TProperty>> selectorExpression,
            TProperty value)
            where TModel : class, IModel<TId>
            where TId : IEquatable<TId>
        {
            return default(TModel);
        }

하지만 이걸 사용할 때에는 매번 3개의 타입을 명시해야 합니다.
이렇게되면 확장메서드로서의 의미가 별로 없겠죠.

new User().ReviseWith<User, int, string>(u => u.Email, "chu@ba.ka");

그렇다고 모든 class에 대해 확장하자니 다소 시끄러울 것 같군요.
그 외에 IModel<T> : IModel 식으로 빈 인터페이스를 만들어주는 방법도 있긴하겠습니다만...

  1. 확장메서드를 쓸 때 명시적인 타입 인자 사용
  2. 모든 class에 대해 확장
  3. non-generic IModel을 기반 인터페이스로 추가

다른 방법이 있을까요?

두 번째 케이스도 .Revise()로 시작했다는 점 외에는 With()의 용법이 첫 번째와 동일합니다.
With()의 최종값이 주어진 타입 IModel이기 때문이죠.

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 6, 2016

@gongdo 음... 글쎄요... 일단 프레임워크에서 사용할 용도는 IModel<>이면 되는데 일반 참조형식을 지원할 필요가 있는지는 생각해보지 않았어요. 지금으로서는 IModel<> 인터페이스로 제한해야하는 이유는 없을 것 같은데 T : class를 지원하는 것은 조심해야하는 일이기도 하죠. 일단 형추론이 어디까지 되는지 궁금하네요.

@gongdo
Copy link
Collaborator

gongdo commented Mar 6, 2016

@gyuwon 어제 바로 해봤는데 generic-generic 타입 파라미터에 대한 형추론은 지원하지 않더라고요.
제가 방법을 몰라서 그런가 싶어서 이렇게 저렇게 해봤는데 'cannot inferred from the usage' 만 보게 되네요. 말씀드린대로 제가 생각할 수 있는 처리 방안은 위의 세가지인데 만약 하나만 고르라고 하면 2번이 가장 용도에 적합하지 않을까 싶습니다.
보완책으로 2번 확장메서드의 네임스페이스를 약간 다르게 하는 방법도 있겠지요. Flip.Models라던가 Flip.Extensions라던가...

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 6, 2016

@gongdo 글쿤요... 저도 위 3개 방법 중에서는 2번이 가장 마음에 듭니다.

@gongdo
Copy link
Collaborator

gongdo commented Mar 6, 2016

확장메서드 예시중 첫번째 용법은 이런 문법도 지원하는건 어떨까요?

var revision = model.ReviseWith(x => new
{
  UserName = "Ben@Kenobi",
  Bio = "Not enough Force.",
});

확장메서드 예시중 두번째 용법은 fluent스러운 코드를 작성할 때 가장 흔하게 겪는 중복 오퍼레이션의 오버헤드가 걱정됩니다.

T With(this T model)은 아마도 내부적으로 T 타입에 대해 reflection이 필요로 할 텐데요, 연결을 할 때마다 중복으로 reflection하는건 매우 비효율적이겠습니다. 이걸 캐싱등의 기법으로 T 타입에 대한 중복 reflection을 막을 수는 있겠습니다만 이런 범용적인 확장메서드에 정적 캐시가 붙는 것도 다소 부담스럽습니다.

이런 연쇄가 필요한 경우 다음과 같은 패턴으로 이 확장메서드가 하려는 행위에 대한 컨텍스트를 유지하는게 좋을 것 같습니다.

var revision = model
  .BeginReviseContext()  // <-- 컨텍스트만 생성
  .With(x => x.Email, "Ben@Kenobi")
  .With(x => x.Bio, "Blah...")
  .DoSomething()
  .AndOthers()
  .Revise();  // <-- 컨텍스트에 쌓인 기능을 한 번에 실행

문제라면, 예시에서 보여준 것과 같이 깔끔한 단어는 잘 떠오르지 않는 정도일까요?
특히 컨텍스트를 시작하는 것을 뭐라 부를지도 좀 애매하긴 합니다.
제 생각에는 굳이 두번째 용법을 지원하는 것 보다는 위에서 제가 제안한 anonymous type도 받을 수 있게 하는게 Revise 확장메서드의 목적에 부합한다고 생각합니다.

(추가:) AutoMapper에서 힌트를 얻었습니다. 두 번째 용법은 이런식으로도 할 수 있겠군요.

var revision = model
  .Revise(revise =>     // <-- ReviceContext 연쇄, 컨텍스트내에 실행할 동작을 추가하는 방식
    revise
    .With(x => x.Email, "Ben@Kenobi")
    .With(x => x.Bio, "Blah...")
    .Increase(x => x.Counter, 1)
    .SomethingMore()
  );

또는 이런식으로 컨텍스트를 재활용할 수도 있겠죠.

var revise = model.GetReviseContext()
  .With(x => x.Email, "Ben@Kenobi")
  .With(x => x.Bio, "Blah...");
var revision = model.Revise(revise);

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

@gongdo 익명형식을 사용하는 것은 표현에 있어서 유용한 반면 컴파일 타임 오류 검출이 되지 않는 단점이 있겠네요. 컨텍스트별 범위의 리플렉션 캐싱도 장단점이 있을 것 같습니다. 자주 사용되는 형식에 대해서는 단점이겠고 일회성 코드라면 효율적이겠죠.

어쩌면 지금 거론된 기능 중 몇 가지를 함께 지원하는 것도 방법이겠습니다.

@gongdo
Copy link
Collaborator

gongdo commented Mar 7, 2016

@gyuwon flip 전역에서 리플렉션 캐싱을 공통으로 사용하는 방안은 어떤가요?
배보다 배꼽이 큰 격이긴한데... 단순하게 ConcurrentDictionary 정도로 관리해도 큰 문제는 없을 것 같습니다.
단, 제가 정확히 몰라서 그런데, 타입 메타데이터(attribute라던가)가 동적으로 타입에 추가될 수 있을까요?
그렇지만 않다면 한 런타임에 하나의 타입에 대한 리플렉션은 단 한 번만 있으면 되니까요.

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

@gongdo 저는 동일한 논리가 동일한 결과를 만들어 캐싱하는 것이라면 굳이 동시성 고려를 하지 않아도 된다고 생각합니다. 즉 그냥,

private static int? _three;

public static int Three
{
  get
  {
    if (_three == null)
      _three = 1 * 3;
    return _three.Value;
  }
}

이렇게 하는 것이 1 * 3 연산의 중복 수행의 가능성이 존재하더라도 치명적인 성능 저하를 야기하지 않는거나 싱글톤 보장이 필요 없다면 괜찮다는 의견이에요.

@gongdo
Copy link
Collaborator

gongdo commented Mar 7, 2016

@gyuwon 넵 동의합니다. 그럼 이 확장메서드의 경우 컨텍스트 연쇄보다는 개별 호출에서 리플렉션 캐시를 이용하는쪽이 더 낫다고 보시는건가요?

최종 pseudo code를 부탁드립니다.

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

@gongdo 저는 제가 작성한 원안과 익명 형식을 사용한 방식 두 가지를 지원하면 어떨까합니다. 후자가 전자의 코드를 활용하는 방식이면 적절할 듯 합니다. 어떤가요?

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

@gongdo 아, 그리고 익명 형식을 사용한 방식에서

var revision = model.ReviseWith(x => new
{
  UserName = "Ben@Kenobi",
  Bio = "Not enough Force.",
});

매개변수 x는 어떤 역할을 하게되나요?

@gongdo
Copy link
Collaborator

gongdo commented Mar 7, 2016

어라? 그러고보니 저런 식으로는 필요가 없었네요.
Expression<Func<TRevision>>로 고치고 아래와 같이 할 수는 있겠습니다.

var revision = model.ReviseWith(() => new
{
  UserName = "Ben@Kenobi",
  Bio = "Not enough Force.",
});

하지만 이렇게 하면 위에서 말씀하신것처럼 컴파일타임 오류도 잡을 수 없고 사이드이펙트만 늘어날 것 같습니다. 지원하지 않는 편이 더 낫다고 봅니다.

최초에 제안하신 용법 1.은 문제가 없고, 용법 2.는 불가능하진 않지만 결과적으로 TModel.With()가 되어버려 의미가 불명확해집니다.

용법 2와 같이 '연쇄'를 하려면 결국 컨텍스트 방식이 필요하겠습니다.

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

@gongdo 그렇다면 이 이슈에서는 용법 1을 우선 처리하고 추가 이슈를 통해 더 논의해볼까요?

@gongdo
Copy link
Collaborator

gongdo commented Mar 7, 2016

네 그게 좋을 것 같습니다. 실제로 써보고 더 편한게 있으면 좋겠다라던가 더 구체적인 기능이 필요하면 추가하죠.

마지막으로 지금까지 논의된 것중에서 유의미한 형태만 정리했습니다.

  1. 용법 1: model.ReviseWith()
  2. 익명형식: () => { ... }
  3. 컨텍스트 연쇄: model.ReviseWith(revise => revise.Set(...).Set(...))

이 중 1번만 우선 구현하는 걸로 하겠습니다. 제가 하죠. :)

(+노트) 용법1은 전역 리플렉션 캐싱을 추가하는 것이 좋겠습니다.

@gyuwon
Copy link
Contributor Author

gyuwon commented Mar 7, 2016

Okay.

@gongdo gongdo self-assigned this Mar 7, 2016
@gyuwon gyuwon added the ready label Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants