3 ms·
> Stop writing stupid classes. This is really it. It's not a game of all-OO, zero-OO or strictly following a pattern you saw on YouTube that one time. It's all
by bob1029 3y ago
> Stop writing stupid classes.
This is really it. It's not a game of all-OO, zero-OO or strictly following a pattern you saw on YouTube that one time. It's all about nuance and minimizing the # of moving parts until each is really serving you some value.
I am in the process of ripping out a bunch of class bloat in a product iteration. Our typical pile of model class POCO spam for "one thing":
Entity.cs --The "official" view
EntityController.cs
EntityView.cs --Another common view that became popular over time (???)
EntityTableRow.cs --gotta have that 1:1 sql model, amirite?
EntityViewForTypeA.cs --what was wrong with the common one :(
EntityViewForTypeB.cs --And again...
...
Ultimately, every time we wanted a "view" of the data, we wound up creating another type to enshrine this and then tried to figure out how many holes we could more-or-less hammer it through. Every one of these has some cranky mapping layer too (NxM if you want to get crazy).
The alternative pattern I am working with is to directly query my database in the exact place the data is required and to use anonymous / scalar return types like so:
//Some function that produces a HTML partial view for a webapp
//View w/ join that would otherwise require a new POCO type to communicate.
var myEntityView = await sql.QueryFirstAsync(@$"
SELECT e.Id AS Id, ep.Name as Name
FROM Entities e, EntityProperties ep
WHERE e.Id = ep.EntityId
AND e.Id = @Id", ...);
//Direct usage of anonymous result type.
return $@"
<h3>Entity:
<a href='Entities/{myEntityView.Id}'>{myEntityView.Name}</a>
</h3>";
In the above, there are zero POCOs or "messenger" types. Just a raw query right where it needs to be taken, without any extra ceremony or implications for other code sites. I actually don't have a single type in the new codebase that is a POCO, other than schema tracking models (which are still useful for interpolating into queries to enforce magic strings). Effectively, the only domain data types are 1:1 with the SQL schema now.
- vkou 3y ago>This is really it. It's not a game of all-OO, zero-OO or strictly following a pattern you saw on YouTube that one time. It's all about nuance and minimizing the # of moving parts until each is really serving you some value. Minimizing the number of moving parts shouldn't be your goal. Testability of the moving/complicated parts should be. I don't really care about how you break up your code, but it should be trivially testable, at both the unit, and the integration test level. OOP makes it at least possible for less experienced developers to accomplish both of those things. It's nice that you got rid of all the indirection and all the nearly-pass-through interfaces in your code sample, but how would you test it? In a unit test? In an integration test? Are you going to need to dial out to a real, live SQL database to do so? For every test? Will you be re-using the database across multiple tests, and thus, require all your tests to avoid object collisions? Paying a startup cost for every test you run? Reusing it for some tests? Who's going to maintain that reused, QA database? Who's going to deal with it when some clown pollutes it with garbage? These are all solvable problems, they have multiple approaches to solving them, with varying tradeoffs, but they are tradeoffs. 'Simpler-to-write' code is usually not the tradeoff you want to optimize for. You write code once, you test it thousands of times.
- bob1029 3y ago> It's nice that you got rid of all the indirection and all the nearly-pass-through interfaces in your code sample, but how would you test it? > Are you going to need to dial out to a real, live SQL database to do so? For every test? Yes. This is actually how we do it. And if you think deeply about it, this allows you to skip seamlessly between unit & integration testing, assuming all of the system components are designed against the same database. The only meaningful difference between unit & integration in this context is how much state you allow to accumulate in the database between invocations. You can reset for each method using a known expected initial state for each, or you can stand up an initial state and fly through an entire playlist of method calls. If all of your functions take a SqlConnection object as their first argument and you've made sure that 100% of application state resides in the database, what can't be tested in this manner?
- wizofaus 3y agoThe usual reason is that if you actually require a disk-based DB to exist to run your tests it affects a) how easy it is to run them (e.g. on a CI build agent that's probably a light weight container with minimal software installed etc.) b) how quickly they run - if I can't run my entire test suite in less than a couple of minutes it's probably reducing productivity c) how much set-up code my tests need and d) it needs some mechanism to ensure tests can still safely run in parallel, esp. if your database schema isn't designed to cope with multiple independent instances sharing a DB - and having each test create and use its very own DB is almost certainly going to impact performance. Being able to use a memory-based DB that still supports enough of the SQL your code uses is a decent compromise much of the time, but it's annoying having to write tests for 90% of your functionality one way and 10% another because it relies on behavior that's different between your memory-based and disk-based DBs. Personally I still prefer to able to unit test code as much as possible without needing a DB at all, which generally means separating out the code that transfers state between the DB and in-memory structures and code that operates on said structures, but there are definitely times that's not entirely practical. Either way, I can't imagine being keen about mixing UI level code with SQL queries, even if I understand your annoyance with having to define a bunch of DTOs just to handle specific queries (I will say that typescript is actually pretty good for that sort of thing, but not sure if it could help in your case).
- naasking 3y ago> In the above, there are zero POCOs or "messenger" types. Just a raw query right where it needs to be taken, without any extra ceremony or implications for other code sites. This is a terrible idea IMO. Creating a POCO per view is not a big deal, you're already implicitly depending on a specific type contract in the view, except now it's implicit and dynamically typed and so subject to all of the problems that follow from that.