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

Computed field for entity #68

Open
vma-vortx opened this issue Dec 13, 2022 · 6 comments
Open

Computed field for entity #68

vma-vortx opened this issue Dec 13, 2022 · 6 comments
Labels
enhancement New feature or request

Comments

@vma-vortx
Copy link

Describe the solution you'd like
Like in Object-oriented design, I'd like to read a field from an entity based on values of other fields. This would bring a concept of encapsulation to herbs' entities. This property, ideally, should not be able to be modified.

For example:

var person = new Person();
person.Name = "Victor;
person.Surname = "Melias";
console.log(person.Fullname); //should return "Victor Melias"

Additional context
In OOD languages this can be done by setting the properties with "private set", but there's no alternative in Herbs to do this without any kind of workaround

@vma-vortx vma-vortx added the enhancement New feature or request label Dec 13, 2022
@dalssoft
Copy link
Member

Hi @VictorTBX , thanks for your suggestion.

It seems your suggestion is related to the concept of computed properties, which is a feature that is not yet available in Herbs.

Here are some benchmarks for this feature:

Django: https://docs.djangoproject.com/en/4.1/topics/db/models/#model-methods
Laravel: https://laravel.com/docs/8.x/eloquent-mutators#defining-an-accessor
Phoenix: https://hexdocs.pm/phoenix/ecto_models.html#computed-fields
PostgreSQL: https://www.postgresql.org/docs/current/ddl-generated-columns.html
SQL Server: https://learn.microsoft.com/en-us/sql/relational-databases/tables/specify-computed-columns-in-a-table?view=sql-server-ver16
and many others

On vanila JS, you can use getters and setters to achieve this:

const person = {
  name: 'Victor',
  surname: 'Melias',
  get fullname() {
    return `${this.name} ${this.surname}`
  }
}

With that, my suggestion would be a new parameter in the entity definition, something like:

const Person = entity('Person', {
  id: field(Number),
  name: field(String),
  surname: field(String),
  fullname: field(String, { value: (person) => `${person.name} ${person.surname}` })
})

A few things to consider:

  • The value parameter should be a function that receives the entity as a parameter and returns the value of the field.

Ex:

const Person = entity('Person', {
    ...
    fullname: field(String, { value: (person) => `${person.name} ${person.surname}` })
})

or

const Order = entity('Order', {
    ...
    total: field(Number, { value: (order) => order.items.reduce((total, item) => total + item.price, 0) })
})
  • The value of a calculated field should should be calculated every time it is accessed, so that the value is always up to date.
const person = new Person({ name: 'Victor', surname: 'Melias' })
console.log(person.fullname) // should return 'Victor Melias'
person.name = 'John'
console.log(person.fullname) // should return 'John Melias'
  • The value of a calculated field should be read-only, so that it cannot be modified.
const person = new Person({ name: 'Victor', surname: 'Melias' })
console.log(person.fullname) // should return 'Victor Melias'
person.fullname = 'John Melias' // should throw an error
  • The value of a calculated field should be used as a normal field, so that it can be used in glues, like herbs2rest and herbs2gql.
const person = new Person({ name: 'Victor', surname: 'Melias' })
console.log(person.fullname) // should return 'Victor Melias'
console.log(person.toJSON()) // should return { id: 1, name: 'Victor', surname: 'Melias', fullname: 'Victor Melias' }
  • The function that calculates the value of a calculated field should be able to access other calculated fields.
const Person = entity('Person', {
    ...
    fullname: field(String, { value: (person) => `${person.name} ${person.surname}` }),
    fullnameLength: field(Number, { value: (person) => person.fullname.length })
})
  • The function that calculates the value could be asynchronous, so that it can access data from other sources.
const Person = entity('Person', {
    ...
    fullname: field(String, { value: async (person) => {
        const { name, surname } = await fetchPerson(person.id)
        return `${name} ${surname}`
    } })
})

This is open to discussion, since this would change how to call a field (sync vs async).

const person = new Person({ name: 'Victor', surname: 'Melias' })
console.log(await person.fullname) // should return 'Victor Melias'

The problem here is that this also changes how to retrive value of a calculated field in glues, like herbs2rest and herbs2gql since they are expecting a sync value.

So having a async value could be a next step, so I think it would be better to have a sync value first.

  • The function that calculates the value should work with this:
const Person = entity('Person', {
    ...
    fullname: field(String, { value: function () {
        return `${this.name} ${this.surname}`
    } })
})

What else do you think we should consider?

Again, thanks for your suggestion. Let's discuss it and see if we can implement it. I think it would be a great feature.

@vxsander
Copy link

  • Should be able to create a field only if a requirement has been met
const Person = entity('Person', {
  id: field(Number),
  name: field(String),
  surname: field(String),
  fullname: field(String, { value: function () {
    if (person.name && person.surname)    
      return `${this.name} ${this.surname}`
    } })
})
const person = new Person({ surname: 'Melias' })
console.log(person.fullname) // should return undefined
person.name = 'Victor'
console.log(person.fullname) // should return 'Victor Melias'

@vma-vortx
Copy link
Author

Great, thanks @dalssoft

I think those are all things to be considered and will help to develop this feature.
The async I can't see an application for this, since entities should not retrieve information from external sources and all entities methods can be done as sync.

@vma-vortx
Copy link
Author

I started developing this feature, but I'm stucked with two problems:

  • The function that calculates the value of a calculated field should be able to access other calculated fields.
  • The function that calculates the value should work with this:

How mandatory are these two concepts?

The async is not developed as well, but I have successfully tested the other features, like toJson()

@dalssoft
Copy link
Member

Great! Thanks @VictorTBX .

I recommend you to create a PR and we can discuss the issues you are facing there. I'm curios to understand why these features are a problem. I think it will be clear with code.

VictorTBX pushed a commit to VictorTBX/gotu that referenced this issue Dec 28, 2022
@VictorTBX
Copy link
Contributor

@dalssoft

I've opened a draft #69

@dalssoft dalssoft changed the title Readonly field for entities Computed field for entity Jan 3, 2023
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
Status: Ready to code
Development

No branches or pull requests

4 participants