Hello,
see comment below.
2008/8/27, Emmanuel Lecharny [EMAIL PROTECTED]:
>
>
> public void setFilters(Map<String, ? extends IoFilter> filters) {
> if (filters == null) {
> throw new NullPointerException("filters");
> }
> if (!isOrderedMap(filters)) {
> ...
>
> @SuppressWarnings("unchecked")
> private boolean isOrderedMap(Map map) {
> Class<?> mapType = map.getClass();
> ...
> Random rand = new Random();
> List<String> expectedNames = new ArrayList<String>();
> IoFilter dummyFilter = new IoFilterAdapter();
> <------------------------------this is here.
> for (int i = 0; i < 65536; i ++) {
> // WTF is this loop ???
> String filterName;
> do {
> filterName = String.valueOf(rand.nextInt());
> } while (newMap.containsKey(filterName));
it generates a "unique" new key to be inserted in the new Map. Cleverly you
omitted the comment that exists in the code:
// Last resort: try to create a new instance and test if it maintains
// the insertion order.
you'll see why it is important to maintain the filter order that the user
intended to have - even if HE/SHE made a mistake by choosing the wrong
collection.
Here is my opinion: It's this small bits and pieces and checks that try to
cover _everything_ that make a framework worth using. If a framework just
provides some sloppy checking and will fail mysteriously (depending on the
filters, it could be quite difficult to fiddle out the mistake if the order
in the map wasn't maintained), you are maybe better of doing it all by
yourself. Just because something isn't understood doesn't mean it is a bad
thing.
christian
newMap.put(filterName, dummyFilter);
> expectedNames.add(filterName);
>
> Iterator<String> it = expectedNames.iterator();
> for (Object key: newMap.keySet()) {
> if (!it.next().equals(key)) {
> if (logger.isDebugEnabled()) {
> logger.debug(
> "The specified map didn't pass the insertion
> " +
> "order test after " + (i + 1) + " tries.");
> }
> return false;
> }
> }
> }
>
>
> Now, for what it worths, I'm really puzzled about this part of the code :
> what is this suppose to do ??? There is _no_ explanation, this is typically
> such a portion of code which makes it almost impossible to maintain. I call
> it a one man show.
>
> Plus I think it's totally useless. We can write defensive code, but at some
> point, we have to stop, and think. The setFilters( Map ) method is never
> called nowhere, and sounds like a YAGNI method to me. Why should we allow a
> user to create his own map, assume that the user can be malicious, and
> inject a cryptic method to check that the user didn't broke something in his
> own code ? To avoid complex questions on the user mailing list ?
>
> IMHO, it's way better to remove such a piece of code, and also remove the
> setFilters() method, as i'm pretty sure no one use it.
>
> KISS should always be a moto when writing code, unless necessary.
>
> --
> --
> cordialement, regards,
> Emmanuel Lécharny
> www.iktek.com
> directory.apache.org
>
>
>
