Re: SDataSet.removeRecord(SRecordInstance rinst)

Good to hear that your problems are resolved.

I have removed the deprecated attach(record) and the associated .dirty flag.  They could potentially be useful in some situation, particularly if one wanted to move a record from one dataset to another (can not see a use case for that).  But they could also cause grief with bugs in obscure corners.  I prefer to keep it clean and simple.

Please update and check it all still works for you.

I am glad to get that resolved, it had been irritating me for some time.  And the Serialization is also now clean.

Anthony

At 11:37 PM 23/02/2009, you wrote:

>Hello,
>
>I check our code, we aren't using SDataSet.attach / 
>createDetachedInstance anymore.
>
>But isDirty() on SRecordInstance is still used. Even if in some case we 
>already know that the record is dirty because we get it from 
>dataset.getDirtyRecord(), in other it helps.
>
>I test our application with your updates and all is OK. The bug is still 
>fixed !
>
>I have more information about serialization. Actually, it was a non Sorm 
>object witch used a SRecordMeta that causes problems. The reference to 
>the meta is now transient
>and there is no serialization error coming from sorm now.
>
>Damien
>
><mailto:anthony%40berglas.org>[email protected] a écrit :
>>
>> Of course, cannot remove SRecordInstant.dirty because of deprecated 
>> instances detached from DataSets. Reverted + comment!
>>
>> Can we get rid of this, or are you still using it?
>>
>> Have removed serializatbility of SRecord|FieldMeta, plus comment.
>>
>> Please update your version and confirm all is OK.
>>
>> I suspect your Tomcat problem is unrelated. If you make the 
>> SRecordMeta variable a static per the normal patterns then it is hard 
>> to see how you could be going wrong.
>>
>> Anthony
>>
>> At 01:28 PM 21/02/2009, you wrote:
>> >Thanks for that.
>> >
>> >I cleaned it up a little. Removed SRecordInstant.dirty which is now 
>> redundant.
>> >
>> >removeRecord is now just
>> >
>> > public void removeRecord(SRecordInstance rinst) {
>> > rinst.setDirty(false);
>> > records.remove(rinst);
>> > rinst.destroy();
>> > }
>> >
>> >Please check it out and confirm it works for you.
>> >
>> >The only reason to move removeRecord is because deleteRecord is on 
>> SRecordInstance. No matter, leave it where it is. Better because clear 
>> that it is data set oriented.
>> >
>> >Anthony
>> >
>> >
>> >At 08:49 PM 20/02/2009, you wrote:
>> >
>> >>Hello,
>> >>
>> >>SDataSet.removeRecord now call instance.setDirty(false), and it 
>> works fine.
>> >>
>> >>I don't really see why it would be interesting to move removeRecord to
>> >>SRecordInstance.
>> >>It would mean "The record removes himself from the dataSet" instead of
>> >>"We ask the dataset to remove a record".
>> >>
>> >>Anyway I can commit the fix and let you check for where to put 
>> removeRecord.
>> >>
>> >>Damien
>> >>
>> >><mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
>> <mailto:anthony%40berglas.org> a écrit :
>> >>>
>> >>> Hello Damien,
>> >>>
>> >>> The checks "if records obtained from SDataSet.getDirtyRecords are not
>> >>> null" should already be there, but please check the checks. Note that
>> >>> DataSet.removeFromDirtyList set it to null.
>> >>>
>> >>> removeRecord should call instance.setDirty(false) which will 
>> remove it
>> >>> from the cache and clear the updateListIndex (which should be
>> >>> renamed). (Maybe removeRecord should be moved to
>> >>> SRecordInstance.rawRemoveRecord, although all the other DataSet only
>> >>> methods are on the DataSet. The comment should certainly refer to the
>> >>> delete method.)
>> >>>
>> >>> Please check this in when done.
>> >>>
>> >>> Anthony
>> >>>
>> >>> At 11:54 PM 17/02/2009, you wrote:
>> >>>
>> >>> >I've just implemented the fix with setting null the removed record in
>> >>> >dirty list.
>> >>> >
>> >>> >The only thing it implies is to check if records obtained from
>> >>> >SDataSet.getDirtyRecords are not null before working with them.
>> >>> >
>> >>> >I also add a test in SDataSet.removeRecord to check consistence 
>> between
>> >>> >dirtyRecordList from SDataset and dirtyRecordsIndex from 
>> SRecordInstance.
>> >>> >
>> >>> >It seems to work better when encountering this case !
>> >>> >
>> >>> >I answer about the serialization in the other thread.
>> >>> >
>> >>> 
>> ><mailto:anthony%40berglas.org><mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
>> <mailto:anthony%40berglas.org>
>> >>> <mailto:anthony%40berglas.org> a écrit :
>> >>> >>
>> >>> >> Hello Damien,
>> >>> >>
>> >>> >> Yes, I think that you have found a bug.
>> >>> >>
>> >>> >> It will only arise in practice if you actually call removeRecord.
>> >>> >> Normal operations such as Flush and Delete do not remove the 
>> records
>> >>> >> from the dirty list, and so will not trigger the bug.
>> >>> >>
>> >>> >> For now please just null the entry in dirtyRecords. Do not try to
>> >>> >> update the updateListIndex references. That is because Flush and
>> >>> >> Delete just null the entry, and the behavior should be consistent.
>> >>> >>
>> >>> >> (If ever you did want to actually dirtyRecords.remove the entry 
>> then
>> >>> >> it should be done consistently everywhere. That is a 
>> substantial job,
>> >>> >> and I think that the benefit, if any, would be minimal.)
>> >>> >>
>> >>> >> Please also rename updateListIndex to dirtyRecordsIndex to be
>> >>> >> consistent. (dirtyRecords used to be called updateList.)
>> >>> >>
>> >>> >> But first please also look at the exception that you received 
>> when you
>> >>> >> found the error. It should have been a clear, fail fast message 
>> about
>> >>> >> inconsistent data structures. If not, we may need another test. 
>> Also
>> >>> >> please add a failing test case before fixing the bug.
>> >>> >>
>> >>> >> Finally, please do try commenting out "implements Serializable" 
>> from
>> >>> >> SRecordMeta and SFieldMeta. If it does not break your 
>> application then
>> >>> >> we will remove them for good. I was always suspicious about 
>> them, and
>> >>> >> now I am pretty sure they are plain wrong.
>> >>> >>
>> >>> >> Thanks,
>> >>> >>
>> >>> >> Anthony
>> >>> >>
>> >>> >> At 08:27 PM 16/02/2009, you wrote:
>> >>> >>
>> >>> >> >Hello,
>> >>> >> >
>> >>> >> >I just find a bug in the method removeRecord from the SDataSet 
>> object.
>> >>> >> >It seems that each SRecordInstance in the dataset have a
>> >>> updateListIndex
>> >>> >> >attribute. This attribute is used to quickly find the index in 
>> the
>> >>> dirty
>> >>> >> >record list from a dataset. When removeRecord is used, the dirty
>> >>> record
>> >>> >> >list shifts all elements from the deleted one to the end of the
>> >>> list, so
>> >>> >> >that the deleted record is removed ! All the record
>> >>> updateListIndex from
>> >>> >> >this point are no longer correct. They bind their old position 
>> in the
>> >>> >> >list, not the new.
>> >>> >> >
>> >>> >> >I don't know which way is better to avoid the bug, substract 1
>> >>> from all
>> >>> >> >index that have been shifted or not remove the record but set the
>> >>> array
>> >>> >> >entry to null ... ? Setting to null is quicker but may be the 
>> cause of
>> >>> >> >futur bugs. I could trust the unit test to validate the fix. I 
>> think
>> >>> >> >shifting the updateListIndex is more reliable even if it add 
>> more cpu
>> >>> >> work.
>> >>> >> >
>> >>> >> >--
>> >>> >> >HOSTIN Damien - Equipe R&D
>> >>> >> >Tel:+33(0)4 63 05 95 40
>> >>> >> >Société Axège
>> >>> >> >23 rue Saint Simon
>> >>> >> >63000 Clermont Ferrand
>> >>> >> >www.axege.com
>> >>> >> >
>> >>> >> >
>> >>> >>
>> >>> >> Dr Anthony Berglas,
>> >>> 
>> <mailto:anthony%40berglas.org><mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
>> <mailto:anthony%40berglas.org>
>> >>> <mailto:anthony%40berglas.org> <mailto:anthony%40berglas.org>
>> >>> >> 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.
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> ----------------------------------------------------------
>> >>> >>
>> >>> >
>> >>> >--
>> >>> >HOSTIN Damien - Equipe R&D
>> >>> >Tel:+33(0)4 63 05 95 40
>> >>> >Société Axège
>> >>> >23 rue Saint Simon
>> >>> >63000 Clermont Ferrand
>> >>> >www.axege.com
>> >>> >
>> >>> >
>> >>>
>> >>> Dr Anthony Berglas, 
>> <mailto:anthony%40berglas.org><mailto:anthony%40berglas.org>[email protected] 
>> <mailto:anthony%40berglas.org> <mailto:anthony%40berglas.org>
>> >>> 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.
>> >>>
>> >>>
>> >>
>> >>--
>> >>HOSTIN Damien - Equipe R&D
>> >>Tel:+33(0)4 63 05 95 40
>> >>Société Axège
>> >>23 rue Saint Simon
>> >>63000 Clermont Ferrand
>> >>www.axege.com
>> >>
>> >>
>> >
>> >Dr Anthony Berglas, <mailto:anthony%40berglas.org>[email protected] 
>> <mailto:anthony%40berglas.org> 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.
>>
>> Dr Anthony Berglas, <mailto:anthony%40berglas.org>[email protected] <mailto:anthony%40berglas.org> 
>> 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.
>>
>> 
>
>-- 
>HOSTIN Damien - Equipe R&D
>Tel:+33(0)4 63 05 95 40
>Société Axège
>23 rue Saint Simon
>63000 Clermont Ferrand
>www.axege.com
>
>

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/