Re: Shipping a build

Hello Anthony,

one way to make entrySet(), containsKey(), etc. more efficient and clean
would be to create an entrySet for each record instance, and add an
entry in it on setObject.

I didn't do it because until then you avoided adding a SFieldInstance
object, keeping data structure in tables. But if we do, we could have
SFieldInstance (implementing Map.entry) that would keep the field value
and the optimistic value in a more object oriented manner.

I don't know what the impact on performance would be. I would tend to
say, not so bad, but I really don't know...

What do you think of it ?

Franck


Le dimanche 02 août 2009 à 11:53 +1000, [email protected] a écrit :
> 
> I had another look at the SRecordInstance code, and saw the
> isValidTest. So they are indeed consistent. isEmpty is wrong, should
> be size() == 0.

Done.

> A separate issue is efficiency. One expects methods like size() to
> have very little overhead, should not generate a new object just to
> get its count. And entrySet should ideally not generate a keySet as a
> temporary object. The worst is containsKey(), which generates a keySet
> each time -- imagine someone innocently putting that in a tight loop
> and wondering why their code runs slow. I would ask you to fix at
> least this last one.
> 

> (If invalid values are included in the keySet, then keySet (but not
> entrySet) can simply be cached on SRecordMeta -- it never changes at
> all. But efficiency is secondary in these matters.)
> 
> Anyway, conclusion is I would not have bothered excising invalid
> values, but it may well be a good thing. Please fix empty. And make
> containsKey() and size() efficient. And a comment on entrySet to say
> that it is inefficient -- must create many Map.Entry objects due to
> bad Map definition. (I would not bother caching keySet or entrySet, or
> trying to update them if values change -- too hard. Easy to not create
> a keySet to create an entrySet although improvement will be minimal
> compared to all the Map.Entry objects.)
> 
> Regards,
> 
> Anthony
> 
> At 11:35 PM 1/08/2009, you wrote:
> > 
> >
> >Hi,
> >
> >Le samedi 01 août 2009 à 18:49 +1000, <mailto:anthony%
> 40berglas.org>[email protected] a écrit :
> >> 
> >> You even implemented EntrySet for the Map, which I was not
> suggesting
> >> but it is good. Should have a comment that it is inefficient --
> >> creates the map each time (OK, just a documentation issue).
> Likewise
> >> KeySet. 
> >
> >I added a comment.
> >> 
> >> get(Object x) -- if x is not String I would throw an exception
> rather
> >> than returning Null. I am more interested in usage patterns than
> the
> >> strict definition of Map. What do you think?
> >
> >You are right.
> >Moreover, the Map interface allows this behaviour (optionnal).
> >I wanted to stick to the interface for cases like using JSF, etc.
> where
> >an external component may rely on the fact that a Map is a Map :)
> >
> >I still have a question: do we only accept Strings as key, and make
> >mandatory to call getObject for SFieldMeta, or do we
> >also accept SFieldMeta as a parameter for get() ?
> >
> >> I would not have bothered with removing InvalidValues from the
> >> values().
> >I did it because if we don't, we would have this kind of weird
> >behaviour:
> >
> >ri.size() ==> 6
> >ri.put(key, value);
> >ri.size() ==> 6 !!
> >
> >> But if you do so then should probably do it everywhere --
> entrySet(),
> >> keySet(). size() is now inconsistent. Common usage would be to
> display
> >> all valid field values in a table. (But probably OK just to return
> >> fieldValues asList and forget about InvalidValues. Size is just
> >> fieldValues.length. But should be consistent.
> >
> >Well, actually I think it is consistent.
> >SRi internal fieldValues is an array whose size is equal to
> >meta.getAllFields().size().
> >For unqueried/unset fields, it will contain the InvalidValue object.
> >
> >So in the Map implementation, we get rid of those values and consider
> >that unset fields don't exist in the Map view.
> >
> >So map.size(), map.values(), map.isEmpty and map.keySet() all behave
> the
> >same: they exclude invalid Fields, until they are set. So what we
> have
> >is:
> >
> >ri.size() ==> 5
> >ri.put(key, value);
> >ri.size() ==> 6
> >
> >[Well, make me think... except if we use put to set a
> SFieldReference...
> >Then we can have
> >ri.put(refName, reference);
> >ri.size() ==> 8 !!] Not sure if it matters...
> >
> >> size() should also be reasonably efficient.)
> >> 
> >
> >> Is there a test case for the Map interface?
> >> 
> >No, I have to make one...
> >
> >> getDouble(field.getFieldName()) is a bit less efficient than I had
> in
> >> mind, string to array and back, but probably OK in practice. Good
> to
> >> see the code uncopied. (I would have created a private method
> >> convertDouble, and then called it from both getDouble methods.
> Field
> >> level methods such as get* should be efficient.)
> >> 
> >Changed the impelementation. It also allows the SException.Data to
> >contain the SFieldMeta information when comming from SRecordInstance.
> >
> >> SimpleOrm is becoming quite elegant.
> >> 
> >I also think so :)
> >
> >Franck
> >
> >
> 
> Dr Anthony Berglas, [email protected] Mobile: +61 4 4838 8874
> Just because it is possible to push twigs along the ground with ones
> nose
> does not necessarily mean that is the best way to collect firewood.
> 
> 
> 
> 





------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/SimpleORM/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/SimpleORM/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:[email protected] 
    mailto:[email protected]

<*> To unsubscribe from this group, send an email to:
    [email protected]

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/