Hacker News Reader

Comment by commandlinefan | Hacker News Reader

I'd be happy if we could just get to the point where programmers wrote code that could be unit tested. i.e. don't write static initializers that connect to live databases for a start.

View on HN

Dependency Injection + Mockito allows one to do that. There's a whole study on writing code that's easy tested. Our rule is no more than 4 layers: initiator, business logic, services, anemic domain model. Initiators abstract away the inbound protocol and serialize to the common model. Business logic controller handle all of the branching. Services effectuate single actions. Services can't call other services. And the domain model is how everything talks. We all build our apps this way and it's really easy to move people between projects. Not perfect but works for about 85% of stuff one has to write.

I'm not sure calling the domain layer "anemic" by default is correct, as it's typically a (negative) descriptions of models which are too data-driven instead of behavior-driven. I would suggest an alternative layer structure: Initiators/Controllers -> Application/Domain/Infrastructure Services -> Domain Models/Business Rules/Invariant Checking

We've thought about that; the style I've described is very non-OO. It is easy to teach though and it makes unit testing a breeze.

I like the architecture you've described. I think we arrived at where did because it's a natural for when using a DI container, which manages state and transaction groups for you.

is database access only in service?

Correct. A business logic controller can not directly corner the outside world, it must talk through a service.

This is nice because it prevents leaky abstractions into the controller layer. Actions across services that need to be atomic can be grouped in an XA transaction:

// Xa start databaseService.reset password(user); emailService.notifyPasswordChange(user); // Xa commit

Haven't seen anything that bad in ages, but yeah... I'd lose my cool if I saw that.

Last bad thing I saw was - quite recently actually - some business code in a request handler mutating state on a controller field (which are singletons.) Lots of fun once we started load testing with concurrent sessions...

I'm kinda confused about that one. Because if the value isn't coming from some external source like a db, api, file, whatever then why do you even need a static initalizer? If you mean they hardcoded the prod database then I'm so sorry.

They hardcode the code that reads a config file and gets the host name of the DB and then connects to the DB. This initializes a singleton object that’s accessed by nearly every file in the rest of the system. They do this for every external source, not just databases. In my experience, this is the most common way Java “developers” design systems, and they usually get angry with me if I try to fix it because their way is “faster”.

I've run into this type of thing too, it's soul-sucking because it's so easy to avoid. I wouldn't say I do TDD per se, but I definitely write tests for many things to prove to myself that it's working. Many devs just build + run the code and poke at it, which enables all types of heinous patterns like this (and in my opinion is a super inefficient way to code).

So why is this design bad? It seems like when you actually start running into scaling issues that singleton translates naturally into borrowing from a shared connection pool.

It's much more ergonomic, flexible, testable, and configurable to inject that connection pool thing instead of bodging it together in the static initializer. Classes shouldn't be managing resources of which they are properly only consumer...

... because nothing can be mocked out.

Do you not just mock the singleton? I mean don't get me wrong, it has all the usual global variable downsides but I don't see too much a meaningful difference between every method passing around the same connection pool handle explicitly vs ambiently. And either you can mock the connection objects the singleton returns or you can't.

In most apps I've seen "services" like Redis/Rabbit/Memcache/Postgres/External APIs/Storage are cross-cutting concerns and you make your life a nightmare by having to pass

    myfunc(actual, params, db, redis, memcache, rabbit,...)

because if you realize deep in your call stack you actually need data from Postgres now you have change all the callers recursively to pass the handle down. If you have this global "service catalog" it's almost always to eliminate the need for passing the same effectively global connection pools to hundreds of call-sites.

It is annoying that it makes every test require a bunch of ambient service mocks but you only really have to set it up once.

Yeah, sure, if you declare every variable global, it makes adding new features feel quick and easy. There's a reason that global variables are considered harmful.

I completely agree with you! But we're talking about one of the few exceptions -- cross-cutting concerns.

I think just about everyone would scoff at the idea that every single method in your codebase should take an explicit `logger` parameter rather than just having a global logger object.

So now we're looking as a case where you have a bunch of services: dbs, caches, apis, storage that are used all over the app ("in every file") and you have to make a judgement call.

* Have hundreds (honestly thousands at current $dayjob) of methods that do a lot of work just to pass around the same connection handle.

* Declare a single object that manages all the connection pools.

I think it's really hard to escape the fact that connection pools are actually global and you either have to admit this or have your runtime hide it from you.

Another example of a cross-cutting concern that runtimes usually hide is event loops. Can you imagine if every function that wanted to use async had to be passed an event_loop variable?

Static methods in general that create side-effects suck. PowerMock allows you to deal with these troublesome classes though so unit testing is possible.

Yes, surely it's possible. But the resultant test is highly tightly coupled to the implementation of the SUT which gives the test more than one reason to change.

We should strive generally to have tests that only change if the business requirements change. But if I want to refactor my unit (whatever that might be) then the test should not change, or at least should not change __much__

Where the hell are you working where people think that's ok? I haven't seen anyone defending that for 10+ years.

Reply on news.ycombinator.com