Talk:Thinking in States
From Programmer 97-things
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
In the version at 2010-01-13, Niclas Nilsson wrote this: > You may still want isComplete for clarity in the code, but then it should look like this: "public boolean isComplete() { return hasShipped(); }"
Even though I (Tomas) do not have a very clear idea of why/when the client code needs to check for a "isComplete" concept, I still have a feeling that the reason would belong to another abstraction level which is composed of the state of the "order events" (i.e. "In progress", "Paid" and "Shipped") which have occurred so far according to the current requirements.
According to the text in the article, the current requirements are that "an order can't be shipped before it's paid", but I would actually like to claim that it is very common with shipping before payment, and therefore it is a very likely future changed requirement, and when that requirement occurrs you will only need to change that "isComplete()" implementation in one place (i.e. use "return isPaid() && hasShipped();" as in the original code which is considered redundant according to the current requirements).
On the other hand, if you do not at all define any method "isComplete()", but rather (everywhere where it otherwise would have been be used) only uses "hasShipped()" method, then it may become more difficult (with potential for introducing bugs) to decide which "hasShipped()" invocations that needs to be changed into "isPaid() && hasShipped()" (i.e. "isComplete()") and which of them that still will only use "hasShipped()" since in some places in the code, the shipping state might still be the only thing of interest rather than the "isComplete()" concept (whatever that is). I do not think it is speculative programming to think like that, since it is so reasonable to assume that paying in advance will not be a requirement forever, i.e. IMHO the YAGNI comment should not be applied here.
In other words, the thing I suggest here, is to not simply write that you _may_ still want "isComplete()" for clarity ... but rather that you _should_ want to use such a method, since it seems to be another kind of abstraction level with implementation logic that uses the status of the order events for deriving the value.
A crucial point of the example in the article, is to understand what the "isComplete()" method will be used for, i.e. what it is really supposed to be used for, i.e. since the method exists, I would assume the code to contain some statement like this:
if(order.isComplete()) {
// do something, but what ?
}
else {
// maybe do something else, but what ?
}
Whatever the if statement will do here, the question is if it would have to change if the requirement change, i.e. if the current requirement is that advance payment is necessary but would change into allowing payment after shipping, then would it be okay to change the "isComplete" implementation according to the new requirement while the above client code could remain unchanged ? Or would the if statement above actually want to use "hasShipped()" instead, i.e. if the implementation of "isComplete()" would change from "hasShipped()" to "isPaid() && hasShipped()", then maybe the "if(order.isComplete()) {" no longer can be used, but rather would actually have to be changed into directly checking the shipping state, i.e. change to "if(order.hasShipped()) {" ?
So, what I am trying to say here is that if the client code never really needs to consider some "isComplete()" method since that method does not represent any existing concept except being a synonymous of "hasShipped()", then it should certainly _NOT_ be kept "for clarity in the code" since different names for the same thing is the opposite of clarity. However, if the opposite is true, i.e. if "isComplete()" represents something that the client code will keep using in the same way, regardless of whether the current requirement is "isPaid() && hasShipped()" or "hasShipped()" or something else, then it indeed _should_ be encouraged to define such a method rather than only saying that "You _may_ still want isComplete for clarity".
The article also writes that:
> "many programmers are quite vague about state too - and that is a problem."
I would like to claim that the example itself is not clear neither, regarding state. It uses the following four kind of states/statuses/events (but IMHO, the distinction between states and events is vague):
"In progress" "Paid" "Shipped" "Complete"
It is explicitly written in the article that the first three above are "distinct states", which I think is intended to imply that these are three statuses wihch can not be combined at the same time i.e. the "state" can not be considered as "Paid" and "Shipped" at the same time, because of the implicit assumption that the order should automatically be considered as "Paid" when it is "Shipped" according to the current requirement. I do not think it is appropriate to define/model the states in such a way that one value automatically implies another status value. (and now I mean within the same kind of status concept ! i.e. it is indeed reasonable to let other kind of statuses derive its status by using other status concepts) I think it would be a better approach to model some of these concepts as Domain Events, i.e. you register the events when they occur, e.g. when the order is paid and when the order is shipped. The "In progress" thing might be registered as a Domain Event representing "Order created". Then when you want to figure out the status of the order, you can apply logic to these registered events, e.g. if you want to know if the order is in status "In progress", this status might be defined as not being complete, while the complete status might be defined as having been created, paid and shipped, i.e. something like this:
public boolean isInProgress() { / but it can indeed be questioned if this method should exist and "pollute" (someone might think) the API or if the client code should rather directly use "!isComplete()" instead
return !isComplete();
}
public boolean isComplete() { // returns an order status, implemented by applying logic of some domain events
return
isOrderCreated() // returns true if domain event "Order created" has occurred (but can be discussed if an Order instance can exist without the order having been created, and then we need to define the semantic of "created" in the context of the application)
&& isShipped() // returns true if domain event "Shipped" has occurred
&& isPaid(); // returns true if domain event "Paid" has occurred
}
Of course, it can also be discussed how the retrieval of the above domain event values should be implemented, and in the above example I have for simplicity only used some helper methods for the readability to illustrate the point. Regarding the actual implementation I think it is offtopic for this discussion about how to think about state and not being vague about the state, but the DDD sample application provides some example code with a "HandlingEventRepository" being able to retrieve a "HandlingHistory" with a List of "HandlingEvent". ( http://dddsample.sourceforge.net/characterization.html#Domain_Event )
Another way to think about the above events is that there can be three or four kind of statuses:
Created/Not Created (But this might not make sense, and we would need to define what "Created" means in the application context, i.e. if we imply created=persisted or what else we might mean...) Shipped/Unshipped (Shipped=true/fase, but of course the shipping might have started but not finished, so boolean values are a simplification here) Paid/Unpaid (Paid=true/false, but of course it might be partially paid, so these boolean values are also a simplification) Complete/Uncomplete (Complete=true/false, where Uncomplete is synonymous with the above used "In progress")
The last above "Complete" status can be derived by the other two or three, e.g. Complete might be true if and only if the shipping and the payment are 100% complete (i.e. no halfway shipping or partial payment acceptable) (in this context, I use the word "status" in the sense that only exactly one of some enumeration of values can be true at the same time for a status, and in this case these enumeration of values are simplified with boolean values)
/ Tomas Johansson
Sorry for a late answer, but Gmail decided to put the notification in the spam folder. Thank you for making me aware of it in another way.
All in all, I agree with with most of the things you are saying. Each contribution did however have a max word count close to three times less than you used in the comment, so the example really had to be terse. That was the main reason for the constraint Consider a simple webshop that only accepts credit cards and does not invoice customers, but maybe I should have used another example completely. Sorry for that.
When it comes to changing requirements in exactly this type of domain, there actually is a metamodel that covers a very big variations when it comes to orders of payments and delivery, payments in parts, shipping in parts, currencies and many other variations, and that's the REA model (Resources, Events and Agents). I warmly recommend Pavel Hruby's book (http://www.phruby.com/book/index.html) on this topic.
However, that was not what I aimed to explain at all. My goal wasn't to explain the design of the generic model that would cater for future requirements without change to the model (which the REA model actually more or less gives you for free), but rather to point out that most code out there have a very liberal view on state (read: holes everywhere), which I consider to be something very simple (in theory) that is too often plainly ignored.
Thank you very much for your comment Tomas.
Kind regards Niclas Nilsson
