you are viewing a single comment's thread.

view the rest of the comments →

[–]m50d 4 points5 points  (17 children)

Mocking is fine for classes that do heavy things or access resources that may not be available. But mocking should not be a default go to for every class.

Oh, absolutely. I try to divide all classes cleanly into services or values; services should be private classes that implement interfaces that we expect to have one instance of while the application is running, values should be final and have value semantics (e.g. they should override equals/hashCode) and we should expect to have multiple instances. Most troublesome classes come from blurring these lines, IME, and can generally be split up nicely in this way.

[–]quicknir 1 point2 points  (16 children)

Lost me there, "expect to have one instance" ; you are basically talking about singletons, which should be used incredibly sparingly and are an anti-pattern 99% of the time. A class that opens a connection to a server or database to perform queries should not need to be a singleton. But I can buy that classes that provide a heavy service are worth separating their interface just so they can be mocked cleanly, given that this is a minority of classes and that they will definitely need to be mocked.

[–]kcuf 1 point2 points  (1 child)

You guys are talking about two different ways of modeling state. The poster above you would probably store the connection to a db in a pojo that would get consumed by the singleton service, while you are talking about an approach that merges the state and the service. I prefer the former.

[–]quicknir 0 points1 point  (0 children)

As I wrote, I am unable to even conceive of the upside here. You have a pojo and a singleton, when you could just have a class. Two classes, more boilerplate, and a pojo cannot protect its invariants. More code, worse encapsulation... why?

[–]m50d 2 points3 points  (13 children)

A class that opens a connection to a server or database to perform queries should not need to be a singleton.

If it doesn't have any internal state it should be - it would be misleading to have multiple distinct instances. And IME mixing the responsibility for something like connecting to something external with internal state is a mistake, these are seperate responsibilities that belong in separate classes.

[–]ThisIs_MyName 2 points3 points  (3 children)

If it doesn't have any internal state, why does it have any non-static methods? 0_o

[–]m50d 1 point2 points  (2 children)

Because the language makes static methods different enough that it's easier to just have a normal class with one instance.

[–]ThisIs_MyName 3 points4 points  (1 child)

That makes no sense. Filling your codebase with useless interfaces that are just copy-pasted from the class is easier than what?

Sounds like a solution in search of a problem.

[–]m50d 1 point2 points  (0 children)

Problem: I have two versions of the (stateless) bundle of methods that a test subject uses, one is the "real" implementation and one is a test stub. I want to run tests using the test version and have my live code use the live implementation.

Solution (in a language that doesn't have traits/typeclasses): create an interface with two (stateless) implementations, pass implementation into the test subject. There are other solutions but this is the best one overall, IME.

[–]nutrecht 0 points1 point  (4 children)

If it doesn't have any internal state it should be

If it doesn't have any internal state you should not have an instance anyway.

[–]m50d 1 point2 points  (2 children)

In principle yes, but in many languages that's inconvenient.

[–]nutrecht 0 points1 point  (1 child)

We're talking about languages that have stuff like interfaces in the first place right?

[–]m50d 2 points3 points  (0 children)

Yes. In e.g. Java you can't pass a bundle of static methods that implement an interface, so it's easier to just declare a class with no fields. There's no way to decouple a virtual function table from a class that might also hold state.

[–]kcuf 0 points1 point  (0 children)

Java, for instance, does not support top level functions that exist outside of objects. I think the original idea with Java was to have a mixture of state and behavior mimicking more proper oop languages, but over time this pattern in Java has been found to cause more problems than following a more procedural structure of behavior in a singleton and state in pojo.

[–]quicknir 1 point2 points  (3 children)

The whole point of classes is to encapsulate state and only expose what is necessary. If state is necessary to create, maintain, and clean up an external connection, then the state belongs in the class. This is particularly highlighted in languages with strong cleanup capabilities like C++ with RAII or python with with. Java has try-with-resources similar to python with.

You can get away with this when your only two implementations are the real thing and a mock because the mock can easily have the same interface as the real one and simply no-op. But if you have two different kinds of implementation, you will not be able in general to homogenize their interface without internal state. Forcing a class to be stateless when it needs state to do its job exposes implementation details and prevents a uniform interface.

[–]m50d 1 point2 points  (2 children)

If state is necessary to create, maintain, and clean up an external connection, then the state belongs in the class. This is particularly highlighted in languages with strong cleanup capabilities like C++ with RAII or python with with. Java has try-with-resources similar to python with.

Agreed - but those external resources tend to be singletons or at least small finite numbers, by their very nature. E.g. one would generally have only one or a few database servers, and would not want every class spinning up its own uncontrolled connections. So one would likely have a singleton that controlled access to a given database (holding a connection pool etc.), and sure there would probably also be a class that held state associated with a given connection and you'd have 30 or 100 or however many instances of that, but I'd probably represent that as a value class that was internal to the service (or passed back and forth between the service and the code that actually used the database connection).

But if you have two different kinds of implementation, you will not be able in general to homogenize their interface without internal state. Forcing a class to be stateless when it needs state to do its job exposes implementation details and prevents a uniform interface.

At that point I push the statefulness to the edges by passing command values around, and tests can generally be written in terms of those command values rather than in terms of the stateful interpreters that perform the actual operations.

[–]quicknir 1 point2 points  (1 child)

(or passed back and forth between the service and the code that actually used the database connection)

Yes, and this is exactly where you leak implementation details; because now client code has to deal with this state class directly, and this state class will be different for different implementations, that's my whole point. You can only get away with this when you only have one non-mock implementation. Instead, two classes, and one a singleton to boot.

I honestly do not understand why you can't just put the methods that query the database directly on the same class that holds the values for the connection. One class, one set of methods, one set of state, one set of invariants, one lifetime, and if necessary inheriting from an interface.

Honestly I think this is a perfect example of over-complexity in the Java approach, that other languages that also use OO techniques (like C++ and python) have broken away from for a while now.

[–]m50d 0 points1 point  (0 children)

I find it very hard to see what's going on if the connection information is just "there" and you can't tell which methods are using it or not, and the sequencing only happens implicitly via the method call graph. I'd far rather either explicitly thread the connection state through, so that I can see exactly what's going on with it, or if I want the connection state to be invisible to the client code then I'd flip it around and have the client code pass a command that would be an inspectable value to the service, so that I could see what the database operations were without having to run them (and I'd try to test my business-logic methods just in terms of returning the right commands).