So yes, it probably would make more sense to have .save return false
or raise on saving a destroyed record - but it should raise if you try
to alter a destroyed record, and it makes not much sense to save a
record you’ve destroyed and not altered… :-/
I don’t know which way to plump… any thoughts?
Or… make save work. Why should a deleted record be any different than
a new unsaved record? When a record is destroyed it should lose its id
and if saved again should insert a new row. At least, that’s what I’d
expect, if the principle of least surprise means anything.
Or… make save work. Why should a deleted record be any different than a
new unsaved record? When a record is destroyed it should lose its id and if
saved again should insert a new row. At least, that’s what I’d expect, if
the principle of least surprise means anything.
Except in this case your “record” is an object instance. Since an object
instance is not a pointer to the actual record in the database it should
not
change when you destroy that record. Keep in mind that there are many
different ways to destroy a record in a database that don’t involve
calling
the .destroy method of the object instance. What should happen is that
the
.save method should return false if it can’t insert or update the record
by
that id. From following the thread it looks like its actually checking
for
an SQL execution error and it is not receiving one from the database.
What
its getting back is a successful execution of the SQL statement but a
message that states the there was no record to create/update. From this
information it then returns the incorrect state of true.
Except in this case your “record” is an object instance.
Of course.
Since an object instance is not a pointer to the actual record in the database
it should not change when you destroy that record.
That doesn’t follow. If there is no longer a row 1 in the database, the
object should no longer reference row 1. Currently the object is left
in an invalid state and save thinks an update is possible when it isn’t.
The abstraction is leaking.
Keep in mind that there are many
different ways to destroy a record in a database that don’t involve calling
the .destroy method of the object instance.
Not relevant to this case.
What should happen is that the
.save method should return false if it can’t insert or update the record by
that id.
At a minimum, agreed; it should show the affected row count. But
success is better than failure and you still haven’t acknowledged that a
destroyed object is conceptually identical to a new object that hasn’t
been saved and logically, should behave the same. Save could insert a
new record and it would perfectly conceptually elegant and plug the
leaking abstraction.
From following the thread it looks like its actually checking for
an SQL execution error and it is not receiving one from the database. What
its getting back is a successful execution of the SQL statement but a
message that states the there was no record to create/update. From this
information it then returns the incorrect state of true.
a new unsaved record? When a record is destroyed it should lose its id
and if saved again should insert a new row. At least, that’s what I’d
expect, if the principle of least surprise means anything.
That sounds dangerous to me. If someone decided to destroy a record,
resurrecting it because in a narrow window of opportunity someone
tried to update it sounds like it would lead to very difficult to
track down bugs. For save to fail/raise an exception would be less
dangerous
it should not change when you destroy that record.
That doesn’t follow. If there is no longer a row 1 in the database, the
object should no longer reference row 1. Currently the object is left in an
invalid state and save thinks an update is possible when it isn’t. The
abstraction is leaking.
.save method should return false if it can’t insert or update the record
From following the thread it looks like its actually checking for
an SQL execution error and it is not receiving one from the database. What
its getting back is a successful execution of the SQL statement but a
message that states the there was no record to create/update. From this
information it then returns the incorrect state of true.
The current behavior is clearly wrong of course.
I see your point. Without being able to modify the object after the
.destroy
call it becomes a read-only object of no value. However, being able to
salvage that object as “new” one would solve the issue. I like it.