Eliminate Data Clumps: Step-by-Step Refactoring Guide

[@zoran-horvat]

Learn more at Brilliant brilliant.org/ZoranHorvat/ This link gives you a 30-day free trial and 20% off of an annual premium subscription.

[@Senboni]

there's nothing more satisfying than removing code

[@zoran-horvat]

@@Senboni I like that the most

[@VakkaHUN]

Listening to Zoran is like listening to grandpa telling war stories.

[@kenbrady119]

But it is a modern and ongoing war. Believe me, I've been on one side or the other for over 40 years now. The winner gets the most efficient and reliable algorithms.

[@westlestrest6442]

Man I love your style. Just discovered the channel and I love the little jokes you throw in and all the explanations! Thank you!

[@JoseTorres-bl9cu]

This is just gold! Thank you for making these videos!

[@Robert-yw5ms]

This demo makes me wonder something: is defensive programming a code smell? After all, if a method is called you generally want to be sure the preconditions are met. And this demo shows that a smarter approach to the domain allows you to know in advance that the preconditions are met, removing the need to code defensively. I noticed something similar in my work, where I'm currently converting an azure function written in python to c#. Defensive checks litter the python code but I've been able to remove the majority of them by using a rich domain model.

[@zoran-horvat]

@@Robert-yw5ms That is the principle I am advocating for. In all compiled languages, the compiler is the most valuable static code analyzer. Postponing the checks for run time is often a sub par decision, often made due to the lack of design.

[@nickcorrado5105]

Excellent as always! This is just what I was looking for after last video about this model.

[@nickbarton3191]

A great follow on from your previous video about defending against all of those invalid combinations. I think I'd have factored it out based on an Interface, it's extremely likely that there'll be alternative implementations, and future requirements too.

[@ozkanb]

Great video, I'd love to see the end result.

[@luckyknot]

We need Zoran to make an ASMR session or an audiobook. Harry Potter would be fine.

[@zkreso]

Loving this series

[@LordErnie]

It is finally clicking. I have done some F#, and I now come back to C# and don't want it any different. This just makes it so much more flexible. You ones said that you control these types of structures by not controlling them, and it clicks now. Its the tell it what not how story. It is now so obvious that you have different types of PublicationInfo classes, and different types of books where we can have a SceduledBook, PublishedBook, PlannedBooks, etc. Same for the Edition, where you can have multiple types and variantions of it. Even though they serve the same purpose, due to their difference in state the overarching business rules change for the types. Because each type focusses on keeping track of its own set of rules for its state, it becomes maintainable. Its just discriminated unions but different. So strange.

[@zoran-horvat]

@@LordErnie The next step from here is to finally introduce polymorphism.

[@stevejohnny1111]

Better video, thanks for taking on my feedback.

[@paarma1752]

Personally I do enjoy them flat data structures. They are typically easier to map (dbmodelexcel...) and move around. And also, in case in the future there's a need for PublicationInfo to know its Book, we don't need to refactor or (as typically happens) work around our earlier decision to create structure based on the assumptions of that time. My rule of thumb is to not create nested structure "early" unless its needed by a 1-to-many relationships or such. The decision of creating nested structure can be postponed, so therefore (imho!) it should. If at some point in the future it becomes infeasible to have that flat structure any longer, then I'll pop those properties to a structure like you created, but I would also have gained a lot more understanding about the domain by then, so I wouldn't be just making a guess about the future in my design. I think I'd use builder pattern to initialize Books to avoid that combinatorial explosion problem... But as always, there are tons of considerations in real life.

[@zoran-horvat]

Is persistence your primary concern?

[@paarma1752]

@@zoran-horvat not the primary concern, but ease of persistence and transfer are definitely nice-to-have at least! What I'm most concerned about is overengineering, which I think is the worst virus out there suffocating projects. Understanding a 1000 line 10-year-old class or method is the TRIVIAL problem - it just requires time and focus. And if we have still misunderstood the logic and thus introduce a bug, fixing that bug is again the trivial problem, since we don't have abstractions and thus got all the knobs "right there" available. We hotfix the bug to the mega-method without needing any indirection - most likely changing only a single file - and we're again good to go. But what is not a trivial problem is a bad (aka previously good) abstraction. We can't get around it by just exploring it and trying to understand it, because it's just simply not viable anymore. If our assumptions back then were incorrect (which is always the case), then we EITHER need to do a potentially infeasible amount of work to re-think the problem, get rid of our old assumptions (which may have cascaded to many places beyond our immediate control, requiring lots of coordination) and replace them with our most recent assumptions. OR ALTERNATIVELY (as usually happens) poke holes to our abstractions to "just make it work". And just like that the spaghetti starts boiling. Nowadays I simply resist the urge to create any structure that is not absolutely necessary for technical reasons. No matter how intoxicating it feels to design these amazing structures that just "kind of click". Instead I keep my code flat and imperative and unbiased and low in abstraction and non-dry all the way until it's not viable anymore, and only then create abstractions and structure. With my approach there will be bugs, since test automation can't necessarily cover everything. But the bugs are DEAD SIMPLE to fix every single time. And I rather be a code-monkey whacking simple bugs every now and then than torture myself and others with my bad early assumptions every single day forever.

[@zoran-horvat]

@@paarma1752 You say "not the primary concern" and then put it first. There are at least a dozen typical, well-founded errors you insist on making, and then, what do you expect from me? Do I have to repeat 100+ videos I made explaining those errors here? You don't mention the consequences here. Start from that.

[@paarma1752]

@@zoran-horvat no you don't have to do that. I don't think I requested anything... just commented to your video and replied to what you asked.

[@ghevisartor6005]

I feel like I have been discouraged of doing proper oop because of Ef core and not knowing how to map these objects in the db context configuration. But eventually I would create these model object in the presentation layer, so all the mapping was manual from the entities or dtos. Lots of duplication for no reason. To the primitive obession point, in another case where i had to work with file paths (so no entities), I took ispiration from your videos so I ditched strings and made a FilePath object with properties hiding the System.IO.Path members i needed or methods to combine them with other FilePath objects.

[@CameronFranchi]

Zoran, thanks for the great content as usual. I would love to see a video on ways to persist these structures to a database, using EF core would be especially helpful. I find myself gravitating towards anemic models without realizing it because they seem simpler to implement from a database standpoint. Do you often use value objects, and if so when would you represent entities as value objects vs separate tables?

[@zoran-horvat]

@@CameronFranchi There are a few videos I made recently that demonstrate the persistence of the same object model, only done fully OO.

[@CameronFranchi]

@ I try to watch each video you put up, must have missed this one on EF core configuration. Watching it now, thanks for your quick reply!

[@alanis4AL]

Amazing as always 👍

[@itsgoubie]

This video made me subscribe instantaneously.

[@zoran-horvat]

@@itsgoubie Glad to hear that!

[@TayyabMughalDIK]

[@PhilWright-kn9kj]

Would you still have to throw exceptions in the static factories if passed an out of range month value?

[@zoran-horvat]

@@PhilWright-kn9kj The DateOnly would do that, so yes, there would be a throw in the end.

[@AlexNovak-b2u]

Hello! Do you have any plans on making a video on the topic of something like "pure-impure sandwich" (the name is taken from some of Mark Seemann's article), or there is already one and I've just missed it? Specifically, mixing sync with async code and the problems it brings to. His talk eventually comes to monads (of which I'm not afraid of using actually), but YOU seem trying to ignore bringing these academic terminology and make pragmatic solution instead, so it would be interesting to hear your thoughts on this problem.

[@zoran-horvat]

@@AlexNovak-b2u I do have a few topics in the queue covering those questions. It is a large area which can be analyzed from several angles.

[@MahmoudSaed98]

Thank you very much, I have a question, how can we modify a specific property which is private? Do we create a public method for each property?

[@zoran-horvat]

@@MahmoudSaed98 Public property getters should return values that reflect the overall state of the object. Public methods would change properties in orchestration, so that the end result is a fully valid state. It is fairly uncommon to just allow setting one property. That may be the case when the property is self-sufficient. The Book class allows direct setting of the title and culture, for example. However, one could imagine that these two can only be set together, via a method, as they are related to each other.

[@GTZ-98]

Hey Zoran, I've been watching your videos for a while, and they've really helped me improve my code quality-thank you! I have a question about the target audience for your videos. I'm a Unity developer, and Unity often lags behind with some of the newer C# syntax, making it tricky to see how certain examples translate directly. Do you have any tips for adapting your content to other C# fields like Unity?

[@zoran-horvat]

@@GTZ-98 I am aware of the situation with Unity, but I have no plans to address the older syntax and types. My intention is to demonstrate the latest modeling and programming concepts, and so every novel syntax is the natural choice for inclusion, whereas older pieces of syntax will likely not make it into a video.

[@GTZ-98]

@@zoran-horvat Reasonable. Thanks for answering! :)

[@RoamingAdhocrat]

This is great and very satisfying. I've just watched (well, skimmed) "'Clean' Code, Horrible Performance" but am I right in thinking this kind of abstraction is just gonna get elided away in any well-behaved contemporary language? Since the new classes aren't doing any elaborate inheritance/polymorphism, they're just Plain Old Data with access controls preventing invalid data combos from being entered. So your changes probably make the code marginally faster by skipping the validity checks! In the name of locality of concerns you might have PublicationDate in the same file as Book (assuming there's no other publishable entities in the project), just to reduce the need for file-hopping

[@zoran-horvat]

@@RoamingAdhocrat Both PublicationInfo and PartialDate will continue to develop into polymorphic types pretty soon. But the performance impact will be negligible - we're talking about nanoseconds there, where any database call would be in the range of milliseconds. That is at least 1:10,000 performance in favor of OO code, so there is nothing to optimize there up-front.

[@Sorc47]

Thanks for the video. It would be great if you cloud also show how the db configuration needs to change in order to accomodate this refactoring.

[@zoran-horvat]

There are a few videos I published recently that demonstrate persistence of this model using EF Core.

[@samuelvishesh]

Having classes as DTOs and just do FP and use functions alone. You’ll thank me later.

[@zoran-horvat]

@@samuelvishesh Did you watch my previous videos and now you're giving me a short version, with a few omissions?

[@samuelvishesh]

@@zoran-horvat I did not. I’ll see the other videos of yours now.

[@samuelvishesh]

@@zoran-horvat too many videos. Can you point me to the right one you’re talking about?

[@zoran-horvat]

@@samuelvishesh You don't recognize functional concepts from their titles?

[@samuelvishesh]

@@zoran-horvat just post the Link here please

[@DavidSmith-ef4eh]

Where would one put PartialDate class? I don't feel like it belongs into the models folders, since I can see it being used outside of models. And it very likely won't be persisted as it's oven table in any scenario. I know it's easy to refactor file locations such things and all that, but it would legit bother me if it was left there.

[@zoran-horvat]

@@DavidSmith-ef4eh It is a model because it is designed as per the customer's requirements that define book dates. If it were a lesser concept, such as a measurement unit, etc., then it might as well fit some Common namespace.

[@DavidSmith-ef4eh]

@@zoran-horvat the problem is, when I open my models folder, there are 50 models in it (counting only tables in db)... but i guess there is no way around it, except maybe vertical slice architecture. then if we add all those "helper models" it could easily go to 80+ files. I guess I need an autclose expanded folders extension for vscode, because it takes up the entiere explorer sidebar. specially something like this partial date. this is a class that gets created once and should be forgotten about.

[@zoran-horvat]

@@DavidSmith-ef4eh I create many sub-namespaces in large models. Also, cutting the models vertically, as you mentioned it, helps a lot.

[@stefan-d.grigorescu]

​@@DavidSmith-ef4ehYep, it is a pain. Imo, after one goes to this, then tries vertical slices, they would never go back. You should try grouping items by their purpose, not by their technical type. It feels more organic

[@DavidSmith-ef4eh]

@@stefan-d.grigorescu The thing is, I don't change models that much. But when I do, my sidebar is full :D On the other hand, it is also nice to have all models in one place to have a glance of what the db looks like. Maybe I'll try grouping them into related groups

[@JanVerny]

I wonder if people will actually agree with me on this one. But I think you are wording this poorly. "Extracting data clumps into separate classes" accomplishes nothing on it's own, I could very well do that for days and it would only create more complications. The key takeaway from my point of view is about identifying the required business specific abstractions (a published or unpublished book) and then expressing them directly using the language.

[@zoran-horvat]

@@JanVerny The next step in this demo is turning the small anemic classes into proper object-oriented classes or functional types. Only after that step is done can we evaluate the effects.

[@JanVerny]

Don't misunderstand me, the refactor done here is good in my eyes. It's just that I feel like the reasoning behind the refactor is a bit diluted with the overly specific advice of take any data clump and turn it into a class. I feel like it won't always produce an actually useful result (unless potentially you do several other refactors afterwards).

[@amotkram99]

@zoran-horvat is there a source code where we get the entire implementation of the book project where all the videos are included instead of individual source code for each video?

[@zoran-horvat]

@@amotkram99 No, there is no one integral code, but a dozen of applications I made so far, none of them actually complete. I use these applications as generators of demos and videos, so to speak. There are many angles from which I am covering software development: monolithic/vertical slices/microservices; object-oriented/anemic object-oriented/functional and even parts done procedurally; specific demos such as top-down, design patterns first, cloud first, etc.; Web application/API/desktop/mobile; persistence with EF Core/Dapper/NoSQL, etc. Given the number of trainings and videos I make, there can never be a single application showing it all...

[@DavidSmith-ef4eh]

I liked it better the way it was beforehand.... jk, pls no bully 🤣

[@zoran-horvat]

@@DavidSmith-ef4eh That was a good trolling.

[@user-fs3qr5yg7e]

why not move to f# alltogehter?

[@zoran-horvat]

That would open the whole new plethora of questions.

[@user-fs3qr5yg7e]

@@zoran-horvat what questions come to mind? It has a learning curve - for sure. But it is worth it. The coolest feateure imo is not even the functional first stuff, but the fact that alle functions and files have a fixed order. Dependecies have to be declared before they are used. That for me cuts in half the time i need to understand what is going on in a new codebase.

[@zoran-horvat]

@user-fs3qr5yg7e It's not the learning curve - it is the mindset change. Reducing it down to a learning curve makes a counter favor to the cause. You asked the principal question yourself: Why not move to F# altogether?

[@danflemming3553]

The PartialDate abstraction and the need for the IsXXX boolean getters seem very weird to me. It should just be a PublicationDate instead, and remove the need for the boolean getters.

[@zoran-horvat]

@@danflemming3553 All boolean getters will go away once this class becomes polymorphic. The type itself will be the discrete choice then, which is now modeled with booleans. This video has only removed the data clumps. But then there should be the next step - turning small anemic classes into proper object-oriented or functional models.

[@ghevisartor6005]

yeah but he said that it was a puzzle for viewers at 12:48