Refactoring refactored code…
There is an article series with examples how to refactor one code piece. But something seemed not right with this code. Maybe it was because the code wasn’t refactored yet? But no, last version of that code still seemed not right.
So let’s start with the problems of that code. This is author’s controller:
If I am a new programmer in that company (or if the author comes to the same code after a year) will somebody remember how the code works? My favorite code to read is code that right away explains what it is doing.
What we see from author’s controller is only that we call some method that returns results. New programmer will need to dig dipper to understand what is happening.
I am suggesting doing this:
When viewing this code new programmer will know what his boss requirements were:
1. search API
2. log results to DB
3. log results to file
1. Question: where is dependency injection?
Dependency injection is mostly used for unit tests. Author didn’t mentioned that he tests his code. But event if he tested, check this example:
Does author know what his code returns? Probably not, he just dumps it into Database and hopes that it is correct. How can you use it later if you haven’t validated what you have saved?
So I am assuming that there are no unit tests. And for a new programmer it will be much easier to understand without all this indirection. Moreover you can easily add it back through constructor injection.
2. Question: What if I want to log every time API is called in other places?
Ah, good question, good question.
Instead of logging in controller, I would raise event in FlightApi class. Then DB logger and File logger would need to listen for that event and log the data.
Conclusion
“Everything should be as simple as possible but no simpler” — that is one of my favorite quotes.
So for simple code like authors example I would not use repositories and adapters. In my opinion static method calls are the way to go.
Of-course, if at some time in future code becomes more complex, you will need to add abstraction. But for now, I don’t think it is necessary.