5 ms·
I understand that the point of his example is to introduce the reader to a variant of the very classic multitier architecture with layers. https://en.wikipedia
by speedgoose 6y ago
I understand that the point of his example is to introduce the reader to a variant of the very classic multitier architecture with layers.
https://en.wikipedia.org/wiki/Multitier_architecture https://en.wikipedia.org/wiki/Multitier_architecture
But I prefer the first version of the example, because while it's named "complex code" I think it's the simplest version. I think that there is no need to decouple this simple and straightforward function in three functions that cannot be reused independently. Splitting a function in smaller functions make sense where the code is complex, but I think it's important to not overly do it.
- rsa25519 6y agoAgreed. Abstractions are more mentally expensive than a single concrete idea but less expensive than many concrete ideas. When something doesn't need to be abstract, especially in a language where refactoring is easy, it should not be abstract.
- berkes 6y agoOr, how I would put it more pragmatic: the first example is a sure way towards seven slightly different mechanisms to call an Api. Per year. On a team with seven devs.
- mathw 6y agoNot doing the first example is a sure way towards either copying that function and changing it a bit because you need a slightly different call, or a function with a pile of branches in it because it needs to operate in several modes. Both ways could go wrong. I don't care what your architectural choice is, you can still mess it up.
- cies 6y agoI find the Multitier Arch (MA) has much better names. The arch proposed by the article has "Application Business Rules" (ABRs) and "Enterprise Business Rules" (EBRs). Now in a small start-up, what are the "enterprise rules", is "enterprise" not merely a buzzword in this context? And how ABRs and EBRs differ is not well explained. In MA this is much clearer: the Data Access Layer (DAL) contains DAOs (the persistence side of the model in MVC), Business Layer (BL) contains the business/domain logic (the other side of the M in MVC and/or the business logic that may end up in the C of MVC; aka services in Rails), the Application Layer (AL) contains what would be typical "controller logic" (authorization/redirection/data gathering for the presentation) in MVC and the Presentation Layer (PL) contains the V from MVC. > [...] I think it's important to not overly do it. Yups. So maybe one can do without a BL at first, and put the business logic in the DAL at first and dont mind to have a litttttle bit of it creaping into the AL (controller).
- yokto 6y agoI've always been confused by the Clean Architecture and even somewhat dismissive, because the terms used are so "not Clean" (from my perspective biased towards startup/indie/freelance). Multitier Arch is already 10 times more appealing just by using more appropriate words.
- ghosty141 6y agoLile the author wrote, the example is only an example, in the real world those functions are way bigger. Such a small function would be fine but most functions are far bigger and often call other functions that again do some IO.
- Tempest1981 6y agoIf so, then please add 2 comment lines above each "chunk", for faster human reading, and everyone will be happy: // build URL // find "definition"
- hyko 6y agoI don't think this is a very good example of clean architecture; this is just a refactoring that extracts three methods. There's no dependency inversion used here, which is one of the key tools you would use to create a clean architecture. The use case itself depends directly on the functionality in requests, which means the business logic is coupled to the concrete details of the requests API. Clean architecture also differs from n-tier in that–although there are layers, they don't have to be physically distinct and the APIs between them are not typically heavyweight gateways designed to work across network boundaries etc. The key design activity in CA is reducing the coupling between abstract business logic and the concrete code that executes it. It's actually quite a simple approach to describe, but more difficult in practice. There's nothing particularly wrong with the first function; functions certainly don't only have to "do one thing" in clean architecture.
- ravenstine 6y agoThis reminds me of Brian Will's "Object-orientation is Bad" where he makes the case that most decoupling tends to be more confusing than long-form code that's got sufficient comments. https://youtu.be/QM1iUe6IofM?t=2235 https://youtu.be/QM1iUe6IofM?t=2235
- zytek 6y agoI love his series. Eye opener for beginner coders.
- ratww 6y agoAgreed. IMO it's an eye opener for experienced coders too.
- ZephyrBlu 6y agoI hate unnecessary functions with a passion and it's refreshing to hear his opinion. Sometimes a big, fat block of code is just easier to understand.
- wsc981 6y ago> Sometimes a big, fat block of code is just easier to understand. John Carmack made once the same comment: http://number-none.com/blow/blog/programming/2014/09/26/carmack-on-inlined-code.html http://number-none.com/blow/blog/programming/2014/09/26/carm...
- valenterry 6y agoIf that is a simple 10 line script, I agree. But if that is part of an application, you will not be able to properly test it - so you either have to do time consuming manual tests or time consuming integration tests. Splitting it into different parts allows to write unit tests and smaller integration tests. Then, when you only change the "pure" parts, you only have to verify the unit tests. That gives a HUGE productivity boost in bigger applications.
- MeteorMarc 6y agoYou only want to test on public API, because else you cannot change the implementation without adapting your perfect unit test on implementation details. So, I prefer the first code example and do not mind some mocking (unless the extracted functions make sense on the public API).
- winkeltripel 6y agoA large application has lots of implementation leak to the API, doesn't it?
- Izkata 6y agoFor this reason, when mocks are involved (that is, just about any external API), I think Listing 2 is the ideal - extract a small function designed around the desired interface, a facade that's easily mockable and won't change with the library implementation.
- valenterry 6y agoIf you take that idea/ideal to the very logical end, then you can only do system tests / acceptance tests (however you want to call it). Your tests must only be able to do what end-users do - everything else would be relying on implementation details. No unit tests at all! ...unless we take a more compositional approach and define our application to consist of many "parts" each of which has a public API and an implementation. Well, in that case I would argue that each function can be considered as such a part, having a public API (the definition of its inputs and outputs) and an implementation that can be changed while keeping the public API stable. Over the time I came reached a conclusion: whenever I have to run some code to be sure it does what it should, I write a test instead. And every often that is a unit test. If I'm highly confident that the code will do what I expect, then I rely on very high level system tests, unless I fear that someone might break the code later by accident without the compiler (or other existing tests) being able to detect that.
- koonsolo 6y agoAs someone who's been coding for 25 years, I agree with you. I want code to be "clean and simple". Thats how I judge my own code and everyone else's. And the first example is indeed cleaner and simpler. In my theory, you are only allowed to introduce more complexity (functions, frameworks, etc), when it makes it overall simpler and cleaner than before.
- mac01021 6y agoYou don't think def find_definition(word): # Listing 3 url = build_url(word) data = requests.get(url).json() # I/O return pluck_definition(data) is more readable that the original code that inlines the definitions of both helper functions?
- koonsolo 6y agoThe original code is 9 lines, and it's very clear what it does. Now you have a 'build_url' method, which is a bad name to start with. It's actually a 'build_def_request_url' or something like that. Imagine you add another method find_synonyms(word). My quick brain would think I can use that same build_url() method. Wrong, because one needs a build_def_request_url() and the other a build_synonyms_request_url(). Let's say I need to debug and need to see what url it's calling. In the original code it's right there, here I have to on a goose chase to find my answer. One thing that wasn't refactored is putting the hardcoded url into a constant. That one I would do. Another complexity is adapting one of the helper functions. If I adapt it, which callers could I break? Who is using this function? Another part is that I would rewrite the pluck_definition(data) to a more generic get_value(data, u'Definition'). That way my other potential find_synonyms() could also use it, and it basically adds no extra lines of code except for an extra parameter. I know this is an easy example to explain things, but the original code needs to be way more complex to justify it splitting up into multiple functions, that in this case can serve nothing else but that original function. Just my feeling towards this code.
- seer 6y agoI usually look at this in the form of “layers”. If you have a lot of duplicated logic, its a lot nicer to abstract those concerns into a “layer” of functions / classes. Whats important is to not have leaky abstractions though. The moment you need to start reading into those functions is the moment you realize you would have been better off leaving the code as is. Its very hard (at least for me) to come up with those layers and requires a ton of iteration. Thats why I also prefer the top example - you need to work with the lower level code first, and go through enough of it before you can start spotting patterns and creating layers, otherwise you’re bound to mess things quite a bit.
- UK-Al05 6y agoMulti-tier is different compared to say hex or clean, in that depedencies for edges of the system depend on interfaces in the "Core". Tradtional mulit-tier the UI depends on Business Logic, Buinsess Logic depends on persistence etc The idea is protect the core, and persistence or UI concerns adapt to how the core wants to interact with the world.
- nahname 6y agoThere are people who cannot use an abstraction unless they understand every line of code behind it. Abstractions are blockers, rather than aids. This lowers the value of abstraction for those individuals. Others find it tiresome to read every line of code and develop intuition around abstractions instead. The "find_definition" function will be all that is parsed. The funny thing is people shift between the two as the scope of the project increases.
- bluGill 6y agoIn a real project - millions of lines of code - there is no chance to understand it all. You have to trust the abstraction does what it says without caring how it works. You only break the abstraction and understand when you have reason to suspect that it somehow isn't obeying your quick mental model of what it does - normally this means you suspect the bug you are working on is down that abstraction, but sometimes it is just the name is bad and so you need to figure out what it is really doing.
- froman88 6y agoI agree that it may be overkill for such a simple function, but the author (at least the original author, Brandon Rhodes) is drawing an analogy to more complex applications. He uses such a simple example so that his idea that applies to much larger, more complex codebases can expressed in a presentation/article, not to argue that all such simple functions need this level of decoupling. https://en.wikipedia.org/wiki/Straw_man#Steelmanning https://en.wikipedia.org/wiki/Straw_man#Steelmanning
- sidlls 6y agoOften this is done in the name of "testability" by the the "(unit) test everything" zealots. I've met some of them who haven't seen a function they wouldn'tsplit up to test different branches or what have you independently. The resulting code is frequently a nightmare of function aliases, mocks and other problematic artifices.