Commands: Maybe result should be part of the Command-Object

[camunda]

Hi!

I am currently working a lot with the commands. I had one problem for a few times by now: How to return more than one object as a result?

For example, if I want to load the ProcessInstance but with the logs? The Logs have to be queried from the database in a seperate call, so I have the ProcessInstance and the Variables-Map as a result.

Because I don't like the idea to introduce some "DTO" again (which just holds ProcessInstance & Map) I thought about the possibility to include the results of a Command in the Command-Object and return the Command in the execute method (new signature then:

Command execute(Command command) throws Exception;

What do you think?

I think the overhead for returning the parameters again is not so much and the benefit is to really improve the power of the commands. And to KNOW what is the result of a command (at the moment it is a Object, that could be everything and is only checked at runtime at the client with a cast).

So: Opinions to that issue?

Regards
Bernd

[tom.baeyens]

the idea is very interesting.

some thoughts:

in case of remote communication, you get back an updated copy of the original command that you supplied as a parameter.

with this you introduce the overhead of introducing a typed member for the result in *all* the commands, while the map containing multiple results was only needed for a few.

when returning an object, it is up to the command what is returned. in any case, that has to be properly documented in the javadocs of the command. so the command could decide to use the proposed pattern and return itself as the result. by keeping object, we still can use the suggested approach, but we don't force it on all the commands.

despite all these cons-thoughts i still think it is an interesting idea and i don't want to ditch the idea just yet. but overall, i'm not yet convinced.

[camunda]

Hi Tom,

by keeping object, we still can use the suggested approach, but we don't force it on all the commands.

Thats right. But you also loose the typed interface (sorry, I like it very much if the compiler tells me errors, and not the ClassCastExceptions).

The problem with JavaDoc is, it is not garanteed to have correct informations there. You have no way to check that automatically. Because we are tied to Java 1.4, we can also not introduce some Annotation which specifies the return value, which is somehow also ok, I think.

with this you introduce the overhead of introducing a typed member for the result in *all* the commands, while the map containing multiple results was only needed for a few.

I think, this overhead is really small. Normally, I have only some longs or Strings in the Command as a parameter...

Maybe one other advantage could be, if you really have some asynchronous messaging architecture, where you just fire the command and get the result back later, there it woul dbe a real advantage to have all the parameters together with the result. But okay, in this scenario, you can build a easy workaround, I know....

But one argument at the moment is maybe, the Interface is already released with jbpm 3.2. Changing it now is maybe not the best idea.

I have the problem at the moment, how to return a ProcessInstance TOGETHER with all the logs, so I think, I will return the Command itself in the GetProcessInstanceCommand, ok? Or you prefer the Map?

[camunda]

I have one new comment on that:

Maybe it would be also a good option to always return a Map (or return the Command and have a Map always in there) to make the following approach possible:

I have written a EJB3-CommandServiceBean which has some additional informations about the processes and some Entities in the process variables. So it can load the required Entity-Objects (via JDO in that project). That could be handled in the CommandServiceBean, no problem, no overhead with multiple remote calls (jepp, we are in a Swing Client).

But how to return these additonal values?

change the result of the execute-method is not a good idea, because then, we have to now in the code which implementation of the CommandService we use, bad idea.

add the values to the process variables? sounds good at first, but has the problem, if I add them to the processInstance (for example) before returning, they are treated as process variables and persistet (which I do not want).

So if we have a Map as a return value, every CommandService-Implementation could add, what it wants to add. And the client can look for entries in the Map and use them, if he finds something. A concept like the "hints" for the PersistenceProvider in EJB 3...

What do you think?

I know, the most of the problems are not problems in the Webapp-World, but I think it is worse to support Remote-Apps too...

Thanks for any hints, ideas and comments! Unfortunatly, I am a little bit under time pressure for that issue :-(

Regards
Bernd

[camunda]

I thought a little bit about that idea, and find it better and better. With a map containing "implementation specific add-ons" in the result, we could also build Interceptors or something like that which for example adds the process-image or whatever to the result and the client can use the information if it is present and he wants to use it...

[tom.baeyens]

"camunda" wrote:
I have written a EJB3-CommandServiceBean which has some additional informations about the processes and some Entities in the process variables. So it can load the required Entity-Objects (via JDO in that project). That could be handled in the CommandServiceBean, no problem, no overhead with multiple remote calls (jepp, we are in a Swing Client).

But how to return these additonal values?

I didn't yet follow the scenario that you describe. Also, I didn't find in your scenario hints at why a plain Object is no good. IMO, if you return an Object in the interface, the command itself can decide to return a Map, the command itself or any other object.

I'm not saying that this the best approach. I'm giving you my reasoning so that you can explain where you see different. This also holds for the rest of this post (probably all of my posts:-)

"camunda" wrote:


change the result of the execute-method is not a good idea, because then, we have to now in the code which implementation of the CommandService we use, bad idea.

i think it is the Command that should determine the return value and document that in the javadocs. the command service implementation should not do anything with the return value.

it might be that some commands are not usable in certain command service implementations. E.g. when you implement a remote command service and the return value of the command is not serializable...

but in any case, it is the command alone that should determine the result, unrelated to the command service implementation

"camunda" wrote:


add the values to the process variables? sounds good at first, but has the problem, if I add them to the processInstance (for example) before returning, they are treated as process variables and persistet (which I do not want).

i don't get this. you only need to explain if it is still relevant.

"camunda" wrote:


So if we have a Map as a return value, every CommandService-Implementation could add, what it wants to add. And the client can look for entries in the Map and use them, if he finds something. A concept like the "hints" for the PersistenceProvider in EJB 3...

What do you think?

That could still be done with an object.

if (returnValue instanceof Map) ...

or

if (returnValue instanceof Hints) ...

"camunda" wrote:


I know, the most of the problems are not problems in the Webapp-World, but I think it is worse to support Remote-Apps too...

I want the command pattern to support both as good as possible. Let's keep this good discussion going.

"camunda" wrote:


Thanks for any hints, ideas and comments! Unfortunatly, I am a little bit under time pressure for that issue :-(

Regards
Bernd

feel free to contact me on skype. or send me your telephone number and we can have a discussion. i'm available for most of today and tomorrow.

[camunda]

I have found a workaround for my problem, so that we are not under a big time pressure any more...

Okay, returning a Map is also fine for all Use-Cases I think. If we really need the command in the result, we could build a CommandService which adds the Command to the Map ;-)

if (returnValue instanceof Map) ...

or

if (returnValue instanceof Hints) ...

I would prefer always return a Map and then

if (resultMap.containsKey("myEnhancement"))
 ....

But thats a little detail then, I think.

SO if we agree on that, I will change all the Get...Command's to return a Map (GetProcessDefinition, GetProcessInstance, GEeTaskInstance, ...), because for them it is always possible that we also need the Logs or additional entities from the server (and only want to call once).


Okay?

i think it is the Command that should determine the return value and document that in the javadocs. the command service implementation should not do anything with the return value.

I think, the CommandService should not change the return value, but adding additional information (to the Map) is feasable I think. And very handy for some problems. You can also think of Interceptors (Spring or EJB 3, technic does not matter) adding informations. A AddProcessDiagramInterceptor, AddDetailedProcessLogsInterceptor, ....

[tom.baeyens]

most important for the default out-of-the-box commands that we ship with jbpm is that they are simple, easy and predictable.

keep in mind that you don't have to design the commands to support all use cases. users can easily inherit from your commands to customize and add more return values.

there is always the tradeoff between covering more features and complexity.

try to find a good balance there.

the command return values can be changed but keep in mind the usability.

[camunda]

Okay, convinced. If we have a map as result it is not very nice to use.

So I will leave the result types as ProcessInstance and so on... The only use case, where this is a big problem is getting the Logs, so I will create a own Command for that (and we need the logs not to often, so this is ok, I think).

The good news are: No big change of the already checked in commands :-)