Eliminate Data Clumps: Step-by-Step Refactoring Guide

Поделиться
HTML-код
  • Опубликовано: 7 ноя 2024

Комментарии • 60

  • @zoran-horvat
    @zoran-horvat  3 дня назад +4

    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
    @Senboni 2 дня назад +37

    there's nothing more satisfying than removing code

  • @VakkaHUN
    @VakkaHUN 2 дня назад +19

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

    • @kenbrady119
      @kenbrady119 2 дня назад +1

      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.

  • @JoseTorres-bl9cu
    @JoseTorres-bl9cu 2 дня назад +3

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

  • @nickbarton3191
    @nickbarton3191 День назад +1

    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.

  • @nickcorrado5105
    @nickcorrado5105 2 дня назад +2

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

  • @ozkanb
    @ozkanb 2 дня назад +2

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

  • @zkreso
    @zkreso 2 дня назад +2

    Loving this series

  • @stevejohnny1111
    @stevejohnny1111 2 дня назад +1

    Better video, thanks for taking on my feedback.

  • @ghevisartor6005
    @ghevisartor6005 2 дня назад +5

    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
    @CameronFranchi 2 дня назад +1

    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
      @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
      @CameronFranchi День назад +1

      @ 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!

  • @Robert-yw5ms
    @Robert-yw5ms 2 дня назад +4

    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
      @zoran-horvat  2 дня назад +9

      @@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.

  • @alanis4AL
    @alanis4AL 2 дня назад +1

    Amazing as always 👍

  • @paarma1752
    @paarma1752 2 дня назад +1

    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
      @zoran-horvat  День назад

      Is persistence your primary concern?

    • @paarma1752
      @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
      @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
      @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.

  • @LordErnie
    @LordErnie 2 дня назад +5

    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
      @zoran-horvat  2 дня назад +1

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

  • @PhilWright-kn9kj
    @PhilWright-kn9kj 2 дня назад +1

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

    • @zoran-horvat
      @zoran-horvat  2 дня назад +3

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

  • @TayyabMughalDIK
    @TayyabMughalDIK 2 дня назад +1

  • @GTZ-98
    @GTZ-98 2 дня назад

    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
      @zoran-horvat  2 дня назад +1

      @@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
      @GTZ-98 2 дня назад +2

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

  • @AlexNovak-b2u
    @AlexNovak-b2u 2 дня назад +1

    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
      @zoran-horvat  2 дня назад +1

      @@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.

  • @itsgoubie
    @itsgoubie 2 дня назад +1

    This video made me subscribe instantaneously.

  • @RoamingAdhocrat
    @RoamingAdhocrat 2 дня назад

    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
      @zoran-horvat  2 дня назад +2

      @@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.

  • @MahmoudSaed98
    @MahmoudSaed98 2 дня назад

    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
      @zoran-horvat  2 дня назад +2

      @@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.

  • @Sorc47
    @Sorc47 2 дня назад

    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
      @zoran-horvat  15 часов назад

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

  • @DavidSmith-ef4eh
    @DavidSmith-ef4eh 2 дня назад +1

    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
      @zoran-horvat  2 дня назад +2

      @@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
      @DavidSmith-ef4eh 2 дня назад +1

      @@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
      @zoran-horvat  2 дня назад +2

      @@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
      @stefan-d.grigorescu 2 дня назад

      ​@@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
      @DavidSmith-ef4eh 2 дня назад +1

      @@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

  • @amotkram99
    @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?

  • @JanVerny
    @JanVerny 2 дня назад

    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
      @zoran-horvat  2 дня назад +1

      @@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
      @JanVerny 2 дня назад +1

      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).

  • @samuelvishesh
    @samuelvishesh 2 дня назад

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

    • @zoran-horvat
      @zoran-horvat  День назад

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

  • @DavidSmith-ef4eh
    @DavidSmith-ef4eh 2 дня назад +1

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

    • @zoran-horvat
      @zoran-horvat  2 дня назад +2

      @@DavidSmith-ef4eh That was a good trolling.

  • @user-fs3qr5yg7e
    @user-fs3qr5yg7e 2 дня назад +2

    why not move to f# alltogehter?

    • @zoran-horvat
      @zoran-horvat  2 дня назад +1

      That would open the whole new plethora of questions.

  • @danflemming3553
    @danflemming3553 2 дня назад

    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
      @zoran-horvat  2 дня назад +3

      @@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
      @ghevisartor6005 2 дня назад +2

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