#5 ConfigOption.equals method is totally incorrect !! - just checking based on selected field? there can be multiple options selected. At a very least, configOption id and configItem id needs to be compared too.
Ritesh Trivedi wrote: > > 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. > > 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); > > 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 > > 4. Also on configOption isMandatory() should be exposed as isSelected > is exposed on configItem > > -- View this message in context: http://www.nabble.com/ProductConfigWorker-and-ProductConfigWrapper-multiple-issues---Please-comment-tp19114133p19114268.html Sent from the OFBiz - Dev mailing list archive at Nabble.com.
