tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1642307 - in /tomcat/trunk: java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java test/org/apache/tomcat/websocket/TestCaseInsensitiveKeyMap.java
Date Fri, 28 Nov 2014 17:47:49 GMT
2014-11-28 17:53 GMT+03:00  <markt@apache.org>:
> Author: markt
> Date: Fri Nov 28 14:53:15 2014
> New Revision: 1642307
>
> URL: http://svn.apache.org/r1642307
> Log:
> Add a map that supports case insensitive keys. E.g. for use with HTTP headers.
>
> Added:
>     tomcat/trunk/java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java   (with
props)
>     tomcat/trunk/test/org/apache/tomcat/websocket/TestCaseInsensitiveKeyMap.java   (with
props)
>
> Added: tomcat/trunk/java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java?rev=1642307&view=auto
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java (added)
> +++ tomcat/trunk/java/org/apache/tomcat/websocket/CaseInsensitiveKeyMap.java Fri Nov
28 14:53:15 2014
> @@ -0,0 +1,206 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements.  See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License.  You may obtain a copy of the License at
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.tomcat.websocket;
> +
> +import java.util.AbstractMap;
> +import java.util.AbstractSet;
> +import java.util.HashMap;
> +import java.util.Iterator;
> +import java.util.Locale;
> +import java.util.Map;
> +import java.util.Set;
> +
> +/**
> + * A Map implementation that uses case-insensitive (using {@link Locale#ENGLISH}

Missing ")"

> + * strings as keys.
> + * <p>
> + * This implementation is not thread-safe.
> + *
> + * @param <V> Type of values placed in this Map.
> + */
> +public class CaseInsensitiveKeyMap<V> extends AbstractMap<String,V> {

(...)

> +    private static class Key {
> +
> +        private final String key;
> +
> +        private Key(String key) {
> +            this.key = key;
> +        }
> +
> +        public String getKey() {
> +            return key;
> +        }
> +
> +        @Override
> +        public int hashCode() {
> +            final int prime = 31;
> +            int result = 1;
> +            result = prime * result +
> +                    ((key == null) ? 0 : key.toLowerCase(Locale.ENGLISH).hashCode());
> +            return result;

"key" field cannot be null, because factory method returns null
without creating a Key in that case.

Cache lowercase string in some field and call its hashCode() method?

The Key object is used to store values or to lookup values. The
hashCode() method will be called at least once so it makes sense to
cache the lowercase value.

> +        }
> +
> +        @Override
> +        public boolean equals(Object obj) {
> +            if (this == obj) {
> +                return true;
> +            }
> +            if (obj == null) {
> +                return false;
> +            }
> +            if (getClass() != obj.getClass()) {
> +                return false;
> +            }
> +            Key other = (Key) obj;

key cannot be null, as I mentioned above

> +            if (key == null) {
> +                if (other.key != null) {
> +                    return false;
> +                }
> +            } else if (!key.toLowerCase(Locale.ENGLISH).equals(
> +                    other.key.toLowerCase(Locale.ENGLISH))) {

A waste to convert and throw away. Especially if string lengths do not match.

If lowercase string is cached, the implementation becomes more simple.

> +                return false;
> +            }
> +            return true;
> +        }
> +
> +        public static Key getInstance(Object o) {
> +            if (o instanceof String) {

o is never null inside this if() block.

> +                return new Key((String) o);
> +            }
> +            return null;
> +        }
> +    }
> +}

BTW,  how the class handles null and non-string keys?

I think put() should throw an exception,  and get() and containsKey()
should return null and false respectively.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message