Made Attribute abstract, rewrote Javadocs.

src/main/java/com/sk89q/worldguard/region/Attribute.java

@@ -29,23 +29,47 @@
  * <p>
  * Example attributes include:</p>
  * <ul>
- * <li>PvP allow/denial flag</li>
- * <li>Owner of the region</li>
- * <li>Faction of the region</li>
- * <li>A picture of your crush</li>
+ *   <li>PvP allow/denial flag</li>
+ *   <li>Owner of the region</li>
+ *   <li>Faction of the region</li>
+ *   <li>A picture of your crush</li>
  * </ul>
  * <p>
- * Normally, asking for attributes from a region index will either return
- * a {@link DataValuedAttribute} or a custom overriding attribute class.
- * This class is never used for that purpose because it loses the
- * data it reads in.
+ * Asking for attributes from a region will return a subclass of
+ * this class, which can be either an instance of {@link DataValuedAttribute}
+ * or a custom overriding attribute class available in WorldGuard or
+ * from another project.
  * <p>
- * To create your own attributes, see the class documentation for
- * {@link DataValuedAttribute}.
+ * Creating your attributes to store any type of data is easy. The most
+ * important decision to make is whether to use {@link DataValuedAttribute}
+ * or your own subclass. The latter lets you store the attribute data
+ * in a structure more native to your software (using primitive data types and
+ * your own objects), but it may be not be necessary if your payload does
+ * consist of raw binary data, which is what {@link DataValuedAttribute} does.
+ * <p>
+ * When you make a subclass, you must define the
+ * {@link #read(DataInputStream, int)} and
+ * {@link #write(DataOutputStream)} methods so that the incoming data
+ * can be deserialized and the outgoing data can be serialized. The reason is
+ * because whatever is storing the region data has to ultimately store the
+ * data as a series of ones and zeros, and rather than using Java's native
+ * serialization routines (which may be subject to change), we instead
+ * delegate that task explicitly to each attribute. Remember that is important
+ * to choose a format that can be changed in the future and still maintain
+ * backwards compatibility.
+ * <p>
+ * During loading, subclasses will automatically be found by searching
+ * the classpath, as the canonical name of each attribute's class is saved
+ * during saving. The process involved here is outlined more closely on
+ * the WorldGuard wiki. If worse comes to worse, and the subclass cannot
+ * be found, then the data will be loaded as an {@link DataValuedAttribute},
+ * which prevents loss of the raw binary data, but it will render the
+ * data unusuable during runtime unless explicitly deserialization is
+ * conducted when recalling the attribute.
  */
-public class Attribute {
+public abstract class Attribute {
     
-    private String name;
+    private String name = "unnamed";
 
     /**
      * A no-arg constructor required for automatic instantiation of
@@ -54,6 +78,19 @@ public class Attribute {
      */
     public Attribute() {
     }
+    
+    /**
+     * Construct an instance and set the name.
+     * <p>
+     * Subclasses do not have to provide this constructor but it is helpful
+     * to do so.
+     * 
+     * @param name the name to set
+     * @see #setName(String)
+     */
+    public Attribute(String name) {
+        setName(name);
+    }
 
     /**
      * Get the name of this attribute. Attribute names allow for distinguishing
@@ -71,13 +108,14 @@ public String getName() {
      * {@link Region} must be made aware of this change or very bad
      * things may happen.
      * <p>
-     * All valid Unicode strings are supported for names.
+     * All valid non-empty Unicode strings are supported for names.
      * 
      * @param name new name to use
      * @see Region#rename(Attribute, String)
      */
     public void setName(String name) {
         Validate.notNull(name);
+        Validate.notEmpty(name);
         
         this.name = name;
     }
@@ -85,9 +123,8 @@ public void setName(String name) {
     /**
      * Read the input data stream and do something with the incoming data.
      * <p>
-     * This method is called whenever the attribute is being loaded. By
-     * default, this method does nothing and therefore causes data
-     * loss if not overridden.
+     * This method is called whenever the attribute is being unserialized
+     * from persistent storage.
      * 
      * @param in input data stream
      * @param len length of the data
@@ -95,24 +132,18 @@ public void setName(String name) {
      *                    of this class to fail, causing the region index to
      *                    fall back to {@link DataValuedAttribute}
      */
-    public void read(DataInputStream in, int len) throws IOException {
-        
-    }
+    public abstract void read(DataInputStream in, int len) throws IOException;
     
     /**
      * Write out this attribute's data.
      * <p>
-     * This method is called whenever the attribute is being saved. By
-     * default, this method does nothing and therefore causes data
-     * loss if not overridden.
+     * This method is called whenever the attribute is being serialized.
      * 
      * @param out output data stream
      * @throw IOException thrown on write error, which is really bad and would
      *                    cause data loss
      */
-    public void write(DataOutputStream out) throws IOException {
-        
-    }
+    public abstract void write(DataOutputStream out) throws IOException;
 
     /**
      * Hash codes are based on attribute name.
@@ -127,13 +158,14 @@ public int hashCode() {
      */
     @Override
     public boolean equals(Object obj) {
+        if (obj == null) return false;
         if (!(obj instanceof Attribute)) return false;
         return getName().equals(((Attribute) obj).getName());
     }
 
     @Override
     public String toString() {
-        return "[Attribute: " + getName() + "]";
+        return "Attribute(" + getName() + ")";
     }
     
 }

src/main/java/com/sk89q/worldguard/region/DataValuedAttribute.java

@@ -23,37 +23,16 @@
 import java.io.IOException;
 
 /**
- * A simple implementation of {@link Attribute} that actually can keep
- * its data between reads and writes.
+ * A simple implementation of {@link Attribute} that saves the raw binary 
+ * data stream for exact recall later during runtime and during serialization.
  * <p>
  * By itself, this class will accept binary data, store it as-is, then
- * dump it back out as binary data when requested. If you wish to use
- * attributes, then one option is to manually decode this binary data somewhere
- * in your code and then re-encode the data and put it back. However, the
- * better idea would be to extend this class with your own version and
- * override the {@link #read(DataInputStream, int)} and
- * {@link #write(DataOutputStream)} methods so they can serialize
- * and unserialize the data.
+ * dump it back out as binary data when requested. If you have special needs,
+ * and do not want to have to explicitly deserialize the contents contained
+ * within this attribute, consider subclassing {@link Attribute} instead.
  * <p>
- * The way attributes are loaded is that the region
- * loader will first search to see if the corresponding overriding
- * {@link Attribute} class exists somewhere in its class path
- * (the class's full canonical name is written during serialization of the
- * region data). If it does, the class will be instantiated with the
- * <strong>no-argument constructor</strong>, the name will be set
- * with {@link Attribute#setName(String)}, and then the raw data stream
- * will be given to {@link Attribute#read(DataInputStream, int)}. If all
- * goes well, the corresponding class found will be what is returned when
- * a copy of the attribute is requested for a region.
- * <p>
- * However, if the class does not appear visible to the class loader, or
- * during instantiation and setup, the process fails, this class
- * ({@link DataValuedAttribute}) will be utilized instead. This way, even if
- * the class for the attribute does not exist at runtime, the data will be
- * still be maintained on the region.
- * <p>
- * The regular {@link Attribute} class is never actually used because
- * it discards data on read.
+ * This is the automatic fallback attribute in case custom attribute
+ * classes are unavailable during load.
  */
 public final class DataValuedAttribute extends Attribute {
     

src/test/java/com/sk89q/worldguard/region/AttributeTest.java

@@ -29,46 +29,40 @@
 import org.junit.Test;
 
 public class AttributeTest {
-    
-    private static Attribute makeAttribute(String name) {
-        Attribute attribute = new Attribute();
-        attribute.setName(name);
-        return attribute;
-    }
 
     @Test
     public void testHashCode() {
-        assertEquals(makeAttribute("testing").hashCode(), "testing".hashCode());
-        assertFalse(makeAttribute("broken").hashCode() == "testing".hashCode());
+        assertEquals(new TestAttribute("testing").hashCode(), "testing".hashCode());
+        assertFalse(new TestAttribute("broken").hashCode() == "testing".hashCode());
     }
 
     @Test
     public void testAttribute() {
-        makeAttribute("testing");
+        new TestAttribute("testing");
     }
 
     @Test
     public void testGetName() {
-        assertEquals(makeAttribute("testing").getName(), "testing");
-        assertFalse(makeAttribute("testing").getName().equals("broken"));
+        assertEquals(new TestAttribute("testing").getName(), "testing");
+        assertFalse(new TestAttribute("testing").getName().equals("broken"));
     }
 
     @Test
     public void testSetName() {
-        assertEquals(makeAttribute("testing").getName(), "testing");
-        assertFalse(makeAttribute("testing").getName().equals("broken"));
+        assertEquals(new TestAttribute("testing").getName(), "testing");
+        assertFalse(new TestAttribute("testing").getName().equals("broken"));
     }
 
     @Test
     public void testRead() throws IOException {
-        Attribute attribute = makeAttribute("testing");
+        Attribute attribute = new TestAttribute("testing");
         byte[] data = new byte[] { 1, 2, 3, 4, 5, 6 };
         attribute.read(new DataInputStream(new ByteArrayInputStream(data)), data.length); // Do nothing
     }
 
     @Test
     public void testWrite() throws IOException {
-        Attribute attribute = makeAttribute("testing");
+        Attribute attribute = new TestAttribute("testing");
         ByteArrayOutputStream stream = new ByteArrayOutputStream(100);
         attribute.write(new DataOutputStream(stream)); // Do nothing
         assertEquals(stream.size(), 0);
@@ -76,8 +70,8 @@ public void testWrite() throws IOException {
 
     @Test
     public void testEqualsObject() {
-        assertTrue(makeAttribute("testing").equals(makeAttribute("testing")));
-        assertFalse(makeAttribute("testing").equals(makeAttribute("broken")));
+        assertTrue(new TestAttribute("testing").equals(new TestAttribute("testing")));
+        assertFalse(new TestAttribute("testing").equals(new TestAttribute("broken")));
     }
 
 }

src/test/java/com/sk89q/worldguard/region/TestAttribute.java

@@ -0,0 +1,44 @@
+// $Id$
+/*
+ * This file is a part of WorldGuard.
+ * Copyright (c) sk89q <http://www.sk89q.com>
+ * Copyright (c) the WorldGuard team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify it under the
+ * terms of the GNU Lesser General Public License as published by the Free Software
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY), without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package com.sk89q.worldguard.region;
+
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+
+import org.junit.Ignore;
+
+@Ignore
+public class TestAttribute extends Attribute {
+
+    public TestAttribute(String name) {
+        super(name);
+    }
+
+    @Override
+    public void read(DataInputStream in, int len) throws IOException {
+        // do nothing
+    }
+
+    @Override
+    public void write(DataOutputStream out) throws IOException {
+        // do nothing
+    }
+
+}