views:

150

answers:

4

In the existing code I am working with I have found the money transfer procedure which isn't wrapped in transaction, is it totally unacceptable or it is OK because there are all the needed sanity checks, i.e. that amount of money to transfer is greater than zero, and ORM level validation that balance is greater or equal to zero.

Thanks in advance

Added: Thank you guys for your responses, all of them are correct, I just marked @Garrett's because he had less reputation.

PS. The reason why I actually got so baffled in the first place with this is because it comes from a solid developer, and it's such a by-book piece of code, that seeing something like this with no transaction in sight is weird:

self.balance   -= amount
save!

target.balance += amount
target.save!
+2  A: 

A transactional approach to database operations approaches - among the others - two kinds of problems:

  1. it provides protection from concurrent access
  2. it provides a rollback mechanism

I don't see how any check might replace these two...

Manrico Corazzi
+4  A: 

Absent seeing any code, I'll make the educated guess that the existing code is NOT acceptable. Even with the sanity checks, if a withdrawal from that account is made in between statements in the "transfer" procedure, you would have a race condition in your code which would cause a transfer of non-existent funds. The probability of this occurrence is greater when there are many concurrent users, of course.

Garrett
A: 

Sure you have to fix it as soon as possible. A transaction is a logical unit of work that may include any number of file or database operation. Only transaction can assure that everything is ok or not and escapes you for a log back of recovering from an inconsistent accidental state.

ruslander
+1  A: 

Let me show you why this is a bad thing with some pseudo code.

Two money transfers :

#1: How much money does George have? $1500, that is fine.
#1: Send $1000 to Martha.
#1: Take $1000 from George.

#2: How much money does George have? $500, this is not enough.  Give up!
#2: Do not send $1000 to Martha.
#2: Do not take $1000 from George.

What really happens:

#1: How much money does George have? $1500, that is fine.
#2: How much money does George have? $1500, that is fine.
#1: Send $1000 to Martha.
#2: Send $1000 to Martha.
#1: Take $1000 from George.
#2: Take $1000 from George.

George has successfully transfered money he does not have!

The more traffic your database is getting, the more likely this sort of collision becomes.

Brian