You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@clerezza.apache.org by Reto Bachmann-Gmuer <re...@trialox.org> on 2011/05/11 18:52:37 UTC
Strange commit (was svn commit: r1101934)
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, <bb...@apache.org> 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
> }
>
>
>
>
>
Re: Strange commit (was svn commit: r1101934)
Posted by Henry Story <he...@bblfish.net>.
Ok, did two small commits to undo most of what I had commited.
Hopefully just the good stuff remains now.
On 11 May 2011, at 20:36, Reto Bachmann-Gmuer wrote:
> On Wed, May 11, 2011 at 7:43 PM, Henry Story <he...@bblfish.net> wrote:
>>> 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.
<sarcasm>
> that's handy so we can now associate everything that use the
> literalfactory to ZZ-511 as it would potentially be different if
> ZZ-511 had been resolved differently. I suggest we rename the issue to
> "Make the world a better place" only slightly broadening the scope.
> (do we need to add sarcasm tags on this list?)
</sarcasm>
There you go :-) Looks better already.
>>>
>>> For the rest of the commit I don't see what usefull change you think
>>> you made.
>>
>> below you agree some where made.
>
> Yes, eventually I discovered something I'm could agree with. I didn't
> *see" it at the time I wrote the sentence above. This shows the
> importance of having tiny patches associated to well defined issues
> that can rapidly be closed. In a small patch for an issue labeled
> "improve perfomance of boolean to literal back-conversions" i would
> have had less troubles finding and understanding the possible
> improvement.
I agree that patch was not so good really. So I undid the bad parts.
Hopefully just the good parts are left.
>
>>
>>> 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.
>
> - They are not because there is no public way to add them.
> - Public members should have Javadoc comments
> - This is in scope of ZZ-423
agree. Will open issue when I need it, as I say below in fact.
>
>>
>> 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.
> I'm sorry you got this impression. I think the best changes for
> responses for questions like the one you added to the code are if you
> keep the question short and post a link to the respective code section
> on http://svn.apache.org/viewvc/, ideally to the patch the questioned
> section was introduced (so one can go back to the issue which makes it
> easier to understand, as long as its not the
> make-the-world-a-better-place issue)
funny but that is exactly what I did in this 511 post. You'll notice I posted a
patch, and left it up for discussion.
I just felt afterwads that some of the work I had written could be useful later
perhaps. But ok, not a bit deal.
>
>> Bug reports seem to work better for a conversation.
> Tha's fine too, there is the issue type "question" which might be usefull.
But also we don't want to open thousand of bug reports, or else people will think
the project is going nowhere. That is why sometimes I think it is good to put
comments in the code, as thoughts. Those are pointers for future reports
that in a good IDE one can find by searching for todo: comments.
>
>> As you saw recently even posting attachements to the list is
>> difficult.
> Attachments were never supported on the mailing list. I think all
> apache mailing lists are configured that way. I just added a note to
> the mailing paste on the website (takes a few hours to be live).
Good. No problem with that policy. It's just that the issue database has
been the way we have been communicating more successfully.
>
>>
>>> - 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.
> yeah well, the bunch of private constants all used in exactly one
> place is not at improvement in my view but its a detail.
yes, I had just written that code, and I did not want to throw it away.
Was not really worth it though.
>
>>
>> Thanks for reviewing.
> you're welcome :)
>
> To your question what "Collections.addAll(decimalTypes, xsdInteger,
> xsdInt, xsdByte, xsdShort);" is for: this adds the literal types that
> can be converted to various java classes representing numbers to the
> collection decimalTypes, this collection is used in various converters
> to check if they are capable of converting the literal at hand.
Ah, silly me. I had not paid attention to the addAll method. I always
use those methods to create collections, so I assumed it returned a
anonymous collection and I could not see why it was doing that.
>
> Reto
>
>>
>> Henry
>>
>>>
>>> Cheers,
>>> Reto
>>>
>>>
>>>
>>>
>>> On Wed, May 11, 2011 at 6:04 PM, <bb...@apache.org> 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/
>>
>>
Social Web Architect
http://bblfish.net/
Re: Strange commit (was svn commit: r1101934)
Posted by Reto Bachmann-Gmuer <re...@trialox.org>.
On Wed, May 11, 2011 at 7:43 PM, Henry Story <he...@bblfish.net> wrote:
>> 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.
that's handy so we can now associate everything that use the
literalfactory to ZZ-511 as it would potentially be different if
ZZ-511 had been resolved differently. I suggest we rename the issue to
"Make the world a better place" only slightly broadening the scope.
(do we need to add sarcasm tags on this list?)
>>
>> For the rest of the commit I don't see what usefull change you think
>> you made.
>
> below you agree some where made.
Yes, eventually I discovered something I'm could agree with. I didn't
*see" it at the time I wrote the sentence above. This shows the
importance of having tiny patches associated to well defined issues
that can rapidly be closed. In a small patch for an issue labeled
"improve perfomance of boolean to literal back-conversions" i would
have had less troubles finding and understanding the possible
improvement.
>
>> 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.
- They are not because there is no public way to add them.
- Public members should have Javadoc comments
- This is in scope of ZZ-423
>
> 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.
I'm sorry you got this impression. I think the best changes for
responses for questions like the one you added to the code are if you
keep the question short and post a link to the respective code section
on http://svn.apache.org/viewvc/, ideally to the patch the questioned
section was introduced (so one can go back to the issue which makes it
easier to understand, as long as its not the
make-the-world-a-better-place issue)
> Bug reports seem to work better for a conversation.
Tha's fine too, there is the issue type "question" which might be usefull.
> As you saw recently even posting attachements to the list is
> difficult.
Attachments were never supported on the mailing list. I think all
apache mailing lists are configured that way. I just added a note to
the mailing paste on the website (takes a few hours to be live).
>
>> - 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.
yeah well, the bunch of private constants all used in exactly one
place is not at improvement in my view but its a detail.
>
> Thanks for reviewing.
you're welcome :)
To your question what "Collections.addAll(decimalTypes, xsdInteger,
xsdInt, xsdByte, xsdShort);" is for: this adds the literal types that
can be converted to various java classes representing numbers to the
collection decimalTypes, this collection is used in various converters
to check if they are capable of converting the literal at hand.
Reto
>
> Henry
>
>>
>> Cheers,
>> Reto
>>
>>
>>
>>
>> On Wed, May 11, 2011 at 6:04 PM, <bb...@apache.org> 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/
>
>
Re: Strange commit (was svn commit: r1101934)
Posted by Henry Story <he...@bblfish.net>.
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 <re...@trialox.org> 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, <bb...@apache.org> 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/
Re: Strange commit (was svn commit: r1101934)
Posted by Hasan Hasan <ha...@trialox.org>.
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 <
reto.bachmann@trialox.org> wrote:
> 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, <bb...@apache.org> 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
> > }
> >
> >
> >
> >
> >
>