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

Immutability & Encapsulation of Entity struct fields #41

Open
BrianLusina opened this issue Oct 4, 2022 · 0 comments · Fixed by #42
Open

Immutability & Encapsulation of Entity struct fields #41

BrianLusina opened this issue Oct 4, 2022 · 0 comments · Fixed by #42
Assignees
Labels
enhancement New feature or request

Comments

@BrianLusina
Copy link
Member

Description

Currently it is possible to get fields for an initialised struct such as URL or User. The preference instead should be to make these fields immutable(unexported) & only possible to update them using getters & setters. This ensures that invalid data is not set on objects & makes it more robust, scalable & maintainable, alongside the added benefit of making it easier to reason about.

Summary

Currently, it is quite easy to create a URL like so:

url := entities.URL{ 
  originalUrl: "http://example.com",
  shortCode: "hfaneuonfaouefnaoudfenaf",
  userId: identifier.New(),
  // ...Other fields ommited for brevity...
}

Even though this is valid, it violates a lot of the rules set in the domain, such as, we should not have a very long shortCode. That short code is longer than the original url which violates the reason for being for this service.

We should instead be able to only create a new url like so(which already happens btw, however, this should now be the only way to do so):

url, err := entities.NewUrl("https://example.com", "hfaneuonfaouefnaoudfenaf", identifier.New(), //other arguments) 
if err != nil {
  return nil, err
}

This allows us to perform validation within the NewUrl function before we can create a URL entity that only contains valid fields.

While performing updates to certain fields we should only be able to perform such updates like below:

// preferred approach

func (url *URL) SetCustomAlias(customAlias string) error {
    err := validateCustomAlias(customAlias)
    
    if err != nil {
       return err
    }

    url.customAlias = customAlias
    return nil
}

// current approach

newCustomAlias = "someAlias"
err := url.SetCustomAlias(newCustomAlias)
if err != nil {
  panic(err)
}

// instead of
url.CustomAlias = "someAlias"

The first example is the preferred approach as it allows us to be strict on what a custom alias field on a URL entity can be set to. Additionally, we can perform further logic on it to ensure it meets the service's requirements.

This should be the same for the User entity as well as other entities that will be added to the system.

Same applies to getting the fields of an entity. There should be Getter methods on the entities.

// preferred approach
func (url *URL) GetCustomAlias() string {
    return url.customAlias
}

// current approach
customAlias := url.CustomAlias

An example using the URL entity

Even though the current approach is okay, the preferred approach allows us to extend the attributes/properties of fields on these entities. For example, we could add a prefix to a URL id when getting the ID or other fancy features on fields without modifying those fields.

Acceptance Criteria

  1. It should not be possible to update a field on an entity directly, e.g. url.ExpiresOn = expiresOn Instead it should be url.SetExpiresOn(expiresOn).
  2. The only way to access fields of entities outside the package of the entity should be through Getter methods
  3. Valid unit tests should be set to test this functionality to ensure nothing breaks
  4. These should be set on the domain entities, the database models should be left as is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant