Skip to content
This repository has been archived by the owner on Sep 7, 2023. It is now read-only.

Provide default init method implementation #734

Open
adrianffletcher opened this issue Feb 11, 2020 · 22 comments
Open

Provide default init method implementation #734

adrianffletcher opened this issue Feb 11, 2020 · 22 comments

Comments

@adrianffletcher
Copy link

I don't want to have to define a default init method when I have nothing special to define in the default state. I would like a default implementation to be provided in circumstances where I am not interested in specifying a stateId.

Screenshot 2020-02-10 at 14 20 32

@jeromesimeon
Copy link
Member

This issue would probably be best moved to Ergo.

There is a default init already, which I think is

clause init() : Response {
   set state AccordState {
       stateId: "...",
   }
  return Response{}
}

Does that address this issue, or is this about something else?

@jeromesimeon
Copy link
Member

This issue would probably be best moved to Ergo.

There is a default init already, which I think is

clause init() : Response {
   set state AccordState {
       stateId: "...",
   }
  return Response{}
}

Does that address this issue, or is this about something else?

I would be in favour of removing stateId altogether, but this might be a bit aggressive.

I'm not sure what the use case for stateId is and if anyone is using it. Feedback on this would be welcome.

@adrianffletcher
Copy link
Author

I would like a default implementation that works even if I have a custom state object. For example:

clause init(request : Request) : Response {
set state BusinessLicenseState {
stateId: "Initial State",
businessNameQuery: none,
businessLicense: none
};
return Response{}
}

@jeromesimeon
Copy link
Member

I would like a default implementation that works even if I have a custom state object. For example:

clause init(request : Request) : Response {
set state BusinessLicenseState {
stateId: "Initial State",
businessNameQuery: none,
businessLicense: none
};
return Response{}
}

Oh. I see, I didn't get that. That sounds like this would need the ability to synthesize a valid value based on its (arbitrary) type, which definitely requires some thinking and possibly non-trivia work.

I'm not personally a super big fan of magic things, which it feels like this is what it would be, but we could discuss requirements & design for something along those lines.

@jeromesimeon jeromesimeon transferred this issue from accordproject/models Feb 11, 2020
@jeromesimeon
Copy link
Member

@adrianffletcher Moved to the Ergo repo. Hope you don't mind.

@adrianffletcher
Copy link
Author

No worries. I don't really use stateId so I would really like the smart clause to be initialised with empty state without me having to do anything. This doesn't seem like magic to me as I would expect state variables to be initialised to none unless otherwise stated.

@jeromesimeon
Copy link
Member

jeromesimeon commented Feb 11, 2020

No worries. I don't really use stateId so I would really like the smart clause to be initialised with empty state without me having to do anything. This doesn't seem like magic to me as I would expect state variables to be initialised to none unless otherwise stated.

stateId -- noted!

initialise to none -- that wouldn't work unless all the fields are optional.

If your state says:

enum Country { o USA o France o Wales }
state MyLoanState {
  o Country jurisdiction
  o balance Double
  o Double[] payments
  o DateTime lastPayment
  o Boolean late
}

You need to invent atomic values (other wise how does the other clauses know what the balance is?).

e.g.,

MyLoanState {
  jurisdiction: USA,
  balance: 0.0, // or NaN?
  payments: [],
  lastPayment: now(), // dateTime("1970-01-01T00:00:00Z") ?
  late: false //or true?
}

Another way to think about it is if those are not initialised during init when are they initialized?

@jeromesimeon
Copy link
Member

Another way to think about it is if those are not initialised during init when are they initialized?

the "magic" part to me is: what if one contract wants the late field to be false by default and the other wants the late field to be true. This is highly contract dependent, and defaults have a way to creep up when you least expect it.

@jeromesimeon
Copy link
Member

Another way to think about it is if those are not initialised during init when are they initialized?

the "magic" part to me is: what if one contract wants the late field to be false by default and the other wants the late field to be true. This is highly contract dependent, and defaults have a way to creep up when you least expect it.

Note that if we do synthesize a default a developer could possibly change that default in a nicer way with e.g.,:

clause init() : Response {
  set state.late = false; // Overriding the default `late: true`
  return Response{}
}

@mttrbrts
Copy link
Sponsor Member

Could a default state instance be an optional part of the template definition? For example, something like state_default.json in the root directory.

If it exists, it is used during initialisation?

@jeromesimeon
Copy link
Member

Could a default state instance be an optional part of the template definition? For example, something like state_default.json in the root directory.

If it exists, it is used during initialisation?

How is that better than doing it inside the code? (which you will need anyway if you e.g., want to calculate something based on the contract values (e.g., balance would be the loan amount in the contract).

@adrianffletcher
Copy link
Author

I would like to define the standard default state when defining the state model. Then when I don't provide an init method I just get the default state defined in the model.

@jeromesimeon
Copy link
Member

jeromesimeon commented Feb 12, 2020

I would like to define the standard default state when defining the state model. Then when I don't provide an init method I just get the default state defined in the model.

That's interesting. Defining a default value with the type itself isn't a bad idea.

A few observations:

  • It's still the same information / nb of lines
  • It does not cover the case where the default state is based on the contract data, so I'm not sure how to remove this from Ergo entirely (for all use cases)
  • We might need to move this issue to Concerto...
  • We do suffer a bit from having things defined in many different places there. Types one place, logic another...

@jeromesimeon
Copy link
Member

I'll rephrase some of the questions raised by this issue with a bit of an analogy:

  1. can we provide a default constructor for a class? (often yes, with some interesting design decision to be made as to when and with what kind of default specification)
  2. can we remove the need for class constructors altogether? (probably not, at least not without some loss of generality / flexibility, and I am not sure I would recommend it).

@jeromesimeon
Copy link
Member

jeromesimeon commented Feb 12, 2020

Data point: Concerto has already a notion of default.

bash-3.2$ cat address.cto
namespace org.acme

concept Address {
  o String city default="Winchester"
  o String country optional
  o String state default="NY"
  o String street optional
  o Long zip default=10027
  o Boolean business default=false
}
bash-3.2$ cat address.json 
{ "$class" : "org.acme.Address" }
bash-3.2$ concerto validate --sample address.json --ctoFiles address.cto
9:23:47 PM - info:
{
  "$class": "org.acme.Address",
  "city": "Winchester",
  "state": "NY",
  "zip": 10027,
  "business": false
}

@jeromesimeon
Copy link
Member

Automatic creation of a value could be part of the editing experience. VSCode hot key or something else.

@jeromesimeon
Copy link
Member

jeromesimeon commented Feb 14, 2020

Proposed resolution:

  1. Remove stateId from the base models (that is related to Base/Cicero Accord Project Models Revisions for version 1.0 models#93)
  2. Add tooling support for creating init clauses
  3. Add tooling support to automatically generate Ergo stub for object creation. See Smart Editing for Ergo object creation expressions based on CTO models vscode-extension#36

Note: a benefit of 3. is that it would work for all classes creation, not just for init.

@jeromesimeon
Copy link
Member

@adrianffletcher Let us know if the proposed resolution seems reasonable to you.

@adrianffletcher
Copy link
Author

@jeromesimeon That sounds great to me. I particularly like the idea of generating a .ergo file from a model.cto file, creating a default init method taking into account the default values and creating a default request clause.

@dselman
Copy link
Sponsor Contributor

dselman commented Feb 14, 2020

Time for your first Open Source contribution? I can walk you through the VS extension code...

@adrianffletcher
Copy link
Author

Sounds good.

@jeromesimeon
Copy link
Member

jeromesimeon commented Apr 13, 2021

@adrianffletcher The first item below (removal of stateId) is done, published as part of Cicero v0.22.0-alpha.* at the moment.

I think we can move this issue fo the VSCode extension?

Proposed resolution:

  1. Remove stateId from the base models (that is related to accordproject/models#93)
  2. Add tooling support for creating init clauses
  3. Add tooling support to automatically generate Ergo stub for object creation. See accordproject/cicero-vscode-extension#36

Note: a benefit of 3. is that it would work for all classes creation, not just for init.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants