Re: svn commit: r882210 [1/2] - in /ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting: ./ invoice/ payment/ period/ tax/ test/ thirdparty/clearcommerce/ thirdparty/gosoftware/ thirdparty/valuelink/ util/

[Scott Gray]

On 20/11/2009, at 9:36 AM, Adam Heath wrote:

Scott Gray wrote:

On 20/11/2009, at 9:16 AM, Adam Heath wrote:

=
=
=
=
=
=
=
=
=
=
=
=
=
=================================================================

---


(original)
+++


Thu Nov 19 17:26:03 2009
@@ -1158,7 +1158,7 @@
      return serviceResult;
  }

-    public static Map<String, Object>
createInvoicesFromShipments(DispatchContext dctx, Map context) {
+    public static Map<String, Object>

context) {
      Delegator delegator = dctx.getDelegator();
      LocalDispatcher dispatcher = dctx.getDispatcher();
      List<String> shipmentIds =
UtilGenerics.checkList(context.get("shipmentIds"));

ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ invoice/InvoiceServices.java ofbiz/trunk/applications/accounting/src/org/ofbiz/accounting/ invoice/InvoiceServices.java createInvoicesFromShipments(DispatchContext dctx, Map<String, Object>

Nope, this is wrong.

Map<String, ? extends Object> context.

THIS IS A BUG.  PLEASE REVERT UNTIL IT IS FIXED.


are not allowed to change it *at all*.

There are already plenty of existing examples.


done to it.

The service engine *owns* the incoming context map. Called services When I did similiar things to framework, I discovered several services that modified the incoming context. I then had to change their code to deal with that problem. This patch needs to have the same thing

Hi Adam,


Object> for the context.

Could you explain a bit more? When I look at the StandardJavaEngine class the serviceInvoker method is specifically passing a Map<String,

have that generics markup.  However, the things it calls *must* keep
it read-only, so that when several services in a chain are

in the set.

serviceInvoker adds/removes things to this context, yes, so it has to called(think secas, service groups), once service can't affect another

Thanks for the explanation, I won't pretend that I understand how using the same markup as the invoking method modifies the context though :-) I'll just flag this piece of generics as something I need to study up on.

==
Map<String, Object> context:

This means that the type of the values stored in the context need to
be of type Object.  Since all objects are this way, anything can be
stored into the context.

Map<String, ? extends Object> context:

This means that the *concrete* type of the value is *some unknown*
class, that extends Object.  Anyone can pull anything out of this
context, as it *will* extend Object.  However, since the actual type
of the values is not known, no one can put anything in, as it might
cause a ClassCastException somewhere.

==

The former can be cast to the latter, but not vice versa.

Plus, it's rather good form, to consider *all* incoming parameters on
*any* method(not just service implementations) to be read-only, and
only return results thru the method's return value.  This is called
having no side-effects.

Also look at this:
http://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html

Thanks
Scott

Ah okay I see, so using Object doesn't modify the context but it does allow the context to be modified whereas ? will block any attempts to use put. BTW, <String, ? extends Object> is the same as using <String, ?> isn't it?

smime.p7s
Description: S/MIME cryptographic signature