[Question]: userId property in User class (also postId in Post etc.)

[vprenner]

A few questions basically:

1. What is the purpose of the userId/postId properties when there is already one called id in the AggregateRoot class? Especially since they are initialized with the same value.
2. Why is UserId an Entity?
3. Why does its create static method return a Result even though it will always be a success?

[stemmlerjs]

1. Making the implicit explicit. Making the differentiation between a PostId and an Id in general. I can lean on it not being entirely necessary. If we just used the UniqueIdentifier as a type instead, that'd make things a lot simpler, but it does help type-checking so that we don't mix types. I'll think about it, but really - either way is fine.

2. It shouldn't be. Allow me to fix that in a PR. They should be value objects.

3. Some code is missing here. We need to do null-checking.

Here's an example of what (3) might look like.

import { UniqueEntityID } from "../../../shared/domain/UniqueEntityID";
import { Result } from "../../../shared/core/Result";
import { ValueObject } from "../../../shared/domain/ValueObject";
import { Guard } from "../../../shared/core/Guard";

export class PostId extends ValueObject<{ value: UniqueEntityID }> {

  getValue (): UniqueEntityID {
    return this.props.value;
  }

  private constructor (value: UniqueEntityID) {
    super({ value });
  }

  public static create (value: UniqueEntityID): Result<PostId> {
    let guardResult = Guard.againstNullOrUndefined(value, 'value');
    if (guardResult.isFailure) {
      return Result.fail<PostId>(guardResult.getErrorValue())
    }
    return Result.ok<PostId>(new PostId(value));
  }
}

Does that help at all? Any other questions?

[vprenner]

1. • makes sense
2. • makes very much sense
3. • Originally, when PostId was a subclass of Entity this didn't make sense because if you pass null to the constructor you get a new uuid() value. This way it makes more sense, although, isn't it enough that the create function method requires a non-null value?

[stemmlerjs]

@vprenner I thought about it, and you're definitely right. Walking through the logic, it could greatly simplify things to just fall back to using UniqueIdentifier for the ids. Great call. I'll throw up a PR and refactor this. Thank you!