Comments on Andrzej on Software: account.transfer(..) - is it good OOP?

2012-05-31T05:33:33.890+02:00

This comment has been removed by the author.

PaymentOrder p = new PaymentOrder(); p.setRemitter...

2012-05-30T11:40:25.052+02:00

PaymentOrder p = new PaymentOrder();
p.setRemitterAccount(...)
p.setBeneficiaryAccount(...);
p.setAmount(new MonetaryAmount(new BigDecimal("200.00"), Currencies.USD));
paymentService.debit(p);

It really comes down to what makes sense in your particular context/use case

That would make the Bank object some type of God o...

2012-05-30T09:49:58.465+02:00

That would make the Bank object some type of God object. I wouldn't do it.
http://en.wikipedia.org/wiki/God_object

Your design will need to grow from your actual use...

2012-05-29T22:22:48.185+02:00

Your design will need to grow from your actual use case. What you propose may be sufficient, but there may be much more involved in "transfer 200 to an account."

For example, some systems might require double-entry accounting. I've use a helpful article on double-entry accounting to guide my design in the past, and I'd recommend reading it to help inform this discussion.

That model seems over-engineered for many scenarios. For my use cases, building out a batch & journal system was unnecessary -- volumes are low, and those can be added later. Additionally, I removed the separation between "account" and "user" since our system only ever has a single account per user.

One way to consider whether the design is "good" is to figure out what you can and can't change if you adopt that design. By adopting the "posting" & "asset_type" pattern from this (I always rename them to "transaction" and "transaction_type"), your system could easily grow to provide more complicated accounting and reporting functionality. Simply doing "account.transfer(...)" might indicate that you're missing supporting infrastructure, the lack of which might bite you later.

I'd just think about that like real money hand...

2012-05-29T22:19:51.297+02:00

I'd just think about that like real money handling.

* You are in control of your own pocket.
* Only you know how much money there are.
* Only you can take out money from your pocket and give it to other people (otherwise it is stealing).
* If receiving party doesn't want to take money then who cares, money is yours after all!

So
John.give_money(allowance, JohnJr)

Accounts should not know about other accounts. The...

2012-05-29T22:16:50.592+02:00

Accounts should not know about other accounts. They don't need to. If it helps your code to let them manage transfers, your code is broken. I think examples keep it 'simple' to prevent the introduction of semaphores etc at beginner level. Try multithreading an app where account.add() works :-S

Why not something like: Account.deposit(otherAcco...

2012-05-29T20:45:45.082+02:00

Why not something like:

Account.deposit(otherAccount.withdraw(200));

Though I agree, putting it in a transaction object which can manage any exceptions is probably a better choice.

John

I agree with kikito and have the same idea. Bank i...

2012-05-29T20:18:58.955+02:00

I agree with kikito and have the same idea. Bank is better for manage account, transfers etc, Transfer should have responsibility for changing account price.

Responsibility for account is probably information about owner and price state, not for managing this data, and for risk operation like transfer I think special class is obvious.

In my mind a balance (pardon the pun) needs to be ...

2012-05-29T17:45:52.558+02:00

In my mind a balance (pardon the pun) needs to be found between modelling the real semantics of the problem domain and writing maintainable easy to understand code.

I think a Bank type would be something of a god class and open to abuse?

A Transfer type maybe more sensible especially if it is invoked as a result of a transfer screen / api?

I would argue that context is very important and in a large system you will have more than one context and thus more than one Transfer type.

Concepts such as Boundary Contexts [1] are probably overkill in most applications though helpful in large financial apps (certainly from my expereince where they swifty turn into big balls of mud and entropy is smacking you on the nose with technical debt every day)

Ultimately...it depends :)

Just thinking... Isn't the bank an object doin...

2012-05-29T16:23:47.598+02:00

Just thinking... Isn't the bank an object doing a transfer within his infrastructure instead of transfer "doing" itself?

Transfer.new(src, target, amount)
Bank#execute(transfer)

There is several possible ways to di that. One is more OOP-ish than the other. This classic account#transfer is just to scratch the sufrace of OOP for beginners. Sadly some developers see nothing wrong with that.

If it was really banking software it would probabl...

2012-05-29T13:55:50.269+02:00

If it was really banking software it would probably look more like:

transaction = Transaction.new
transaction.add(Operation.new(account1, amouont, TYPE_DEBIT)
transaction.add(Operation.new(account2, amount, TYPE_CREDIT)
transaction.process

And this snippet would be all over the place :)

Bartosz, exactly, it's very common in Rails/Ac...

2012-05-29T13:05:54.304+02:00

Bartosz, exactly, it's very common in Rails/ActiveRecord, however as I wrote, this example is very popular in many other places.

In OOP, an Account should know nothing about other...

2012-05-29T13:02:48.662+02:00

In OOP, an Account should know nothing about other accounts.

It's very common antipattern to find in "too fat" ActiveRecord models, responsible for everything related to them.

Thanks Paul, wise words. I'm just unable to f...

2012-05-29T12:58:43.974+02:00

Thanks Paul, wise words.

I'm just unable to find models where it feels good that the source account should know about the target account, even for just one second.

There is an infinite number of ways to represent t...

2012-05-29T12:56:30.960+02:00

There is an infinite number of ways to represent this. You could really start to pull out every single piece of functionality into differing abstractions and combine them in wonderful ways.

This example is used this way because its how many people think about transfers between accounts, and pretty much everyone has a bank account and does transfers. So its a universal concept.

Many banks actually create a transfer object, containing the amounts, the two accounts. They apply initially a reduction in the source account (but its more complex than that) and then execute the transfer separately via a transfers system to other banks if needed. But how any organisation decides to do that really depends on their model of the world and the way it works for them.

Yes, wrapping it in a transfer object is certainly...

2012-05-29T12:54:01.624+02:00

Yes, wrapping it in a transfer object is certainly a good idea.

I don't think that it may be an overkill in smaller apps. It's always good to have a nice OOP design.

Well, you certainly could do this: Transfer.new(2...

2012-05-29T12:48:47.412+02:00

Well, you certainly could do this:

Transfer.new(200, source_account, dest_account)

The adecuacy of this depends on whether it makes sense to have Transfers as individual objects in your system. I a banking system it would probably make a lot of sense, but in a video store app it would probably be overkill.