Mathieu Lirzin <[email protected]> writes: > Taher Alkhateeb <[email protected]> writes: > >> - I still don't understand your answer on the new data structure >> "MultiValuedMapContext". This is essentially a whole new data >> structure designed by you and it overrides nearly all standard >> methods. Having such a custom and un-tested data structure might be >> worrying. Why aren't you going with a standard and tested solution? > > All the methods are overridden because they implements the > ‘MultivaluedMap’ interface. :-) > > ‘MultiValuedMapContext’ is a clone of ‘MapContext’ but adapted to > MultivaluedMap. The idea of ‘MapContext’ and ‘MultiValuedMapContext’ is > to provide a Map/MultivaluedMap view of a collection of configuration > files which are included from each other without any cycle (i.e. it's a > DAG of files). Does it help? I agree that there is room for > improvements in terms of documentation, refactoring, and tests. > >> What methods exactly do we need that requires that level of >> customization? Is it the getRequestMapMap and getRequestMultiMap? > > Mainly those two methods + the ‘MapStack’ which extends ‘MapContext’ and > is used at some other places. > >> Why not simply create two custom logic methods in a Util class to >> handle that instead of that mesh of object inheritance? > > This concerns design choices that were already made in ‘MapContext’ and > ‘MapStack’. That could be reworked but given the non-trivial amount of > work it represents this should be done outside of this review IMO.
For the record, I will start refactoring MapContext/MapStack in parallel of this review process. -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37
