Re: SDataSet.removeRecord(SRecordInstance rinst)

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

[email protected] 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>[email protected] 
> <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>[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, [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




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

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/