Hi Max,

> The same variable name should ALWAYS refer to the same variable / value.

I think, I can second this.

> For setters and constructors this makes sense -> after all, in each of
> these you have a simple assignment, and both variables will carry the
> same value.

In my attachment Tag.java, you can see a variable named "value" in the
constuctor and as field. According the rule, the variable in the
constructor hides the field. But its really the same thing. I even
assign it in the last line of the constuctor. 

Another example is the method getEntriesInOffsetOrder() in the attached
file OffsetTable.java. It is just a getter of entries but it is named
different.

> But in most other methods, the parameter you pass is NOT assigned to the
> internal variable, so they actually refer to a different thing, and
> thats where the confusion starts.

You are absolutely right. In most cases the variable really refers to a
different thing. The above two examples are the only two cases where I
violated the HiddenFieldRule in 155 new classes.

> I know modern IDEs can show you which variable you actually refer to,
> but this usually requires selecting the variable or hovering over it,
> which you will not do if you are just reading the code trying to
> understand it.

Not in Intellij Idea. There fields are bold and dark magenta and all
other variables are just normal and black.

> However, since we cannot agree to keep the rule, I'll have to be content
> with removing it (which is already done).

I think this rule ist mostly helpful in order to think about variable
names. But I also think that here are a few cases where violating this
rule makes sense. So maybe the rule ist just not smart enough to detect
the remaining special cases.

Thats the same as with the "Javadoc on public things rule". If there
isn't anything to say about a public thing which will say more than the
name itself, than I prefer no comment at all. But how should Checkstyle
detect such cases? 

There is a @SuppressWarnings annotation. I don't know if Checkstyle uses
it. So maybe if we switch to Java 1.5, we could use it. But even than
this annotation is a lot of clutter. It's a pity that computers can't
think.


Best Regards
Alex
/*
 * 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.
 */

/* $Id */

package org.apache.fop.fonts.opentype.common;

import java.io.UnsupportedEncodingException;

/**
 * Array of four uint8s (length = 32 bits) used to identify a script, language system, feature, or
 * baseline.
 * <p/>
 * Tags are the names given to tables in the OpenType font file. All tag names consist of four
 * characters. Names with less than four letters are allowed if followed by the necessary trailing
 * spaces. All tag names defined within a font (e.g., table names, feature tags, language tags) must
 * be built from printing characters represented by ASCII values 32-126.
 */
public final class Tag {

    public static final Tag TTCF = Tag.valueOf("ttcf");
    public static final Tag OTTO = Tag.valueOf("OTTO");
    public static final Tag TRUE = Tag.valueOf("true");
    public static final Tag TYP1 = Tag.valueOf("typ1");

    private static final int MIN_BYTE_VALUE = 0x20;
    private static final int MAX_BYTE_VALUE = 0x7E;

    private final int value;

    public static Tag valueOf(String str) {
        int length = str.length();
        if (length < 4) {
            throw new IllegalArgumentException("str.length() < 4; was: " + length);
        }
        try {
            return new Tag(str.getBytes("ISO-8859-1"));
        } catch (UnsupportedEncodingException e) {
            //TODO: not the best solution
            throw new InternalError(e.getMessage());
        }
    }

    Tag(byte[] bytes) {
        if (bytes.length < 4) {
            throw new IllegalArgumentException("bytes.length < 4; was: " + bytes.length);
        }
        int value = 0;
        for (int i = 0; i < 4; i++) {
            checkByte(bytes[i], i);
            value += (bytes[i] << ((3 - i) * 8));
        }
        this.value = value;
    }

    public boolean isAllUpperCase() {
        String tagStr = toString();
        for (int i = 0; i < tagStr.length(); i++) {
            char ch = tagStr.charAt(i);
            if (!Character.isUpperCase(ch) && !Character.isDigit(ch) && !isSpace(ch)) {
                return false;
            }
        }
        return true;
    }

    public boolean isAllLowerCase() {
        String tagStr = toString();
        for (int i = 0; i < tagStr.length(); i++) {
            char ch = tagStr.charAt(i);
            if (!Character.isLowerCase(ch) && !Character.isDigit(ch) && !isSpace(ch)) {
                return false;
            }
        }
        return true;
    }

    public boolean equals(Object o) {
        if (this == o) {
            return true;
        }

        if (o == null || getClass() != o.getClass()) {
            return false;
        }

        Tag tag = (Tag) o;

        return value == tag.value;
    }

    public int hashCode() {
        return value;
    }

    public String toString() {
        return new String(toChars());
    }

    private static void checkByte(int value, int position) {
        if (value < MIN_BYTE_VALUE || value > MAX_BYTE_VALUE) {
            throw new IllegalArgumentException("byte " + position + " out of range ["
                    + MIN_BYTE_VALUE + ", " + MAX_BYTE_VALUE + "] inclusive; was " + value);
        }
    }

    private char[] toChars() {
        char[] chars = new char[4];
        chars[0] = (char) (value >> 24);
        chars[1] = (char) ((value >> 16) & 0xFF);
        chars[2] = (char) ((value >> 8) & 0xFF);
        chars[3] = (char) (value & 0xFF);
        return chars;
    }

    private boolean isSpace(char ch) {
        return ch == 0x20;
    }
}
/*
 * 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.
 */

/* $Id */

package org.apache.fop.fonts.opentype.file;

import org.apache.fop.fonts.opentype.common.Tag;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
 *
 */
final class OffsetTable {

    private final Map entries;

    OffsetTable(Entry[] entries) {
        this.entries = new HashMap(entries.length);
        for (int i = 0; i < entries.length; i++) {
            Entry entry = entries[i];
            this.entries.put(entry.getTag(), entry);
        }
    }

    public List getEntries() {
        return new ArrayList(entries.values());
    }

    public List getEntriesInOffsetOrder() {
        List entries = getEntries();
        Collections.sort(entries, ENTRY_OFFSET_COMPARATOR);
        return entries;
    }

    /**
     * Determines whether this {...@code OffsetTable} contains an entry for the given tag.
     *
     * @param tag the tag to query
     * @return {...@code true} if this {...@code OffsetTable} contains an entry for the given tag: {...@code
     *         false} otherwise
     */
    public boolean containsEntry(Tag tag) {
        return entries.containsKey(tag);
    }

    /**
     * Returns the entry with the given tag or {...@code null} if there is no such entry.
     *
     * @param tag the tag to query
     * @return the entry with the given tag or {...@code null} if there is no such entry.
     */
    public Entry getEntry(Tag tag) {
        return (Entry) entries.get(tag);
    }

    private static final Comparator ENTRY_OFFSET_COMPARATOR = new Comparator() {
        public int compare(Object o1, Object o2) {
            long offset1 = ((Entry) o1).offset;
            long offset2 = ((Entry) o2).offset;
            return offset1 < offset2 ? -1 : offset1 > offset2 ? 1 : 0;
        }
    };

    static class Entry {

        private final Tag tag;
        private final long checksum;
        private final long offset;
        private final long length;

        Entry(Tag tag, long checksum, long offset, long length) {
            this.tag = tag;
            this.checksum = checksum;
            this.offset = offset;
            this.length = length;
        }

        public Tag getTag() {
            return tag;
        }

        public long getChecksum() {
            return checksum;
        }

        public long getOffset() {
            return offset;
        }

        public long getLength() {
            return length;
        }

        public String toString() {
            return "OffsetTable.Entry["
                    + "tag = " + tag
                    + ", checksum = " + checksum
                    + ", offset = " + offset
                    + ", length = " + length
                    + "]";
        }
    }
}

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to