How to increase readability of an interface (C#)
-
The current example project for the chapter I'm on has an interface that looks like this:
using System.Collections.Generic; using System.Threading.Tasks; namespace AutoLotDAL.Repos { interface IRepo<T> { int Add(T entity); Task<int> AddAsync(T entity); int AddRange(IList<T> entities); Task<int> AddRangeAsync(IList<T> entities); int Save(T entity); Task<int> SaveAsync(T entity); int Delete(int id); Task<int> DeleteAsync(int id); int Delete(T entity); Task<int> DeleteAsync(T entity); T GetOne(int? id); Task<T> GetOneAsync(int? id); List<T> GetAll(); Task<List<T>> GetAllAsync(); List<T> ExecuteQuery(string sql); Task<List<T>> ExecuteQueryAsync(string sql); List<T> ExecuteQuery(string sql, object[] sqlParametersObjects); Task<List<T>> ExecuteQueryAsync(string sql, object[] sqlParametersObjects); } }
As is, the eye has to keep jogging back and forth between the regular and async versions of each function (which is easier to do with this highlighting scheme than VS's, but still not great). How would one go about making that easier to read? My first thought was to knock the names down under return type like so:
int Add(T entity); Task<int> AddAsync(T entity);
But I'm not sure that's the best way to do it, nor whether it'd be idiomatic for C#. Any suggestions?
-
I would split them into normal and async, put a #region round each group, and never worry about it again :)
-
@coldandtired said in How to increase readability of an interface (C#):
I would split them into normal and async, put a #region round each group, and never worry about it again :)
Basically this.
-
You could also consider splitting the thing out into two separate interfaces, which are then both implemented by your concrete implementation. Only do this if necessary, it might not make sense.
-
@Dreikin said in How to increase readability of an interface (C#):
Any suggestions?
Alphabetize and columnificate:
using System.Collections.Generic; using System.Threading.Tasks; namespace AutoLotDAL.Repos { interface IRepo<T> { int Add (T entity); Task<int> AddAsync (T entity); int AddRange (IList<T> entities); Task<int> AddRangeAsync (IList<T> entities); int Delete (int id); int Delete (T entity); Task<int> DeleteAsync (int id); Task<int> DeleteAsync (T entity); List<T> ExecuteQuery (string sql); List<T> ExecuteQuery (string sql, object[] sqlParametersObjects); Task<List<T>> ExecuteQueryAsync (string sql); Task<List<T>> ExecuteQueryAsync (string sql, object[] sqlParametersObjects); List<T> GetAll (); Task<List<T>> GetAllAsync (); T GetOne (int? id); Task<T> GetOneAsync (int? id); int Save (T entity); Task<int> SaveAsync (T entity); } }
Has the compelling advantage that weenies who insist on editing code using a proportional font will hate your guts for it.
-
@flabdablet said in How to increase readability of an interface (C#):
Has the compelling advantage that weenies who insist on editing code using a proportional font will hate your guts for it.
Nice.
I'm not too keen on the tabulated style, simply because you can wipe it all out with an auto format command when you only meant to get your method indented correctly.
-
@Jaloopa Proportional-font weenies who also insist on using formatting tools with no sense of visual style have only themselves to blame.
On a serious note, though: the human eye is really good at picking out anomalies if you maximize whatever regularities exist in whatever you're giving it to look at. The alphabetized columnar style lays out all the method names in a way that lends itself to very fast visual scanning, as well as making unexpected pattern holes obvious and inviting questions like "why does Save(...) exist here instead of SaveOne(...) and SaveAll(...)" that can lead you to a better understanding of the code base.
It's immediately obvious that each of the sync methods has an async counterpart, because alphabetization forces their names to end up nearby.
The pattern break after a couple of alternating Name/NameAsync pairs alerts the reader to the fact that some of these methods are polymorphic, without making a bigger deal out of that than needs to be made.
Of course, if you hate this sort of thing then this is the sort of thing you'd hate with a passion.
-
@flabdablet no just no.
-
@flabdablet said in How to increase readability of an interface (C#):
It's immediately obvious that each of the sync methods has an async counterpart.
That's a regularity that could be maximised further by always putting the matched pairs together. I'm surprised you didn't do that already…
-
@lucas1 said in How to increase readability of an interface (C#):
@flabdablet no just no.
Exactly the kind of response I'd expect from a whippersnapper with no FORTRAN punch-card coding forms in his soul. Get off my lawn.
-
@dkf said in How to increase readability of an interface (C#):
could be maximised further by always putting the matched pairs together
But that would require (a) breaking strict alphabetization, which my OCD will not allow and (b) losing the pattern break that alerts you to the existence of the polymorphic methods.
-
@flabdablet said in How to increase readability of an interface (C#):
losing the pattern break that alerts you to the existence of the polymorphic methods
You'd see that in the strictly alphabetised Intellisense list.
-
@Jaloopa said in How to increase readability of an interface (C#):
Intellisense
If your code isn't comprehensible when printed on 15" greenbar, you're .
-
@flabdablet if you still feel the need to adhere to standards that were only practically necessary 40 years ago you're
I bet you insist on not going over 80 columns don't you
-
@Jaloopa Nonsense. Proper greenbar has 132 columns.
Failing to leave enough space on the ragged right for the maintenance folks to scribble WTF!??!? in red ballpoint is, of course, impolite.
-
@flabdablet said in How to increase readability of an interface (C#):
Failing to leave enough space on the ragged right for the maintenance folks to scribble WTF!??!? in red ballpoint is, of course, impolite.
Kids use crayons these days…
-
@dkf How they expect to be able to achieve the required emphatic puncture depth with such inadequate tooling is more than I can understand.
-
@flabdablet said in How to increase readability of an interface (C#):
inadequate tooling
-
I would say take the non-async versions and flush them down the toilet entirely. If the consumer wants to run it synchronously, it can simply do:
var myResult = GetOneAsync(poop).Result;
Which'll block his thread until the Result is populated.
... that's almost certainly what the non-async implementation will do anyway.
Also why is the id for
GetOne()
nullable? What does it return if you pass null? Just literally any one of them?
-
@blakeyrat said in How to increase readability of an interface (C#):
I would say take the non-async versions and flush them down the toilet entirely. If the consumer wants to run it synchronously, it can simply do:
var myResult = GetOneAsync(poop).Result;
Which'll block his thread until the Result is populated.
... that's almost certainly what the non-async implementation will do anyway.
I'll keep that in mind for my own projects. In this particular example, the sync and async versions map directly to EF equivalents:
public T GetOne(int? id) => Table.Find(id); public async Task<T> GetOneAsync(int? id) => await Table.FindAsync(id); public List<T> GetAll() => Table.ToList(); public Task<List<T>> GetAllAsync() => Table.ToListAsync();
Also, I figure any answer that works well for this question will work in the more general case of many different return type lengths.
-
@blakeyrat said in How to increase readability of an interface (C#):
Also why is the id for GetOne() nullable? What does it return if you pass null? Just literally any one of them?
I saw that too. I don't know why it's nullable, and just left it that way since for the purposes of the example it was irrelevant. Based on the summary text for what it maps to (
DbSet<T>.Find(params object[] keyValues)
andDbSet<T>.FindAsync(params object[] keyValues)
), I think it just returns null.
-
@Dreikin said in How to increase readability of an interface (C#):
@blakeyrat said in How to increase readability of an interface (C#):
Also why is the id for GetOne() nullable? What does it return if you pass null? Just literally any one of them?
I saw that too. I don't know why it's nullable, and just left it that way since for the purposes of the example it was irrelevant. Based on the summary text for what it maps to (
DbSet<T>.Find(params object[] keyValues)
andDbSet<T>.FindAsync(params object[] keyValues)
), I think it just returns null.Tested it. It returns null.
-
interface IRepoSync<T> { int Add( T entity ); int AddRange( IList<T> entities ); // ... snip ... } interface IRepoAsync<T> { Task<int> AddAsync( T entity ); Task<int> AddRangeAsync( IList<T> entities ); // ... snip ... } interface IRepo<T>: IRepoSync<T>, IRepoAsync<T> {}
-
@flabdablet only if you align with spaces instead of a single tab like a typical cromagnon.
-
@Dreikin said in How to increase readability of an interface (C#):
I'll keep that in mind for my own projects. In this particular example, the sync and async versions map directly to EF equivalents:
Wait, are you building an abstraction directly over an almost-identical abstraction?
-
If all the async versions are doing is adding an async wrapper, why not just pull that out of the interface and anywhere you want to do the operation async put a wrapper around it. Doesn't sound any more messy than having two methods that do the same thing two different ways.
-
@JazzyJosh said in How to increase readability of an interface (C#):
If all the async versions are doing is adding an async wrapper
Or vice versa. Either way, it's dumb to say everything twice. Asking for trouble.
-
@dkf Yeah, was going to say that, but you can't really make the interface async and call them as synchronous methods
Well, unless you don't care about the result, whether it failed or not.
-
@JazzyJosh said in How to increase readability of an interface (C#):
Well, unless you don't care about the result, whether it failed or not.
Convert failures to exceptions? Each mechanism needs a way to report failures, but they'll happen differently. Ought to be robo-wrappable…
-
@dkf Yeah, we actually do the same thing.
I'm still tired. Time for coffee.
-
@blakeyrat said in How to increase readability of an interface (C#):
building an abstraction directly over an almost-identical abstraction
It's the enterprise way.
-
@flabdablet said in How to increase readability of an interface (C#):
@blakeyrat said in How to increase readability of an interface (C#):
building an abstraction directly over an almost-identical abstraction
It's the enterprise way.
You know, in an enterprise it can make a lot of sense. Consider that you could have many, many, many dependencies on that interface, and if you don't directly control it, a small change could necessitate thousands of updates. Or one, if you put it inside a simple wrapper.
-
@blakeyrat said in How to increase readability of an interface (C#):
@Dreikin said in How to increase readability of an interface (C#):
I'll keep that in mind for my own projects. In this particular example, the sync and async versions map directly to EF equivalents:
Wait, are you building an abstraction directly over an almost-identical abstraction?
The book is, yes. There was a brief mention of "the repository pattern" with not much of an explanation, but I've noticed that the DbContext intellisense already says it does that, so... Book says:
Adding the Repositories
A common data access design pattern is the Repository pattern. As described by Martin Fowler, the core of
this pattern is to mediate between the domain and data mapping layers. While the full explanation of the
repository pattern is beyond the scope of this book, the pattern is helpful in eliminating duplicate code.The context is that this chapter builds a DAL for use in later chapters (WPF and ASP.NET sections, I suspect), so this may be done this way to make future examples easier. Or for some other reason (demonstrate some form of encapsulation?), or for no good reason at all. My best guess is that it's meant as a demonstration of wrapping the EF implementation behind an API, but with allowances made due to its nature as an example (e.g., the book warns against using the
ExecuteQuery
methods in a proper version because SQL injection).@error said in How to increase readability of an interface (C#):
@flabdablet said in How to increase readability of an interface (C#):
@blakeyrat said in How to increase readability of an interface (C#):
building an abstraction directly over an almost-identical abstraction
It's the enterprise way.
You know, in an enterprise it can make a lot of sense. Consider that you could have many, many, many dependencies on that interface, and if you don't directly control it, a small change could necessitate thousands of updates. Or one, if you put it inside a simple wrapper.
Or this, maybe.
-
@error said in How to increase readability of an interface (C#):
Consider that you could have many, many, many dependencies on that interface, and if you don't directly control it, a small change could necessitate thousands of updates. Or one, if you put it inside a simple wrapper.
Every time I've seen this particular antipattern actually deployed in an enterprise context, what actually happened when the underlying APIs changed is that the changes got propagated all the way up through the stack; they never actually stopped at any of the supposed-to-be-immutable wrapper layers. Which is exactly what you'd expect, given wrappers inserted willy-nilly with no actual design applied.
-
I think the author of that book hasn't ever heard of the concept "YAGNI". You Ain't Gonna Need It.
Sure your "repository" pattern will be useful in the year 2631 when all databases turn to dust after the super-Martian attacks and return of Gozer, but you ain't gonna need it now.
-
@JazzyJosh It is not the the same, it is EF specific and cannot be mocked easily. TBH I think EF is utter shite and won't touch it with a barge pole.
-
@blakeyrat Stop giving bad advice. If you use it with DI testing your controllers or whatever else is using the repository is stupidly easy to mock. I agree that I probably wouldn't bother when knocking up a simple CRUD app, but he is supposed to be learning the correct way from the book.
-
@blakeyrat said in How to increase readability of an interface (C#):
I think the author of that book hasn't ever heard of the concept "YAGNI". You Ain't Gonna Need It.
I really hate it when people use YAGNI as an excuse for shitty design.
YAGNI refers to features, not to software design.
-
http://nickgravgaard.com/elastic-tabstops/columnblocks_coloured.gif
Long live proportional fonts.
-
have you tried turning the computer off and on again?
-
@asdf said in How to increase readability of an interface (C#):
@blakeyrat said in How to increase readability of an interface (C#):
I think the author of that book hasn't ever heard of the concept "YAGNI". You Ain't Gonna Need It.
I really hate it when people use YAGNI as an excuse for shitty design.
YAGNI refers to features, not to software design.
unnecessary indirection is shitty design, you fucking cargo cultist
-
@asdf said in How to increase readability of an interface (C#):
YAGNI refers to features, not to software design.
I don't know how you use it, but in our shop it refers to both.
-
@fbmac said in How to increase readability of an interface (C#):
unnecessary indirection is shitty design, you fucking cargo cultist
That depends on your definition of "unnecessary indirection". Proper encapsulation and separation of concerns require some degree of indirection.
-
@asdf I disagree, indirection layers change as often as whatever you're trying to separate. It's an illusion.
-
@fbmac said in How to increase readability of an interface (C#):
I disagree, indirection layers change as often as whatever you're trying to separate. It's an illusion.
So OOP is bullshit, then, in you opinion? And tight coupling is OK because loose coupling is an illusion anyway?
-
@asdf OOP isn't an indirection layer.
-
@Magus said in How to increase readability of an interface (C#):
http://nickgravgaard.com/elastic-tabstops/columnblocks_coloured.gif
Long live proportional fonts.
What dark sorcery is this?
Filed under: And why is it not built-in by default to every IDE on the planet?
-
@fbmac said in How to increase readability of an interface (C#):
OOP isn't an indirection layer.
What are getters and setters, then, if not a level of indirection?
-
@Dreikin said in How to increase readability of an interface (C#):
Tested it. It returns null.
@blakeyrat said in How to increase readability of an interface (C#):
"YAGNI"
But what if we have to swap out for a database that does things much better, but we want to retain our, now, bottleneck of an API?
-
@error Elastic Tabstops. Available now for VS, CodeBrowser, VIM, and a variety of other editors.