Is this instance of mixing FP and OOP a good design?

Question

I had a strange-feeling pattern come up in some code I was writing. In a project with user accounts, there was a lot of code that needed to do common things such as creating accounts, deleting them, logging them in and out, extending their logged-in session, etc. etc. etc. Now when I was first writing this, I just threw all those responsibilities into one class (which was horrible I know). When it was time to clean this up, I ended up going with an approach that felt right but also strange.

I set up an AccountAccessor class which held private functions for getting the data transfer objects associated with an account, then I made several "action" classes that each would do one thing with access to an account, letting me write:

new AccountPasswordSetter(new AccountAccessor(username)).SetPassword(password);

This felt wrong, because

1. The action classes were very, very tightly coupled with the AccountAccessor class - there was no hope of testing them
2. The AccountAccessor class would have to expose it's data access functions for the action classes to use them.
3. I have to expose construction of the action classes to the client code. This felt problematic because really, there's no sensible reason anything outside the project needs to concern itself with their instantiation.
4. Each of the action classes only needed one of the data transfer objects the AccountAccessor provided - for example an action class responsible for logging the user out doesn't need access to the account's password.
5. Writing this feels very wordy and "inside out".

What I ended up doing instead was I made the action classes all able to be constructed only inside the project, and instead of needing an AccountAccessor I had them set up to simply accept a function that would take a connection to the database and return the one particular DTO they needed. Then, when the account accessor was constructed, would would create a whole slew of these action objects and pass each of them one of it's private methods for getting the DTO. Something like:

public class AccountAccessor {
    public AccountPasswordSetter PasswordSetter { get; private set; }
    public AccountSessionExtender SessionExtender { get; private set; }
    public string Username { get; private set; }

    public AccountAccessor(string username) {
        Username = username;
        PasswordSetter = new PasswordSetter(getPasswordEntity);
        SessionExtender = new SessionExtender(getSessionEntity);
    }

    private PasswordEntity getPasswordEntity(Connection databaseConnection) { ... }
    private SessionEntity getSessionEntity(Connection databaseConnection) { ... }
}

Now, client code can simply call something like:

new AccountAccessor(username).PasswordSetter.Set(password)`

The responsibilities are split up, and the action classes don't have to know or care about what an AccountAccessor is. The client code doesn't have to know or care about how the various objects that provide functionality are set up. The AccountAccessor only selectively exposes access to the database entities. This felt much, much better. What I liked most about this design was it solved a lot of the problems I was having without imposing any architectural headaches on the client code. If client code wants to log out an account and delete it, they just get an accessor then log it out and delete it – no need to work out how to properly pass objects around. The obvious approach for the client code just works.

But it still seems a little off – passing private methods as arguments to other objects comes across as fundamentally strange to me. I wanted to know more about what sort of problems this has, and what I should keep in mind when designing things fitting this sort of scenario in the future.

Answer 1

passing private methods as arguments to other objects comes across as fundamentally strange to me.

They're called first-class functions. It's a perfectly valid technique.

That said, are you sure that this isn't all a bit over-engineered? What's wrong with simply doing something like this?

user.SetPassword(newPassword);

If you're bothered by tight-coupling, then provide a constructor overload that hands the necessary machinery for setting a password to the User object. Dependency Injection is also a perfectly valid technique. If you want even further decoupling, use an IoC container.

Answer 2

I think your statements

new AccountPasswordSetter(new AccountAccessor(username)).SetPassword(password);

and

new AccountAccessor(username).PasswordSetter.Set(password);

cannot benefit from FP. It is good that you know Scheme, so I can use FP terminologies to explain: clearly the sole purpose of your above statements is to cause side effect.

The purpose of an expression is to compute a value, not to cause a side effect. The purpose of a statement is to cause a side effect.

The following statement should be clear enough:

user.Password = password;

This is basic OOP, but inside the setter, you can use FP.

public string Password
{
    ...
    set { _password = Encrypt(value); }
}

This time, Encrypt(value) is an expression, not a statement. The method can be tested independently if it is static. As long as the input is the same, the output is always the same, no matter there is a database or not. So you can break the complex operation to small and testable expressions.