Quoting Ritesh Trivedi <[EMAIL PROTECTED]>:
Hi Ritesh, comments inline
Hi,
Have been trying to use configure feature and am encountering several
limitations (possible bug(s)/issues) with the current implementation.
Both items #1 and #2 causes a bug where if you modify ProductConfigWrapper
the cache instances still remains unchanged as both the methods returns deep
copy of the wrapper.
In the cache is stored ProductConfigWrapper instances which are not
configured/modified. If you check the cache key, you will see that
there is no indicator for selected options, so it is not possible to
make difference between configured options in the cache.
1. ProductConfigWrapper.getProductConfigWrapper() not sure why we need
to call copy constructor again when the configWrapper was just created
brand new just in the previous statement?
productConfigCache.put(cacheKey, new ProductConfigWrapper(configWrapper));
can just be
productConfigCache.put(cacheKey, configWrapper);
2. Again ProductConfigWrapper.getProductConfigWrapper() not sure why
we need to call copy constructor again when an instance from cache
already exists
configWrapper = new
ProductConfigWrapper((ProductConfigWrapper)productConfigCache.get(cacheKey));
can just be
configWrapper = (ProductConfigWrapper)productConfigCache.get(cacheKey);
Changes #1 and #2 will cause to addition to the cache an instance
which will be accessible and configured from the user who added it.
But then every next user using the same cache key (the same store,
catalog and currency) will get the already configured object with
selected options from the first user.
3. ConfigOption inner class, needs to expose either configItemId or
parent configItem - if lets say I store the selected configoptions in
the session and later time what to mark those items selected in
ProductConfigWrapper - both the setSelected() methods in
ProductConfigWrapper needs configItem information and not being able
to access it from the stored config options forces storage of
configItem objects too
I think it is ok to expose parent configItem.
4. Also on configOption isMandatory() should be exposed as isSelected
is exposed on configItem
If configItem is exposed (from point 3) isMandatory() can be called.
As options can't be mandatory, adding isMandatory() to configOption
could be confusing.
Bilgin
--
View this message in context:
http://www.nabble.com/ProductConfigWorker-and-ProductConfigWrapper-multiple-issues---Please-comment-tp19114133p19114133.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
