Hi Henry EasyGraph seems completly unrelated to CLEREZZA-511
For the rest of the commit I don't see what usefull change you think you made. I see some code rearrengement and introduction of private fields with imho don't make things more readable but harder to maintain. I see that you added questions which I think belong on the mailing list and not in the code, some lobbying for the modularized literal factory and yes, I think I could agree with the TRUE/FALSE constants. I see no reason to make TypeConverter public. Please: - separate the patches for code-renicing from actual improvements (here the performance improvement) - associate patches to an issue from which the motivation for the patch can be deducted - for performance improvement: focus on bottle-necks that show up in the profiler, otherwise we might make the code no complicates without actual gain - post questions to the mailing list - if adding todos to the code refer to the existing issue that would cover them, the "todo: create a subclass of TypedLiteral that contains the original value, then one would not need" could refer to CLEREZZA-423 I'm sorry for being pedantic, but I think that clerezza can only be a stable and manageable code base if we stick to the process and are a bit conservative on which patches we accept. Cheers, Reto On Wed, May 11, 2011 at 6:04 PM, <[email protected]> wrote: > Author: bblfish > Date: Wed May 11 16:04:20 2011 > New Revision: 1101934 > > URL: http://svn.apache.org/viewvc?rev=1101934&view=rev > Log: > CLEREZZA-511: Was going to Allow access to single individual > SimpleLiteralFactory converters but reto prefers not. Other changes are still > useful > > Modified: > > incubator/clerezza/trunk/parent/rdf.core/src/main/java/org/apache/clerezza/rdf/core/impl/SimpleLiteralFactory.java > > incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EasyGraph.scala > > Modified: > incubator/clerezza/trunk/parent/rdf.core/src/main/java/org/apache/clerezza/rdf/core/impl/SimpleLiteralFactory.java > URL: > http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/rdf.core/src/main/java/org/apache/clerezza/rdf/core/impl/SimpleLiteralFactory.java?rev=1101934&r1=1101933&r2=1101934&view=diff > ============================================================================== > --- > incubator/clerezza/trunk/parent/rdf.core/src/main/java/org/apache/clerezza/rdf/core/impl/SimpleLiteralFactory.java > (original) > +++ > incubator/clerezza/trunk/parent/rdf.core/src/main/java/org/apache/clerezza/rdf/core/impl/SimpleLiteralFactory.java > Wed May 11 16:04:20 2011 > @@ -55,8 +55,36 @@ public class SimpleLiteralFactory extend > > > final private static Set<UriRef> decimalTypes = new HashSet<UriRef>(); > + > + final static private TypeConverter<byte[]> BYTE_ARRAY_CONVERTER = new > ByteArrayConverter(); > + final static private TypeConverter<Boolean> BOOLEAN_CONVERTER = new > BooleanConverter(); > + final static private TypeConverter<Date> DATE_CONVERTER = new > DateConverter(); > + final static private TypeConverter<String> STRING_CONVERTER = new > StringConverter(); > + final static private TypeConverter<Integer> INTEGER_CONVERTER = new > IntegerConverter(); > + final static private TypeConverter<BigInteger> BIG_INTEGER_CONVERTER > = new BigIntegerConverter(); > + final static private TypeConverter<Long> LONG_CONVERTER = new > LongConverter(); > + final static private TypeConverter<Double> DOUBLE_CONVERTER = new > DoubleConverter(); > + final static private TypeConverter<UriRef> URIREF_CONVERTER = new > UriRefConverter(); > + > + final private static Map<Class<?>, TypeConverter<?>> typeConverterMap > = new HashMap<Class<?>, TypeConverter<?>>(); > + final static Class<? extends byte[]> byteArrayType; > + > static { > + //what's this for? > Collections.addAll(decimalTypes, xsdInteger, xsdInt, xsdByte, > xsdShort); > + > + > + byte[] byteArray = new byte[0]; > + byteArrayType = byteArray.getClass(); > + typeConverterMap.put(byteArrayType, BYTE_ARRAY_CONVERTER); > + typeConverterMap.put(Date.class, DATE_CONVERTER); > + typeConverterMap.put(Boolean.class, BOOLEAN_CONVERTER); > + typeConverterMap.put(String.class, STRING_CONVERTER); > + typeConverterMap.put(Integer.class, INTEGER_CONVERTER); > + typeConverterMap.put(BigInteger.class, BIG_INTEGER_CONVERTER); > + typeConverterMap.put(Long.class, LONG_CONVERTER); > + typeConverterMap.put(Double.class, DOUBLE_CONVERTER); > + typeConverterMap.put(UriRef.class, URIREF_CONVERTER); > } > > final private static UriRef xsdDouble = > @@ -64,13 +92,8 @@ public class SimpleLiteralFactory extend > final private static UriRef xsdAnyURI = > new UriRef("http://www.w3.org/2001/XMLSchema#anyURI";); > > - final static Class<? extends byte[]> byteArrayType; > - static { > - byte[] byteArray = new byte[0]; > - byteArrayType = byteArray.getClass(); > - } > > - private static interface TypeConverter<T> { > + public static interface TypeConverter<T> { > TypedLiteral createTypedLiteral(T value); > T createObject(TypedLiteral literal); > } > @@ -125,15 +148,22 @@ public class SimpleLiteralFactory extend > > private static final UriRef booleanUri = > new UriRef("http://www.w3.org/2001/XMLSchema#boolean";); > + public static final TypedLiteralImpl TRUE = new > TypedLiteralImpl("true", booleanUri); > + public static final TypedLiteralImpl FALSE = new > TypedLiteralImpl("false", booleanUri); > > @Override > public TypedLiteral createTypedLiteral(Boolean value) { > - return new TypedLiteralImpl(value.toString(), > booleanUri); > + if (value) return TRUE; > + else return FALSE; > } > > + //todo: create a subclass of TypedLiteral that contains the > original value, then one would not need > + //to do these conversions > @Override > public Boolean createObject(TypedLiteral literal) { > - if (!literal.getDataType().equals(booleanUri)) { > + if (literal == TRUE) return true; > + else if (literal == FALSE) return false; > + else if (!literal.getDataType().equals(booleanUri)) { > throw new > InvalidLiteralTypeException(Boolean.class, literal.getDataType()); > } > return Boolean.valueOf(literal.getLexicalForm()); > @@ -248,20 +278,6 @@ public class SimpleLiteralFactory extend > } > } > > - final private static Map<Class<?>, TypeConverter<?>> typeConverterMap > = new HashMap<Class<?>, TypeConverter<?>>(); > - > - static { > - typeConverterMap.put(byteArrayType, new ByteArrayConverter()); > - typeConverterMap.put(Date.class, new DateConverter()); > - typeConverterMap.put(Boolean.class, new BooleanConverter()); > - typeConverterMap.put(String.class, new StringConverter()); > - typeConverterMap.put(Integer.class, new IntegerConverter()); > - typeConverterMap.put(BigInteger.class, new > BigIntegerConverter()); > - typeConverterMap.put(Long.class, new LongConverter()); > - typeConverterMap.put(Double.class, new DoubleConverter()); > - typeConverterMap.put(UriRef.class, new UriRefConverter()); > - } > - > > @SuppressWarnings("unchecked") > @Override > > Modified: > incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EasyGraph.scala > URL: > http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EasyGraph.scala?rev=1101934&r1=1101933&r2=1101934&view=diff > ============================================================================== > --- > incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EasyGraph.scala > (original) > +++ > incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/EasyGraph.scala > Wed May 11 16:04:20 2011 > @@ -24,8 +24,10 @@ import collection.mutable.Queue > import impl._ > import org.apache.clerezza.rdf.ontologies.{RDF, RDFS, FOAF} > import java.math.BigInteger > -import java.util.Date > import org.apache.clerezza.rdf.utils.{UnionMGraph, GraphNode} > +import java.util.{HashSet, Collections, Date} > +import java.lang.Boolean > +import com.sun.tools.internal.xjc.reader.xmlschema.BindGreen > > object EasyGraph { > final val en = "en" > @@ -33,10 +35,17 @@ object EasyGraph { > final val fr = "fr" > val litFactory = new SimpleLiteralFactory() > > + import org.apache.clerezza.rdf.core.impl.SimpleLiteralFactory._ > + > implicit def string2lit(str: String) = new PlainLiteralScala(str) > implicit def date2lit(date: Date) = litFactory.createTypedLiteral(date) > - implicit def int2lit(date: Int) = litFactory.createTypedLiteral(date) > - > + implicit def int2lit(int: Int) = litFactory.createTypedLiteral(int) > + implicit def bigint2lit(bint: BigInt) = > litFactory.createTypedLiteral(bint.underlying()) > + implicit def bigint2lit(bigInt: BigInteger) = > litFactory.createTypedLiteral(bigInt) > + implicit def bool2lit(boolean: Boolean) = > litFactory.createTypedLiteral(boolean) > + implicit def scalaBool2lit(boolean: scala.Boolean) = > litFactory.createTypedLiteral(boolean) > + implicit def long2lit(long: Long) = > litFactory.createTypedLiteral(long) > + implicit def double2lit(double: Double) = > litFactory.createTypedLiteral(double) > > > // val g = new EasyGraph(new SimpleMGraph) > @@ -103,6 +112,12 @@ class PlainLiteralScala(string: String) > > class EasyGraph(val graph: TripleCollection) extends SimpleMGraph(graph) { > > + /* > + * because we can't jump straight to super constructor in Scala we > need to > + * create the collection here > + **/ > + def this() = this( new SimpleMGraph() ) > + > def +=(other: Graph) = { > if (graph ne other) graph.addAll(other) > } > @@ -115,6 +130,9 @@ class EasyGraph(val graph: TripleCollect > > def apply(subj: NonLiteral) = new EasyGraphNode(subj, this) > > + //note one could have an apply for a Literal that would return a > InversePredicate > + //but that would require restructuring EasyGraphNode so that one can > have an EasyGraphNode > + //with a missing ref, or perhaps a sublcass of EasyGraphnode that > only has the <- operator available > } > > > > >
