0

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.

  • is your question c# or language-agnostic or something else? – gnat Sep 19 '14 at 16:28
  • The specific project is C#, but it most likely applies to any language that is primarily object oriented but supports minimal functional programming. I'll tag it as C# though since that would be most relevant. – Jack Sep 19 '14 at 16:31
  • Passing around a private method as Action<…>/Func<…> is conceptually no different from passing around an object of type private class Impl : Interface as Interface, and you probably do that all the time in OOP. All that the receiving end should care about is that it gets an object that fulfills a public, well-known contract (e.g. Action, Func, Interface). – stakx Sep 19 '14 at 16:35
  • That was exactly my thought. If I went the interface route I'd have to have some interface for each of the entity-getting methods, and that would mandate those methods be publicly exposed. Ideally I don't want the client code to care about any of the database entities. They should just care about getting the things they want done, done. – Jack Sep 19 '14 at 16:42
  • 1
    Just one more thought regarding Action<…> and Func<…>: While the compiler has great support for these delegate types, they are (intentionally) very generic. I can see why you might not want to define tons of single-method interfaces instead, but consider declaring your own custom delegate types instead of using Action and Func by default: One major advantage would be that you could give your delegate types and their parameters meaningful, intention-revealing names. – stakx Sep 19 '14 at 16:46
4

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.

  • It wasn't any unfamiliarity with first class functions that bothered me (Scheme is my personal language of choice), it just felt like I was using them here because I'm familiar with them rather than looking for a better solution. I went with this setup because the code to set the password actually has to do some cryptography, and there's more like ten or a dozen action classes rather than just the two I went with. Sticking all that code in one class was horrible to work with. I don't know that much about inversion of control beyond the general idea, know any good teaching resources? – Jack Sep 19 '14 at 16:39
  • Fundamentally, IoC is just "handing your dependencies to the class through the constructor." IoC containers automate the process of providing those dependencies, and centralize them in one place via a settings mechanism. In this case, your dependency would be the PasswordSetter. – Robert Harvey Sep 19 '14 at 16:41
  • It would really help to see some in-depth talks/writings/etc. about it. – Jack Sep 19 '14 at 16:56
  • 3
    I'm probably not the right person to ask about that. I'm big on "sensible design," and I believe that IoC frameworks are overused. There are many different ways to approach this; I just happen to believe that composition works best when you do it through the constructor of an object, because you can validate that you have a properly constructed object before you begin operating on it. At the end of the day, you still need all those dependencies, so do what works best for you. – Robert Harvey Sep 19 '14 at 16:59
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.