Comment by shitgoose | Hacker News Reader

Comment by shitgoose | Hacker News Reader

Hilarious! Doing x=C.getX() is bad, but if it is hidden by numerous layers of libraries and monstrous config files, somehow it becomes acceptable. Out of site, out of mind. The fact that global scope bean is essentially a singleton, doesn't seem to bother architecturally inclined crowd - they are too busy admiring sound of their own voice pronouncing words "dependency injection", "mutability" and "coupling".

The top answer is a perfect example of what is wrong with IT today. It takes a working solution, declares it wrong and starts piling up classes and interfaces to solve a problem, that was never a problem in first place (OP never said that their singleton-based cache didn't work, he merely asked if there are "better" ways of doing it). So in the end we have the same singleton cache, but hidden behind interfaces ("It makes the code easier to read" - yea, right, easier, my ass! Ctrl+click on interface method and try to read the code), thousand lines xml Spring configs, and other crap that is completely irrelevant, hard to follow and debug, but glamorous enough for SOA boys to spend endless hours talking about it.

Doing x=C.get(x) is bad because there's no way of mocking C, thereby making unit testing a component that uses C impossible (the unit test will need to use the concrete implementation of C). Using dependency injection as described in the accepted answer, and particularly separating the concerns of C by using several different interfaces allows you to not only mock C, but actually create mock classes each of which is mocking one particular logical subset of C's functionalities.

It's easy to dismiss all this as "bloated" and "enterprisey" and people that use this as "architecture astronauts" but in reality, this pattern really does help. Well, at least if you want to be able to unit test your code properly. Otherwise, you might as well just use a singleton object.

I realized recently that the reason that there is no way of mocking C is because of a weakness in the language implementation/design. In Python, it's easy to mock and unit test globals, because in the test you can just overwrite them in the test's setup, and restore them in the test's teardown.

You could argue that it's a static vs dynamic typing thing, and I don't know enough to really dispute that, but it seems to me that there's no inherent reason that a static type system can't handle using a different concrete implementation. I think you can do this with Java/C# by doing some library loading trickery, but it'd be inconvenient.

And that's the whole point of all this, isn't it? Automated testing is a convenience that saves you from doing manual testing. Unit testing is a convenience that catches test failures before commit rather than during the automated testing phase. Creating the unit test, well, is it more convenient to create 8 interfaces and completely change the way the code is structured, or is it more convenient to mock out the singleton? It depends on the circumstance, I've done both in various languages. It's good to have the option.

It's a scoping thing that has no direct relation to static/dynamic typing, though probably has plenty relation through the underlying philosophies of many lanaguages which motivate them to choose particular typing and scoping rules.

Who says that you have to be able to mock C? Not every class should be mockable. In case described by OP, why would we want to mock cache anyways? In most cases singleton is used as a global state that can be easily accessed from all parts of the app without explicitly passing the state obj. We are dismissing a useful technique (singleton) based on requirement, that is not applicable to this technique.

> Who says that you have to be able to mock C? Not every class should be mockable. In case described by OP, why would we want to mock cache anyways?

Well it isn't really a unit test if you don't, is it? If you don't mock it then you're not only testing classes under unit tests, but also C's methods. And you're doing that not just in one place but in several different places in your testbase. The same methods. Over and over again.

I don't understand. If my unit test calls C.getX() over and over again, how is it bad?

In my case a cache could be something remote, having any calls directly to that will slow down unit tests a lot, having one or two calls to it unit testing the cache implementation it self is OK, but having every method which makes use of it make those calls isn't going to work.

In reply to your original question, its not extremely bad if you can't unit test that class alone , but in this case you wouldn't be able to unit test anything which uses it either.

C.getX() is going to be called by many different methods in many classes. This means that each time you unit test any of these methods, you're also testing C.getX() when you should only be testing what the method does.

+ sign and - sign are also called by unit tests many times, so you also repeatedly testing arithmetic operations "each time you unit test any of these methods, ... when you should only be testing what the method does".

Edit: I see what you say, but I think that effort we spend on decomposing app into perfectly isolated unit tests is far greater then the effort of identifying and troubleshooting where the less perfect test failed.

You want to mock cache so you can replace it with a trivial implementation that always contains precisely what the test expects it to contain.

Fill up the cache with mocks of the objects it needs to contain. 10 objects = 10 lines of code.

If you really want to have different cache implementation for testing purpose, use C as just an instance storage, and return reference not to self but to cache interface. Put a switch inside static C.getX() that returns IX. Inside of switch statement see if System.getProperty("isTest")=true and return XSimple (which implements IX), otherwise return XReal (which also implements IX). Few lines of code, transparent and debuggable.

"Fill up the cache with mocks of the objects it needs to contain. 10 objects = 10 lines of code."

But then I'm not testing the code that uses the cache, I'm testing the cache plus the code that uses the cache. Which is a valuable test but a separate one.

Regarding your proposed implementation, that sounds like basically what I proposed here:

https://news.ycombinator.com/item?id=7153126

Funny, I saw your code, but didn't put your name and code together:) There are many ways to mock a singleton. I am not against mocking it, but against over-complicating. Also, I think that we are taking unit testing a bit too far in trying to decompose the app into smallest pieces. Unit tests have specific goals, like verifying correctness of calculations or performance benchmarks. If test raises a red flag, it takes a few minutes to isolate the piece of code that is at fault.

I might be speaking blasphemy here, but is the advantages of unit testing worth the increased code complexity?

In quite a few codebases I've seen, the amount of tests was entirely unrelated to how stable and maintainable the system was. Some times developers just fixed the tests, rather than the bugs. And in many cases, there were a slew of failing tests even though the system ran fine.

Strictly, you could mock C by providing a static "replace the singleton" method, right?

    class SomeResource : public Singleton<SomeResource> {
        private:
            SomeResource *instance
  
        public:
            SomeResource &Instance() {
                if(!resource) makeResource();
                return *resource;
            }

            void Mock(SomeResource *mock) {
                resource = mock
            }
    }

    class MockResource : SomeResource {
        ...
    }

Sure, but:

a) Now your singleton is also responsible for providing a mock implementation of itself (which shouldn't really be one of its concerns).

b) You didn't really do anything here: the class under test is still going to use the concrete implementation of C. You need a way of providing a mock implementation to your class while testing. The obvious answer to how to do it is dependency injection. But if you're going down that route, you can just ditch the whole singleton thing, instantiate a class using "new" somewhere else and pass it to your class. And in your unit test, you simply pass the mock implementation. This is what DI containers like Spring do. We'd be doing the exact same thing here, only manually.

a) True, but it can probably be pushed into the parameterized Singleton class. I didn't try, since that might get more language dependent and since I wanted to keep things explicit-but-short for demonstration purposes anyway.

b) You're touching a tiny part of the implementation of the real C, yes. If you're able to push the Instance and Mock functions into Singleton, you might be touching literally no code of C other than relying on the fact that it in-fact inherits from Singleton.

I nonetheless agree dependency injection is a better solution. The biggest objection I have to this implementation is that there's nothing that tells me I should be mocking C, or catches it when I forget to mock C.

Not only can you mock it for testing but it encapsulates the system better, there is no reason for straight access to C.get with this pattern you can easily drop in a brand new system for caching and not break your original code. Some might find that a bit too much like coding for the future, but in my case that is quite a likely situation to occur especially if you ever run into scaling issues.

I've never really understood the point of most of the Spring usage I've seen; seems like most people want to replace Java code that is type-checked and can throw exceptions at reasonable places when something goes wrong with XML 'code' that has neither of these properties and thus turns debugging into a black art. Apparently, this is the right way to do things.

The only reasonable use case I've encountered in practice is the ability to replace some major components of your program with stubs for use in automated testing.

The "makes code easier to read" thing is easier if you have a certain style of programming. Compare (a trivial example):

    bool isCommentSpam(int id, ICommentCache commentCache) {
        return commentCache.getCommentById(id).getScore() > THRESHOLD;
    }

    bool isCommentSpam(int id) {
        return Cache.getCommentCache().getCommentById(id).getScore() > THRESHOLD:
    }

In the former, you make the fact that the method uses the comment cache explicit. If you were to have another method that deleted a video (and all its associated comments), it would have to ask for a comment cache explicitly in its signature. This would make the fact that these two methods interact with each other explicit and make reasoning easier.

This is perhaps less important than the ease of testing, etc. that you get as well, but I find that I work better with this style.

I am not sure how first case is more explicit. Try Ctrl+click (say we are in Eclipse world) on getCommentById(...) in first case and you will see interface signature - bummer. And while you will be pressing Ctrl+Shift+G and trying to guess (just guess because only Spring knows for sure) which implementation is more likely to be used on this particular line, I will jump straight to getCommentCache(...) implementation (2nd example, non-interface) and move miles ahead.

EDIT: Not to mention, that passing references to cache object along each method call bloats the code. Now you have references to cache not in 50 cases where cache is used (example #2), but in 5000 cases because each method signature needs to include reference to cache (each method along the call hierarchy until you reach the point where you use cache object).

That's not what I mean. When you see an instance of explicit `isCommentSpam` used, you know immediately that it is using the CommentCache. Compare:

    if (isCommentSpam(423)) {
        doSomething();
    }

    if(isCommentSpam(423, commentCache)) {
        doSomething();
    }

See, the fact that the first kind uses the CommentCache is completely opaque to you when you're reading the code. You don't know it does until you actually look inside `isCommentSpam`. The second way, you can trace the flow of information through the program very easily.

Well, I see what you are saying, but I still don't think this is a strong case against singletons.

Spring is not the only way to do DI.

It's perfectly feasible to instrument your app at the top-level to pass the needed components down, and then use them through interfaces. Thus, you pay the cost of the interfaces, plus the cognitive cost of using those interfaces, instead of implementation classes. Same effect as DI, but no container.

Aside: there are costs and benefits to a good software architecture, but you won't learn it from reading HN.

It does add complexity, but it's a trade-off. Sometimes you need something to be easily testable, and this pattern can be worth the complexity it brings, if done minimally and properly (not blindly). Obviously we all prefer simplicity, but sometimes complexity is a necessary evil. (But, I would argue, not nearly as often as some people think.)

Reply on news.ycombinator.com