5 ms·
Perhaps the problem isn't mocking dependencies, but trying to hide the fact that GetSolarTimesAsync needs two pieces of data to work: a date and a location. Bu
by DougBTX 6y ago
Perhaps the problem isn't mocking dependencies, but trying to hide the fact that GetSolarTimesAsync needs two pieces of data to work: a date and a location.
But the original signature is just this:
public async Task<SolarTimes> GetSolarTimesAsync(DateTimeOffset date)
That introduces a lot of complexity:
* The SolarCalculator needs to be able to work out its own location, so it needs a LocationProvider
* SolarCalculator needs to be IDiposable since it owns a LocationProvider
* The SolarCalculator will need more methods if it ever needs to calculate the times in a different location
* If fetching the location is slow, but the application needs to calculate times for multiple dates (eg to build up a table of times), then the SolarCalculator will need an method that takes in an array of dates to be efficient
But all that could be solved by making the function take all of the arguments it needs to return its value:
public SolarTimes GetSolarTimes(DateTimeOffset date, Location location)
No location provider needed, no IDiposable, just one efficient stand-alone method.
Unit testing this is now just:
var calculator = new SolarCalculator();
var actual = calculator.GetSolarTimes(new Date(...), new Location(...));
var expected = new SolarTimes(...);
actual.Should().BeEquivalentTo(expected);
...so, perhaps the issue isn't that unit testing is a bad idea, but that code which is hard to use in a unit test might also be hard to use in a wider application? And perhaps the fix is to make the code easier to use?
- chrisandchris 6y agoI agree with you and that‘s most times also my observation: Unit tests tend to show how decoupled and re-usable your code is. Is a function gets hard to test with a unit test, this points usually towards an architectural issue.
- fernandotakai 6y agoif i got a dollar every time i started testing a piece of code, realized that it was waaaay to complex, refactored the code so the tests were easier to write/understand... i would have a lot of dollars. 100% of the time, it was the right idea and the code became a lot better.
- romwell 6y agoDidn't expect to scroll this far to find this. TL;DR: GetSolarTimes(Location, Date) is a unit-testable function. Had some thought been put into writing with unit tests, there would be no problems with that example.
- andycunn 6y agothat feels pretty dismissive / reductive to me... seems like you're all saying that var actual = calculator.GetSolarTimes(new Date(...), new Location(...)); is JUST SIMPLER and better than public async Task<SolarTimes> GetSolarTimesAsync(DateTimeOffset date) with an internal location provider as a dependency because it's easier to test. but i think that ignores the reason why DI containers were invented in the first place and assumes that the solar calculator is just a simple entrypoint-type application, rather than being a component in real application. You might have 20 layers of THING, somewhere inside which, this solar time calculator lives and is used... and you still have to get Location from SOMEWHERE to pass it into the calculator. so what happens when Whatever uses the location provider to get the location and pass it along needs to be tested? and through how many layers of stack do you need to pass Location before you realize that every test of every intermediate layer needs to know about location, but only for the purpose of passing it along? I think it's a more nuanced case than you're making it seem. Beyond some level of complexity in an application, it becomes simpler to co-locate dependencies where they're actually used.
- leoedin 6y agoCompletely agree. The author is trying to blame tests for code complexity. GetSolarTimes() is a simple function evaluating an equation - treat it as such. If your code is broken down clearly into logic and plumbing, unit testing the logic becomes super easy. It allows you to construct software using blocks you have absolute confidence in. Unit testing plumbing is harder, and that's when integration testing shines.
- underdeserver 6y agoExactly. The author's tests are overly complex. Instead of gleaning the actual value of this insight, which is that you're not cleanly separating your inputs and your outputs, the author concludes that unit tests are a waste of time. Nope. Unit tests are a tool, but writing proper unit tests and understanding the value they give you is an art and a science. It requires experience and deliberate design.
- robalni 6y agoI thought about this too when I read the article but then I thought that it might not always be possible to rewrite the code like that in other cases. Maybe the example used in this article was just not the best example.
- dorianh_ 6y agoExactly! Thank you! It seems that most devs (me included) learn at school to write pure functions, which is great. Then they come to the industry and all of the sudden the "parseXml" function takes a ftp port as a parameter... ("be in my case the xml was on a ftp server!") Why there is no CS course that explains this kind of stuff?
- monocularvision 6y agoMentioned elsewhere but that is largely the heart of Functional Core, Imperative Shell: https://www.destroyallsoftware.com/screencasts/catalog/functional-core-imperative-shell https://www.destroyallsoftware.com/screencasts/catalog/funct... (And I am sure a bunch of other similar but differently-named concepts)
- aakresearch 6y agoThe author puts forward exactly same point, somewhat closer to the conclusion of the article: public SolarTimes GetSolarTimes(Location location, DateTimeOffset date)