On 11 May 2011, at 19:19, Hasan Hasan wrote: > Hi all, > > I fully support the process described by Reto > > Best regards > Hasan > > On Wed, May 11, 2011 at 6:52 PM, Reto Bachmann-Gmuer > <[email protected]> wrote: > Hi Henry > > EasyGraph seems completly unrelated to CLEREZZA-511
Well not quite. EasyGraph has automatic conversations which are listed in CLEREZZA-511. Here a some implicit def string2lit(str: String) = new PlainLiteralScala(str) implicit def date2lit(date: Date) = DATE_CONVERTER.createTypedLiteral(date) implicit def int2lit(int: Int) = INTEGER_CONVERTER.createTypedLiteral(int) Now of course, since I asked by opening the issue and you preferred not to use the converters directly, I have followed your advice and not used those directly, but the method call instead. Seems like the discussion was fruitful. > > For the rest of the commit I don't see what usefull change you think > you made. below you agree some where 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. Ok. So that's useful then. > I see no reason to make TypeConverter public. Well it is quite easy to see that there are going to be a lot more type conversions than you can list in your code. One can come up with new literal formats at the drop of a hat. So clearly people should be able to open add new TypeConverters. But ok, one can open that in a new issue. And indeed I thought I had left it private. I'll make it private again for the moment. > > 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 I have not found the list very responsive usually. Bug reports seem to work better for a conversation. As you saw recently even posting attachements to the list is difficult. > - 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 Sounds like a good idea. I think it is good to have pointers going both ways. > > 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. As long as we don't end up in a process nightmare. So of this patch I think most of it is ok, just the public constructor is not. I'll fix that. Thanks for reviewing. Henry > > 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 > > } > > > > > > > > > > > Social Web Architect http://bblfish.net/
