9 ms·
> as much as you can into static helper functions and most of the rest into dumb private stateless functions In our work we use C# and it is very hard, even n
by positivecomment 9y ago
> as much as you can into static helper functions and most of the rest into dumb private stateless functions
In our work we use C# and it is very hard, even next to impossible to make a static class pass a code review - given it's not for extension methods (which I hate... why not be explicit about the first parameter and stop acting as a part of the class </rant>). They just tell us to use IoC and move to the next point. I honestly don't know why. Our IoC library can treat a dependency as static or singleton, but those are also discouraged. Once I had a static class named GraphRequestHelpers* and the reviewer got really negative, FSM knows why. She told me that we need IoC to make everything testable and "Helper" in the name is a code-smell. Sounds cargo-culting to me but I have only 6 years of experience so who I am to know.
* Now we have RequestExtensions and everything is apparently perfect.
- knocte 9y agoC# is the new Java... facepalm
- Chris2048 9y agofacepalm of enlightenment?
- away2017throw 9y agoRifle is the new pistol? You can shoot yourself with both?
- hacker_9 9y agoDo you practise TDD? If you did a lot of this would make more sense to you. TDD is actually quite fun when you get the hang of it (less mental burden as you push all the 'intent' onto the computer).
- barrkel 9y agoTDD is completely orthogonal as to whether you write functional code that doesn't have a default receiver for routines, or OO code.
- lkitching 9y agoI don't see why TDD requires ruling out static methods and insisting on hiding everything behind an interface. Static methods are straightforward to test, certainly more than a class with multiple dependencies which need to be mocked. Usually the complaint is about coupling when calling static methods but these can be wrapped in a delegate if required.
- hacker_9 9y agoSimply because you can't mock the static dependency, therefore that method is now dependent on the static class and you don't have any control over it. This is problematic - what if at some point later another developer adds a database call into the static method to do some logging? Now your testing will dirty whatever database you're using, as well as run 10x slower - and yet the test will still pass and everyone will be none the wiser as to what happened. If you start using a custom delegate solution, then your code is not consistent with everything else that uses DI, making it harder to understand. I can understand interfaces are annoying when navigating code, but the IDE still helps with that even if it is a few more button clicks, and the pros outweigh the cons.
- lkitching 9y ago> that method is now dependent on the static class and you don't have any control over it. I don't see how you have any less control over it than any other code you wrote. If you don't want it to write log statements, then don't do that. Most static methods are small and pure so don't need to write log statements anyway. > Now your testing will dirty whatever database you're using, as well as run 10x slower. I've never used a logging framework that didn't allow you to configure where log statements were written, or give you control over the logging threshold for individual classes. However if your method is writing logs then presumably there is a reason, which is just as useful in the tests. If you mock it out then you're testing against different code to the one you will actually run against. > If you start using a custom delegate solution, then your code is not consistent with everything else that uses DI. Passing functions as arguments directly is 'DI', just without the need to configure that through an external container. Reducing the amount of interfaces (often with a single implementation) and external configuration makes navigating the code easier.
- pjc50 9y agoCould someone expand IoC for me please?
- cjsuk 9y agohttps://martinfowler.com/bliki/InversionOfControl.html https://martinfowler.com/bliki/InversionOfControl.html
- deleted 9y ago[deleted]
- cjsuk 9y agoThere is some cargo culting there but it's mostly correct. Helper is a code smell as it's a sign of "we don't know what the responsibility of this is or what to call it so we'll just chuck a load of shit in this file and call it a helper". The methods in should belong to something and live on that class, not in an external class. RequestExtensions is more shit than the original solution. Extension methods are even worse! Shoot the reviewer.
- positivecomment 9y ago> Shoot the reviewer Duly noted! Although I'll try talking to her first, I'm sure there's more behind the decision :) One of the methods that was inside takes a request, extracts the body and returns the parsed graph from the body. It's used by many controllers from many projects. I don't know where to put such a thing, hence the request extension.
- cjsuk 9y agoAlways ask for the reason before slating it :) Usually that's a single responsibility class: interface IGraphParser { Graph Parse(Request request); } Inject that into the caller via the container then you can mock the thing that calls it and just return a static Graph object, which you can't do with a simple extension method (which is why it sucks).
- quickthrower2 9y agoThis is a matter of taste not fact. In functional languages the style is compositional with static functions everywhere. It works well. The keeping data and methods together thing is one approach. Sometimes it's great. Sometimes unnecessary. For example would you argue against string formatting helpers? Or would they need to be written to an interface and added to myriad DI bucket lists?
- cjsuk 9y agoIt's not that simple and it's not a fact. I'm an advanced user of functional languages as well and have written an entire scheme implementation before. I only semi-agree. That's slightly disingenuous representation of functional languages which have more than a few pitfalls. They certainly aren't the silver bullet and they really do not scale to the same height and complexity of the problem domain as the OO languages do due to the nature of the abstraction you describe. Nothing is particularly explicit. I'd rather take the compromises of OO over the maintenance problems of a functional language. String formats are data so they would be stored as constants so that they are interned. They can be stored in a const class which is a static class with no methods i.e.: sealed class StringFormats { public const DateFormatX = @"..."; } Also string formats for example tend to be owned by the respective objects so you can add overloads to the object to provide certain arbitrary representations. If the translation between an object and the string representation is complex, then you're really serializing it so that should be an abstracted concern.
- seanmcdirmid 9y agoExtension methods are useful for only one reason: they trigger code completion for browsing what this object can do. Static methods suffer from FP code completion problems (you can’t complete easily in the first arg of a function/procedure).
- emodendroket 9y agoI think I am not mistaken in saying extension methods, like lambda functions, were invented primarily for the use case of Linq. Even if they weren't, that's how Linq is implemented, so extension methods serve more than that "one purpose" if you don't insist on writing C# in the style of C# 2.0.
- seanmcdirmid 9y agoThey came out at the same time, I’m sure there was some influence between them (Mads Tergesen would know better). However, all the functionality added in could have been done with static methods, just with more verbose syntax. LINQ query syntax could have been special cases. Anyways, I like what they came up with, it’s very versatile.
- emodendroket 9y agoIt's not clear that would have been much less work
- emodendroket 9y agoWhy hate extension methods? Do you really want to write Enumerable.ToList(Enumerable.Select(Enumerable.Where(someList, e => e.someBool), e => new {a = e.x, b = e.y)) and so on?
- positivecomment 9y agoThat would suck, on the other hand, the extension methods make people create huge chains continuations of which comes from who knows where. Best solution would have been a pipe operator if you ask me.
- olavk 9y agoYeah there is some cargo cult aversion towards statics. Static methods with no side effect are wonderful, but static state is really bad and static methods which perform IO are horrible because they cannot be mocked in a unittest. But some people miss this distinction and just say static methods are bad for testing.