Hacker News new | ask | show | jobs
by monadmoproblems 1480 days ago
Just on your point on Testability, I don’t believe this is the issue you make it out to be: my standard approach is to make all the types/methods I need to test ‘internal’ and then expose them to my unit test assembly using this assembly level attribute, https://docs.microsoft.com/en-us/dotnet/api/system.runtime.c...
1 comments

I don't understand this logic. Test things that are public. You made them public for a reason. Those are what your class says it will do. Confirm it does what it says it will do and you're done with testing.

I'm not sure I've ever used the "internal" keyword in 12+ years of C#. Assemblies are about deployment; public/private is about the contract your code is agreeing to. They have nothing to do with each other, so internal is an inappropriate conflation of concepts, by definition.

Because all of our projects use Microsoft.Extensions.DependencyInjection, I make all our service classes internal

The way other projects can access these services is by using the project's helper to register them all in the DI container by their interface. That way I can be sure no one is directly using the classes and make breaking changes to the constructors (like swapping loggers or api clients), but still access them from tests.

And if you have two different DLLs that offer this pattern, but they overlap in the interfaces they register? The downstream consumer has no way of saying "I want my IFoo to be a FooA from assembly A, and my IBar to be a BarB from assembly B". And this may depend on a configuration setting! Real example: we have a bunch of useful Azure functionality in one DLL and a bunch of general useful web functionality in another; they implement many common interfaces and are alternative services you can pick depending on your deployment. But it's not as simple as "when on Azure, pick all the Azure stuff" -- it depends on which Azure services you've configured for that individual project.

If they overwrite each other, you might be able to get away with calling the god-registration-functions in the correct order (which you can determine only by reading the source code of the two functions, which are probably in different solutions), but if you really have a well mixed set of needs from A and B, this is probably not possible.

So you say: "okay, well, in this case maybe those concrete services should be public instead of internal. Obviously in this case we'd go back and do that." So your logic is: use "internal", except when it turns out we shouldn't have used it, then don't. So what purpose was it serving to begin with? None. The last lesson is that if you find yourself saying "in this case", you're actually talking about every case. That is to say: "special cases" are a symptom of a weak mental model. Good models don't have special cases.

So indeed we've identified several ways to improve, all stemming from the fact that you are using the "internal" keyword!

Wow, there really are people who just drive-by downvote anything that doesn't a-priori agree with their cargo cult mentality instead of stopping to think about it for 30 seconds? Of course they can't leave a comment, because that would require them to actually consider my point, which would put them in danger of actually learning something.

Downvoter, the problem with your downvote is that I am correct. DLL dependency/deployment is a completely separate concern from code contracts and visibility; therefore "internal" is always a mistake and should never have been added to the language. If you use "internal", we've identified that you have gaps in your understanding of software development that you have the opportunity to fill. But instead, go ahead and just downvote me again without comment. That's a lot easier than improving.