ergo icon indicating copy to clipboard operation
ergo copied to clipboard

Provide default init method implementation

Open adrianffletcher opened this issue 5 years ago • 22 comments

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

adrianffletcher avatar Feb 11 '20 20:02 adrianffletcher

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 avatar Feb 11 '20 20:02 jeromesimeon

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.

jeromesimeon avatar Feb 11 '20 20:02 jeromesimeon

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{} }

adrianffletcher avatar Feb 11 '20 20:02 adrianffletcher

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 avatar Feb 11 '20 20:02 jeromesimeon

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

jeromesimeon avatar Feb 11 '20 20:02 jeromesimeon

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.

adrianffletcher avatar Feb 11 '20 20:02 adrianffletcher

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 avatar Feb 11 '20 20:02 jeromesimeon

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 avatar Feb 11 '20 20:02 jeromesimeon

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{}
}

jeromesimeon avatar Feb 11 '20 20:02 jeromesimeon

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?

mttrbrts avatar Feb 11 '20 20:02 mttrbrts

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).

jeromesimeon avatar Feb 11 '20 20:02 jeromesimeon

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.

adrianffletcher avatar Feb 11 '20 21:02 adrianffletcher

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 avatar Feb 12 '20 01:02 jeromesimeon

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 avatar Feb 12 '20 02:02 jeromesimeon

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 avatar Feb 12 '20 02:02 jeromesimeon

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

jeromesimeon avatar Feb 14 '20 16:02 jeromesimeon

Proposed resolution:

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

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

jeromesimeon avatar Feb 14 '20 17:02 jeromesimeon

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

jeromesimeon avatar Feb 14 '20 17:02 jeromesimeon

@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.

adrianffletcher avatar Feb 14 '20 21:02 adrianffletcher

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

dselman avatar Feb 14 '20 22:02 dselman

Sounds good.

adrianffletcher avatar Feb 14 '20 22:02 adrianffletcher

@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.

jeromesimeon avatar Apr 13 '21 22:04 jeromesimeon