I'm working on a simple class to get a token from an API. As simple as it is, it has different aspects.

  • A connection to the API is needed for this to work at least the first time. Credentials are stored in the code (security is not the subject).
  • A token should not be fetched from the API more than once every 9 hours, thus it should be saved in a file when successfully fetched.
  • If the file is too old, it needs to be generated again.

Now I'm thinking about doing four classes :

  • File : responsible for creating, overwriting and returning the file.
  • Parser : responsible for parsing a received file.
  • DateCheck : responsible for checking whether the token is too old or not
  • Connection : responsible for fetching the token through the API if needed

Now all these classes are fine (what do you think ?), but I obviously need a higher level class at some point to wrap all this logic and get the token with one simple call to this class.

Firstly there will be 4 dependencies, isn't that too much ? Also, it will actually do many things and the reason why it is troublesome to me is because the unit tests are pretty big (15-30 lines) with all the mocking required, and most tests in successful projects code I've read are very small and simple to understand.

Is there something wrong with this reasoning, or is that the downfall the splitting up things that much ?


You seem to be making classic OOP design error : you are focusing on "things" instead of "behavior". In your case, there are three behaviors : Fetching the token, Caching the token and Persisting the token. There is also one nice abstraction : Because client shouldn't care where or how the token is received, it can use Fetching and Caching interchangeably.

So the design would look something like this : enter image description here

This way, client only uses the interface and doesn't care if token is cached or not. Also, you can introduce different ways how to fetch the token and still have caching available, because caching is just a decorator. I would also argue that there is no need for classes as you designed them, and just put the code inside the classes as in my design.

And if you want to get really fancy, you could introduce another abstraction for persisting the token, but that will needlesly complicate the design if you only have single persistence mode.

| improve this answer | |
  • Thanks for the answer. I don't quite understand the difference between Caching and Persisting in your example. Also, does FetchTokenSource persist the Token itself, or does it delegate this behavior to CacheTokenSource ? – Steve Chamaillard Oct 5 '16 at 10:47
  • 1
    @SteveChamaillard Fetch only fetches and immediately returns the token. The Cache wraps around another token source and caches the value it returns. I created Persistence as class that has file saving and parsing logic and Caching as just the timing behavior. They could be part of single class or they could be separate. – Euphoric Oct 5 '16 at 10:51
  • 1
    I'd say that having a TokenfilePersistence is in fact not the cleanest design choice here. Go with a FilePersistence class instead. It should not know about a token, but should just store data into a file. The same goes for CacheTokenSource. I'd say you should have Cache, which does not know about what it is caching. Also I don't think sharing an interface here is a good solution. In fact you should expose only a single method in a single class (as it may take time to get the token), that gets the token. The other classes involved should not be exposes beyond that one class. – Jonathan van de Veen Oct 5 '16 at 11:14
  • @Euphoric this makes a lot of sense. In the most basic way, there'd be only Persistence and Fetching. This is very simple and feels right. Is having a class using those two classes fine then ? – Steve Chamaillard Oct 5 '16 at 11:54
  • 1
    @SteveChamaillard That would be some form of startup configuration / dependency injection / factory logic. It would initialize proper way to "query for token" and then pass that to it's user. – Euphoric Oct 5 '16 at 12:34

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.