6 ms·
> Redbox.HAL.Configuration > .ConfigurationFileService implements IConfigurationFileService > STOP MAKING SERVICES AND FACTORIES AND INTERFACES AND JUST READ
by xnorswap 2y ago
> Redbox.HAL.Configuration
> .ConfigurationFileService implements IConfigurationFileService
> STOP MAKING SERVICES AND FACTORIES AND INTERFACES AND JUST READ THE FUCKING
> JSON FILE YOU ENTERPRISE FUCKERS
I know it's cool to "hate" on OO, but "just read the fucking file" doesn't work if you want to run your unit tests without reading a fucking file.
It makes sense to abstract configuration behind an interface so you can easily mock it our or implement it differently for unit testing.
Perhaps you also want to have some services configured through a database instead.
This isn't a ConfigurationFileServiceFactoryFactory.
- rkachowski 2y agomost of these issues disappear with the introduction of first class functions. There's nothing noble about the thick indirection inherent in old school enterprise programming.
- gpderetta 2y agoA fist class function is just an interface. Just ask the venerable master Qc Na.
- xnorswap 2y agoCould or should there just be a `IConfigurationService` instead of a separate IConfigurationFileService? Yes, probably. "Interface all the things" is a bit lazy, but it's easy, especially if you have Moq as a way to auto-mock interfaces and a DI framework to setup factory methods. But spinning into rage just because you see an interface or abstract factory isn't healthy.
- throwaway365x2 2y agoI don't follow .Net closely, but it seems like there should be a better alternative. Java has a library called "Mockito" that can mock classes directly without requiring an interface. I assume something similar exists for .Net, as they have similar capabilities. Making an interface for one class, just so another class can be tested seems like we allow the tool (tests) to determine the architecture of what it is testing. Adding complexity in the name of TDD is a close second on my list of triggers There's nothing that triggers* me more than seeing an interface that only has one implementation. That's a huge code smell and often a result of pre-mature architecture design in my opinion. It also often leads to complexity where if you have an interface, you create a factory class/method to instantiate a "default" implementation. Fortunately it seems that it is not used as often as before. Our code has no factories and only a few interfaces, that actually have a practical use. The same applied to my previous workplace * The trigger applies to 2024 Java code written as if it was 2004. I may have a form of PTSD after many years of interfaces and FactoryFactory, but fortunately times have changed. I don't see much of that today except in legacy systems/organizations.
- xnorswap 2y agoI'm sure the same exists for .NET ( Moq can probably do it? ), but writing against an interface and having concrete implementations supplied by the DI framework is pretty much the ordained way to do things in .NET. I used to be in the "Interfaces with only a single implementation is a code smell" camp, but I prefer to follow the principle of least surprise, so going with the flow and following the way the MS standards want you to do things makes it easier to onboard developers and get people up to speed with your code base. Save "Do it your own way" for those parts of the system that really requires it. And technically the auto-generated mock is a second implementation, even if you never see it.
- throwaway365x2 2y agoI think you have good approach. I also tend to go with the flow and follow the common practice. If I tried to do "Java in C#", it would make it more difficult to follow my code and decrease maintainability. I sometimes work on legacy C# code that we inherited from another team and I try to follow the style as close as possible. I just haven't invested enough time to make any informed decisions about how things should be.
- Jerrrrrrry 2y agoYou are both over thinking it. GIT? unit tests? and i thought debuggers spoiled us? although cavemen-esque in comparison to 'modernity'; it wasn't a nightmare to Pause/resume program flow and carefully distill every suspected-erroneous call to Console.Log(e)/stdout/IO/alert(e)/WriteLine(e); `everything to find the fun/troublesome bits of one's program - instead of a tedious labyrinth of stack traces obfuscating out any useful information, further insulted by nearly un-googable compiler errors. Tests were commented out functional calls with mock data. If you never need to instantiate another instance of a structure so much so that it would benefit from an explicit schema for its use - whether it be an object or class inheritance or prototype chain - then sure, optimize it into a byte array, or even a proper Object/struct. But if it exists / is instantiated once/twice, it is likely to be best optimized as raw variables - short-cutting OOP and it's innate inheritance chain would be wise, as well as limiting possibly OOP overhead, such as garbage collection. >interface in C# Coincidentally, that is where my patience for abstraction for C# had finally diminished. yield and generators gave off awkward syntatic-over-carmelized sugar smell as well - I saw the need, to compliment namespaces/access modifiers, but felt like a small tailored class would always outweigh the negligible time-save.
- gregmac 2y agoYeah, IConfigurationService implies separation of concern. Code using it doesn't have to care where the configuration came from, just that it is there. Someone separately can write the concrete ConfigurationFileService:IConfigurationService that reads/parses files. IConfigurationFileService implies abstraction of file system-based configuration. Are we planning that there's going to be a different way to read configuration files in the future, and what exactly is that? If no one can articulate it, it just seems like architecture astronautism and: YAGNI. IConfigurationService makes writing unit tests for anything that uses it way easier, too. There can be a simple TestConfigurationService:IConfigurationService that just implements everything as settable, and in your test code you can provide exactly the properties you need (and nothing more), and easily have 100 variations of configs to ensure your code is working. Without the headache of dealing with actual files separate from your test code, or worse, shared with other test code. I've actually written multiple long-lived pieces of software this way, and more than once ended up implementing stuff like environment variable-based configuration, REST API-sourced configuration, and even aggregations that combine multiple sources, eg: new AggregateConfig(new ServerConfig("https://whatever"), new EnvironmentConfig(), new FileConfig("/some/path.config")); All that code that used IConfigurationService is completely untouched and unaware of any of this, letting whoever is doing this as part of changing deployment (or whatever) be productive quickly with very little knowledge of the rest of the (possibly massive) app.
- guax 2y agoWhy do you need the interface? You can extend/mock the class itself. Refactoring code is easy and cheap. There is no reason for complex abstractions that protect implantation outside of libraries and frameworks.
- jjice 2y ago> You can extend/mock the class itself. Refactoring code is easy and cheap. There is no reason for complex abstractions that protect implantation outside of libraries and frameworks. "Mock" can be a loaded word in this context, so please excuse me if I'm looking at it through a difference lens, but if you're using some sort of mocking set of tooling (like Jest or similar), I'd argue that those mocks are much more confusing than an interface with an implementation. I personally love an interface because it defines the most narrow set of operations an object needs to support to be passed in, and the implementation of those are completely irrelevant for calling. In many cases, I personally find that a lot simpler and cleaner to read.
- proikan 2y agoIsn't ```dependency injection``` (aka passing arguments) the big thing that's supposed to solve this? Config config; // production config_from_file(&config, "config.json"); run_production_stuff(&config); // unit tests Config config; config_from_memory(&config, &some_test_values); run_tests(&config);
- xnorswap 2y agoYes, and the typical pattern for .NET DI is to do so with interface based parameters. So let's say you have a service FooService that requires some configuration. ( Ignoring the System.configuration namespace for now) You'd have: class FooService(IConfigurationService ConfigurationService){ // Access Configuration Through IConfigurationService } Then elsewhere you'd set up your DI framework to inject your ConfigFileService to satisfy IConfigurationService in prod. Yes, it can sometimes feel a bit like "turtles all the way down", where sometimes you just wish you had a bunch of concrete implementations. In unit tests, you'd auto-mock IConfigurationService. For integration tests you might provide a different concrete resolution. There are some advantages to service based DI though. The standard ASP.NET DI framework makes it trivially easy to configure it as a singleton, or per-request-lifetime, or per-instantiation, without having to manually implement singleton patterns. This gives you good control over service lifetime.
- xnorswap 2y agoMy example above is terrible, because in reality you'd have another level before this, which sorts out your global configuration, reads it and just injects service specific parameters and configuration for each service. But I just wanted to illustrate idiomatic .NET DI, and on reflection picking configuration was probably the worst way to illustrate it.
- abrookewood 2y agoKind of off topic, but can someone explain why else C# has factories and interfaces? Is it just mocking? I really don't understand the pattern at all. FWIW I am no dev. EDIT: Found xnorswap's comment below about configuration, which makes sense I get - but as they mentioned, it does feel like "turtles all the way down".
- FroshKiller 2y agoA non-mocking use of mine: I have a factory with a method that returns instances of a particular interface for publishing typed events to a pub/sub service. The caller of the factory doesn't have to be updated with new concrete types as new events are added, because it's the factory's responsibility to create the events. The event types themselves just implement the interface that's required to serialize them for the pub/sub service.
- criddell 2y agoI've used them in the past to keep interface and implementation separate. It's an easy way to stick an adapter between something concrete and the thing that needs something but doesn't care where it's coming from. So, for example, I could have a IGadgetStore with methods for creating, retrieving, updating, and deleting gadget instances and then I can have a bunch of different classes implementing that interface. An obvious example is to have a PostgresGadgetStore and a MysqlGadgetstore and CsvFileGadgetStore. If the user wants to implement their own store that I haven't written, they can.
- abrookewood 2y agoOK that makes sense. As an outsider, the C# code bases I look at seem to do this as standard practice, even if the requirement for different classes never materialises. I guess you get used to looking at it, but it seems (perhaps naively) as wasteful and a bit distracting.
- neonsunset 2y ago> it seems (perhaps naively) as wasteful and a bit distracting. It is. Nowadays, .NET is usually able to do away with the abstraction cost of such interface abuse luckily, but it remains an additional item you mentally have to deal with, which isn't good. Single-implementation interfaces are still considered an anti-pattern, and teams that over-abstract and mock everything out when writing unit tests usually just waste time in pursuit of multi-decade old cargo cult. These also often test that modules comprise of specific implementations and not whether they simply satisfy the interface contract, which is terrible. And often turn stateless parts of logic that could have lived on some static class into an interface and an implementation injected with DI, that is then mocked out, instead of just calling methods on a class. More difficult to remove, worse locality of behavior, does not answer the question "if tests are green, are we confident this will work in prod?", sadness all around. I agree with your sentiment. It's much more practical to write functional and component-level tests with coarser granularity of individual test items, but with more extensive coverage of component inputs. There's a wealth of choices for doing this with little effort (e.g. testcontainers).
- rcxdude 2y agoBut why is it so hard to read a file during a unit test? Files are pretty easy to mock in many different ways, all of which are pretty fast. You don't need a special-purpose interface to be able to test the code that uses a config file.
- wobblyasp 2y agoIt's not, but maybe I don't want to create a file for the tests. The point theyre trying to make is that it's a personal preference and not an obvious "this way is better"
- stickfigure 2y agoLet's say you want to test bootstrapping your system with various configurations. You could make a few dozen different configuration files. Or maybe it's more than that because you want to test permutations. Now you're maintaining a bestiary. So instead you think "I'll write code that generates the config file for each test". And that's reasonable sometimes. On the other hand, the single-responsibility principle can be reasonably applied here and "reading the config data" is a good single responsibility. You don't want to have to write code to muck with json or xml every time some component needs a configuration value. So there should already be a clean boundary here and it often makes sense to test the components separately. There's not one rule. The article author sounds like an excitable jr engineer that's never written software at scale.
- dogleash 2y ago> that's never written software at scale. Is this like a never version of that insult where people would say someone's opinion doesn't matter because they worked on a project that never shipped (regardless of how much or how little they contributed to the failure)? Just replacing it with an AWS bill-measuring contest?
- stickfigure 2y ago"Software at scale" is different from "data at scale" is different from "compute at scale". But yeah, when I hear "STOP MAKING SERVICES AND FACTORIES AND INTERFACES AND JUST READ THE FUCKING JSON FILE YOU ENTERPRISE FUCKERS" I think "developer who's never worked on anything more complicated than a chat app, and isn't old enough to have learned humility yet".
- FrustratedMonky 2y agoThe thing that should be tested is whatever you are handing to a client app to use. If it is an interface, then test the interface, not the file behind it. If the client will get the file, then test that the file is correct. So in this case, is this entire file being handed to clients to do what they will with it? Does that make sense as an interface. If you are building an app, and you want other parts of the app, or clients of the app, to use this data, does it make sense to just hand over the entire file as the interface. Basically : Programmer 1: "Hey man, show me your data interface so I can build a quick lookup on something" Programmer 2: "here is the whole damn file, do whatever you want and leave me alone. Figure it out yourself, and no I'm not going to give you a schema and I'll change it whenever I want".
- sigseg1v 2y agoYou are right. I read their posts as the ramblings of someone who is currently in shock, found a bunch of bad practices in the logging + data retention, and is now just tongue-in-cheek mocking (the puns...) everything even if they don't have much experience with it. I would probably say something similarly incorrect if I found some perl and tried to understand it because I know nothing about writing maintainable perl.
- dylan604 2y ago> maintainable perl isn't that an oxymoron?
- cruffle_duffle 2y agoI am lost is that a code comment or the author commenting on something they found? Because I always enjoy leaked code comments. It’s like “tell me how you really feel about this shitty bloated enterprise framework you are using”. There were some good ones in the leaked windows source code, weren’t there?
- aidenn0 2y agoIf you need to make your code more baroque and harder to understand in order to unit-test it, that seems like the tail wagging the dog.
- mempko 2y agoExactly! It's like that Skinner Simpsons meme. Are unit tests the problem and I'm wasting my time? No, it's the config files that are wrong.
- consteval 2y ago> more baroque and harder to understand I don't understand how this is the case. If anything, an interface is MUCH easier to understand than a variety of functions strung together. I mean, this is the whole reason we have APIs. If I'm a consumer, I would much rather read and understand the API and its contract than try to read through the code to find out requirements.
- aidenn0 2y agoWe are talking about a single function that possibly takes zero arguments versus an interface (TFA doesn't seem to show the code, but the interface presumably exists for DI). I have waded through such code in Java, rather than C#. At least some of it is fighting the language; Java is pretty hostile to writing DI style code. On top of that, even in languages that are more DI friendly, DI significantly decreases the lexical locality of code that is dynamically very close.
- consteval 2y agoI don't think that you necessarily need lexical locality, rather you need specification. And interfaces are just an easy way to make a specification. The problem with "too much" lexical locality is that now you have to search DEEP into stacks to figure out what really going on. The whole point of specifications is trying to extract the most requirements in the least amount of time. With more "top level" interfaces and tools like DI you can do that, but certainly you can take it too far. A single function, I'd say, is way too far.
- burnte 2y ago> I know it's cool to "hate" on OO, but "just read the fucking file" doesn't work if you want to run your unit tests without reading a fucking file. Then don't do that, if in the real world it'll read a fucking file, then test with reading a fucking file. Tests aren't there to just be passed, they're to catch problems and if they're not testing the same workflows that the code will see IRL then the test is flawed. The first test should be reading a fucking file and that fucking file could be full of all sorts of garbage. Same goes for non-fucking files.
- jessekv 2y agoYeah modeless software is one honking great idea. (RIP Larry Tesler)
- xnorswap 2y agoThose are integration tests. Integration tests are great, but not when you want to run thousands of them in a few minutes. And not when you want to have lots running in parallel, accessing and potentially making "changes" to the same files. I'm happy to have a long running integration test suite that runs on a build server. But while working on a project, I need fast running unit tests that I can edit and run to get fast feedback on my work. I find that "time to iterate" is key to effective and enjoyable development. That's why hot module reloading is an amazing innovation for the front-end. The back-end equivalent is quickly running affected unit tests. So I'd rather unit test my FooFileReader to make sure it can read parse (or not) what's in various files, and unit test my service which consumes the output of my FooFileReader by either parameterising the FooFile result or having an IFooFileReader injected. ( Either works to separate concerns. ) While unit testing, I'm going to test "given that System.IO.File can read a file", and write tests accordingly. I don't want a test sometimes fails because "read errors can happen IRL". That doesn't help test my business logic. I can even test what happens if read failures do happen, because I can mock my mock IFooFileReader to return a FileNotFoundException or any other exception. I'd rather not have to force a real-world scenario where I'm getting such an error. In a functional world, it's the difference between: function string -> result and function string -> parsedType -> result The second is cleaner and neater, and you can separately test: function string -> parsedType function parsedType -> result The second is more testable, at the cost of being more indirect. Interfaces and factories are just an idiomatic .NET way of doing this indirection over services and classes. Of course you can also write more in a functional style, and there are times and places to do that too.
- qwertycrackers 2y agoYour unit tests should just take the result of loading the file as an argument or other type of injection param. Then you can hardcode your unit test config parameters in the test code itself. That's the appropriate place for this kind of indirection.