jcoplien / trygve

The trygve language project - Building a DCI-centric language from the ground up
GNU General Public License v2.0
103 stars 15 forks source link

Bug in role-object contract? #35

Closed mbrowne closed 8 years ago

mbrowne commented 8 years ago

I started playing with the a mock Currency object for the amount role and it almost compiles except for these errors:

line 30: Method decreaseBalance' not declared in RoleSourceAccount'. line 39: Method increaseBalance' not declared in RoleDestinationAccount'.

Am I missing something here or is this a bug?

context TransferMoney {
    public TransferMoney(Account source, Account destination, Currency amount) {
        SourceAccount = source;
        DestinationAccount = destination;
        Amount = amount;
        Bank = this;
    }

    stageprop Amount {} requires {
        double value() const;
    }

    // Execute the use case
    public void run() {
        Bank.transfer();
    }

    stageprop Bank {
        public void transfer() {
            SourceAccount.withdraw();
            DestinationAccount.deposit();
        }
    }

    role SourceAccount {    
        public void withdraw() {
            if ( Amount.value() > this.getBalance().value() ) {
                assert(false, "Insufficient funds");
            }
            this.decreaseBalance(Amount);
        }
    } requires {
        void decreaseBalance(Currency amt);
        Currency getBalance();
    }

    role DestinationAccount {
        public void deposit() {
            this.increaseBalance(Amount);
        }
    } requires {
        void increaseBalance(Currency amt)
    }
}

class Account {
    private Currency balance_;

    public Account() {
       balance_ = new Currency(0.0);
    }

    void increaseBalance(Currency amount) {
        this.balance_ = new Currency(this.balance_.value() + amount.value());
    }

    void decreaseBalance(Currency amount) {
        this.balance_ = new Currency(this.balance_.value() - amount.value());
    }

    Currency getBalance() {
        return this.balance_;
    }
}

class Currency {
    private double value_;
    public Currency(double val) {
        value_ = val;
    }
    public double value() const {
        return value_;
    }
}

{
    Account src = new Account();
    Account dst = new Account();
    TransferMoney transfer = new TransferMoney(src, dst, new Currency(10.0));
    transfer.run()
}
rvalimaki commented 8 years ago
role DestinationAccount {
    public void deposit() {
        this.increaseBalance(Amount);
    }
} requires {
    void increaseBalance(Currency amt)
}

Probably a missing semicolon after contract definition ("requires")?

mbrowne commented 8 years ago

Nope, that wasn't it (apparently semicolons are optional there). I suspect it's probably because the contract is not correctly recognizing the type of 'Amount' as a role-player as opposed to the Currency object prior to role-binding. It works if you add a myAmount Currency property to the Context and change this.decreaseBalance(Amount) to this.decreaseBalance(myAmount).

mbrowne commented 8 years ago

(Note that I just corrected an important typo in the above.)

jcoplien commented 8 years ago

What is “Amount”?

Den 07/01/2016 kl. 14.40 skrev rvalimaki notifications@github.com:

role DestinationAccount { public void deposit() { this.increaseBalance(Amount); } } requires { void increaseBalance(Currency amt) } Probably a missing semicolon after contract definition ("requires")?

— Reply to this email directly or view it on GitHub https://github.com/jcoplien/trygve/issues/35#issuecomment-169666762.

jcoplien commented 8 years ago

The code works as it should. Why should the program be able to look ahead?

But I think there is a more fundamental error here, and it is a design error. Can you share the original use case?

Den 07/01/2016 kl. 14.45 skrev Matt Browne notifications@github.com:

Nope, that wasn't it (apparently semicolons are optional there). I suspect it's probably because the contract is not correctly recognizing the type of 'Amount' as a role-player as opposed to the Currency object prior to role-binding. It works if you add a myAmount Currency property to the Context and change this.decreaseBalance(Amount) to this.decreaseBalance(this.myAmount).

mbrowne commented 8 years ago

I was intending to follow the use case of our running example, and also used in the Lean Architecture book, although this is a simplified version. Obviously a real Currency class would look very different. Also, I am thinking that the contract for the Currency object should be a compareTo method rather than the value method and that there's no need to call value() directly from within the TransferMoney context. But aside from whatever design errors there may be, I don't understand what you mean by, "Why should the program be able to look ahead?". I thought maybe you were referring to the fact that the classes are declared after the context, but that doesn't seem to make a difference.

mbrowne commented 8 years ago

Also, note that this is not my final design. I was just keeping it as similar as possible at first to the version with double for the amount to get it to compile that way before changing it more. While this is still nothing like a real Currency class, I was headed for something like this:

class Currency {
    private double value_;
    public Currency(double val) {
        value_ = val;
    }
    double value() {
        return value_;
    }
    Currency add(double amount) {
        return new Currency(value_ + amount);
    }
    Currency subtract(double amount) {
        return new Currency(value_ - amount);
    }
    //TODO
    //compareTo()
}
mbrowne commented 8 years ago

Thinking about this more, I think I see what you mean now about "looking ahead". Amount could be played by something other than a Currency object (as long as it satisfies the contract), and the type checks happen at compile time, not run-time. Would there perhaps be some way of indicating that Amount will always satisfy a certain interface, so that that interface could be used for type hints in the Account class (or Context)?

jcoplien commented 8 years ago

Den 07/01/2016 kl. 15.57 skrev Matt Browne notifications@github.com:

Thinking about this more, I think I see what you mean now about "looking ahead". Amount could be played by something other than a Currency object (as long as it satisfies the contract), and the type checks happen at compile time, not run-time.

I think you’ve come to the right answer, albeit apparently via a kind of circuitous route, based more on the implementation and on what the compiler might be able to do than on DCI design thinking.

Would there perhaps be some way of indicating that Amount will always satisfy a certain interface, so that that interface could be used for type hints in the Account class (or Context)?

You can specify the interface that Amount presents to the other Roles in the Context, in the interface of the Amount Role declaration.

You can specify what interface is required by Amount Role-players in the requires section of the Amount declaration.

It seems like what you are asking for is to provide information to the clients of Amount about the class interface of its Role-players. That’s none of their business. It violates the encapsulation of the Role. It is an encapsulation leak that violates the abstraction boundary in Trygve’s diagram, by making information from the class world available at the interaction level:

So in the code:

role SourceAccount {    
    public void withdraw() {
        if ( Amount.value() > this.getBalance().value() ) {
            assert(false, "Insufficient funds");
        }
        this.decreaseBalance(Amount);  // <== line 30
    }
} requires {
    void decreaseBalance(Currency amt);
    Currency getBalance();
}

you say at line 30 that you are interacting with an object of another Role named Amount. But then in the declaration of the interface you stipulate that (by implication) something that is an Amount Role-player is also of class Currency. You can probably make the argument that an object of class Currency will be playing the Amount Role. It is of course true that any given Role will be played by an object and that the object likely has a class — sure, so why not Currency? It’s as good a class as any other.. And you hope that you know which class and that it will be Currency. But it is inappropriate, within a Role, either to stipulate or assume which class gave rise to the Role-player; to do so limits genericity (something I’m focusing a lot on in the new user guide). However, your Role does. You could equally well have written Rectangle instead of Currency and tried to persuade me that I (and the compiler) should take your word that all had been arranged elsewhere in the program for those two types to be aligned. You certainly hope that it will. And you believe it will. The place to localize and express that information is inside Amount, as described above — to put it here in SourceAccount is an inappropriate separation of concerns. Hope is not a plan, and belief is not a guarantee.

This isn’t a trygve llimitation: this is a DCI staple.

The looking-ahead you’re asking for would have to be magic. I know that any sufficiently distinguishable technology is indistinguishable, but... I hate to bring up theory here since people have been very vocal here about their theory-averseness, but you’d have to solve the halting problem to be able, in general, to validate what your code apparently intended to establish. You’re asking the compiler to solve a provably undecidable problem. If you don’t understand the theory you may not be able to understand why this construct is impossible to compile properly.

I can envision some DCI implementations that would allow this — basically by ignoring the limitations or not giving them voice in the first place. For example, Trygve’s squeak implementation would very likely allow it. The price you pay is the possibility that the methods on Amount wouldn't meet the contract of Currency at run-time, so the enactment at line 30 would cause decreaseBalance to die in a message-not-understood error. I want to make those impossible. And I can — without violating any of the design principles we hold for DCI.

You mentioned yesterday that you had built several environment-kinds of things, and it makes me wondered how you handled situations like this in those environments.

The reason I asked to see your use case was to see if the actor SourceAccount incorporated any knowledge of a Currency actor. This implementation suggests that it does. I wish you would have written the use case here in mail. I asked for it as part of one step of leading you to an understanding of the design mistake you had made. I know that in general your use case smells like the one in the book. And we all think we have it in our heads. We don’t — not in detail. Mies van der Rohe reminded us that “God is in the details.” So is the devil. Details are important — for example, whether the use case makes a reference to Currency, and exactly how it does so. In the future I think it would help your designs — even your simple ones — to have a back-of-the-envelope domain analysis and a fairly well-thought-out use case.

But there is even a more basic design puzzle that is unrelated to DCI. One does not reduce a balance by some currency: one decreases a balance by an amount (at least this is true if the ubiquitous language comprises both). A currency may be a property of an amount, or both an amount and currency may be properties of some other idealized Role in some alternative mental model. But I can’t envision any mental model in which one reduces a balance by a Currency. I think your design is maybe schizophrenic in that it is trying to build the same abstraction boundary both with the Currency class and the Amount Role. But there is not enough information here to tell, and a more complete articulation of both (in the analysis) would tell. So I have only wishy-washy objections to it.

I have a hard time understanding even what the basic Chen diagram would look like for the value you return from this.getBalance().value()and its relationship to Amount and Currency. There seems to be a leveling error in the domain analysis but it’s confusing enough that I can only guess. Is a this.getBalance().value()substitutable for a Currency? If not, why not?

These are DCI basics. One thing I am encouraged about is that the restrictions in trygve’s semantics enforce them and (now) make them clear in ways that other environments may overlook because of leaky abstractions. So in fact I think that your example upset trygve shows a strength in its design. I apologize that the compiler was less than polite and sophisticated in its original abrupt termination in light of this error. The environment has unfortunately been designed largely to work properly with proper code. The state space of improper code is unbelievably large and it’s difficult to foresee all the ways in which people will circumvent the paradigm. In your case, the problem has been fixed and I think the current error message should be enough. That’s why I appreciate the kinds of examples you are working on; they help explore this error space.

The new trygve version has been pushed to GitHub.

So in summary to Issue 35: just a bug in the error stumbling logic, and that’s been fixed. And it’s more than a coding error: the code just reflects a design error that might have been caught with good modeling. It starts with clear thinking up front and a proper separation of Roles and classes: what the system is and does, per the Lean Architecture book.. I think my favourite quote of the past 365 days is:

Den 05/01/2016 kl. 02.56 skrev Hai Quang Kim wangvnn@gmail.com:

And I often spend more time for up front design with DCI thinking nowadays.

When I ask DCI programmers for their use cases, it’s probably a good sign if they can regurgitate them immediately, concretely, and directly.

jcoplien commented 8 years ago

Issue: Closed. The example now goes through the compilation process to completion with a suitable set of error messages.

jcoplien commented 8 years ago

Making the context readable is one thing and making a context working for all cases in future is another thing. In reality, to be agile, I just use double as the Amount. Change to currency later when we introduce Currency.

Code reuse is not cheap and easy. To do that properly we need all the use cases including the all the future ones. The good thing is if the code is readable then it is changeable without too much trouble.

I found that DCI code is very easy to be changed for this kind of situation. I refactor the code heavily but the form of the algo still in place, just replace RolePlayer/Role's method/interface.... And It still works.

/quang

On Friday, January 8, 2016 at 3:46:07 AM UTC+8, cope wrote:

Den 07/01/2016 kl. 15.57 skrev Matt Browne <notifi...@github.com

>: Thinking about this more, I think I see what you mean now about "looking ahead". Amount could be played by something other than a Currency object (as long as it satisfies the contract), and the type checks happen at compile time, not run-time. I think you’ve come to the right answer, albeit apparently via a kind of circuitous route, based more on the implementation and on what the compiler might be able to do than on DCI design thinking. Would there perhaps be some way of indicating that Amount will always satisfy a certain interface, so that that interface could be used for type hints in the Account class (or Context)? You can specify the interface that Amount presents to the other Roles in the Context, in the interface of the Amount Role declaration. You can specify what interface is required by Amount Role-players in the requires section of the Amount declaration. It seems like what you are asking for is to provide information to the clients of Amount about the class interface of its Role-players. That’s none of their business. It violates the encapsulation of the Role. It is an encapsulation leak that violates the abstraction boundary in Trygve’s diagram, by making information from the class world available at the interaction level: So in the code: ``` role SourceAccount { public void withdraw() { if ( Amount.value() > this.getBalance().value() ) { assert(false, "Insufficient funds"); } this.decreaseBalance(Amount); // <== line 30 } } requires { void decreaseBalance(Currency amt); Currency getBalance(); } ``` you say at line 30 that you are interacting with an object of another Role named Amount. But then in the declaration of the interface you stipulate that (by implication) something that is an Amount Role-player is _also_ of class Currency. You can probably make the argument that an object of class Currency _will_ be playing the Amount Role. It is of course true that any given Role will be played by an object and that the object likely has a class — sure, so why not Currency? It’s as good a class as any other.. And you _hope_ that you know which class and that it will be Currency. But it is inappropriate, within a Role, either to stipulate or assume _which_ class gave rise to the Role-player; to do so limits genericity (something I’m focusing a lot on in the new user guide). However, your Role does. You could equally well have written Rectangle instead of Currency and tried to persuade me that I (and the compiler) should take your word that all had been arranged elsewhere in the program for those two types to be aligned. You certainly hope that it will. And you believe it will. The place to localize and express that information is inside Amount, as described above — to put it here in SourceAccount is an inappropriate separation of concerns. Hope is not a plan, and belief is not a guarantee. This isn’t a _trygve_ llimitation: this is a DCI staple. The looking-ahead you’re asking for would have to be magic. I know that any sufficiently distinguishable technology is indistinguishable, but... I hate to bring up theory here since people have been very vocal here about their theory-averseness, but you’d have to solve the halting problem to be able, in general, to validate what your code apparently intended to establish. You’re asking the compiler to solve a provably undecidable problem. If you don’t understand the theory you may not be able to understand why this construct is impossible to compile properly. I can envision some DCI implementations that _would_ allow this — basically by ignoring the limitations or not giving them voice in the first place. For example, Trygve’s squeak implementation would very likely allow it. The price you pay is the possibility that the methods on Amount wouldn't meet the contract of Currency at run-time, so the enactment at line 30 would cause decreaseBalance to die in a *message-not-understood *error. I want to make those impossible. And I can — without violating any of the design principles we hold for DCI. You mentioned yesterday that you had built several environment-kinds of things, and it makes me wondered how you handled situations like this in those environments. The reason I asked to see your use case was to see if the actor SourceAccount incorporated any knowledge of a Currency actor. This implementation suggests that it does. I wish you would have written the use case here in mail. I asked for it as part of one step of leading you to an understanding of the design mistake you had made. I know that in general your use case smells like the one in the book. And we all think we have it in our heads. We don’t — not in detail. Mies van der Rohe reminded us that “God is in the details.” So is the devil. Details are important — for example, whether the use case makes a reference to Currency, and exactly how it does so. In the future I think it would help your designs — even your simple ones — to have a back-of-the-envelope domain analysis and a fairly well-thought-out use case. But there is even a more basic design puzzle that is unrelated to DCI. One does not reduce a balance by some currency: one decreases a balance by an amount (at least this is true if the ubiquitous language comprises both). A currency may be a property of an amount, or both an amount and currency may be properties of some other idealized Role in some alternative mental model. But I can’t envision any mental model in which one reduces a balance by a Currency. I think your design is maybe schizophrenic in that it is trying to build the same abstraction boundary both with the Currency class and the Amount Role. But there is not enough information here to tell, and a more complete articulation of both (in the _analysis_) would tell. So I have only wishy-washy objections to it. I have a hard time understanding even what the basic Chen diagram would look like for the value you return from this.getBalance().value()and its relationship to Amount and Currency. There seems to be a leveling error in the domain analysis but it’s confusing enough that I can only guess. Is a this.getBalance().value()substitutable for a Currency? If not, why not? These are DCI basics. One thing I am encouraged about is that the restrictions in _trygve’s_ semantics enforce them and (now) make them clear in ways that other environments may overlook because of leaky abstractions. So in fact I think that your example upset _trygve_ shows a strength in its design. I apologize that the compiler was less than polite and sophisticated in its original abrupt termination in light of this error. The environment has unfortunately been designed largely to work properly with proper code. The state space of improper code is unbelievably large and it’s difficult to foresee all the ways in which people will circumvent the paradigm. In your case, the problem has been fixed and I think the current error message should be enough. That’s why I appreciate the kinds of examples you are working on; they help explore this error space. The new _trygve_ version has been pushed to GitHub. So in summary to Issue 35: just a bug in the error stumbling logic, and that’s been fixed. And it’s more than a coding error: the code just reflects a design error that might have been caught with good modeling. It starts with clear thinking up front and a proper separation of Roles and classes: what the system _is_ and _does_, per the Lean Architecture book.. I think my favourite quote of the past 365 days is: Den 05/01/2016 kl. 02.56 skrev Hai Quang Kim >: And I often spend more time for up front design with DCI thinking nowadays. When I ask DCI programmers for their use cases, it’s probably a good sign if they can regurgitate them immediately, concretely, and directly.
jcoplien commented 8 years ago

Den 08/01/2016 kl. 02.58 skrev Matt Browne notifications@github.com:

I'll may respond to your messages later if I have additional questions, but for now...what is is that you changed? When I run the example I'm still getting the same messages as before.

The error message displays the type of the requested actual parameter, with the implication that no method of a matching signature (rather than just the method selector) can be found. It’s a broad hint to look for a mismatch in the signature.