20 ms·
Here's the part that resonated with me most: A particular type of complexity is over-engineering, where developers have made the code more generic than it
by petilon 7y ago
Here's the part that resonated with me most:
A particular type of complexity is over-engineering, where developers have made the code more generic than
it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be
especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be
solved now, not the problem that the developer speculates might need to be solved in the future. The future
problem should be solved once it arrives and you can see its actual shape and requirements in the physical
universe.
Note how Google does NOT say "make sure the code is properly architected". Instead they say "make sure the code is not over-engineered"! At top companies like Google, projects rarely fail because there isn't enough architecture. Instead projects end up costing 10x to 30x because of unnecessary complexity. Over-engineering is a trap that very good developers fall into all too often. I am glad to see Google has cautioned against this, because I can now point my fellow developers to this when they are about to fall into the same trap!
- tayo42 7y agoIf your making mistakes like over engineering are you actually a good developer then if that is part of what bad code is. Maybe you fall into the expert begginer at that point
- abacadaba 7y agoI'll take a noob who doesn't understand basic syntax all day every day vs. the dev who finds a way to make everything complex. At least in the former the damage is limited.
- colek42 7y agoGood dev's understand how to simplify complex problems by breaking it into smaller components. Bad devd add complexity to already complex problems.
- badpun 7y agoBreaking down complex problem does not simplify it, you merely move the complexity to the graph of dependencies between smaller components.
- laythea 7y agoI've seen this a lot. Where it is broken down so much that it involves mental gymnastics to follow whats going on. It also results in Spaghetti-code with fragmented functionality. In my experience, projects using Java are the worst offenders here.
- yoz-y 7y agoIt can happen when a really good coder gets bored.
- pitaj 7y agoThe quote, for people on mobile: > A particular type of complexity is over-engineering, where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. Reviewers should be especially vigilant about over-engineering. Encourage developers to solve the problem they know needs to be solved now, not the problem that the developer speculates might need to be solved in the future. The future problem should be solved once it arrives and you can see its actual shape and requirements in the physical universe.
- baby 7y agoThanks! I agree so much. Zero-cost abstraction is a misnomer.
- uryga 7y agoisn't "zero-cost abstraction" shorthand for "zero-performance-cost abstraction"? i.e. computer, not human, performance?
- baby 7y agoIt is! Which is why I don’t like the term. It often masks the fact that it has a cost in complexity.
- uryga 7y agoi've never read it that way! could you give an example of that?
- ahartmetz 7y agoTwo examples in C++: virtual method dispatch via vtable (in C you'd implement the same mechanism manually), templated generic code. C++'s generic data structures and algorithms are faster than C's because they don't have to use indirection through pointers. The compiler creates specialized code for each type instead. It causes binary size bloat, but execution time is low.
- derekp7 7y agoIt sounds like walking a tight line between over engineering, and falling into technical debt. If you design code specifically solves the immediate need, that may need to be thrown away or extensively worked on / around when future needs come up. On the other hand, you can write code that solves future needs that never appear, and still fail to solve the actual needs that end up appearing. For me, I would rather put a bit of additional effort up front gathering enough information about the direction of the project or usage of it to better formulate an attack plan so that it does actually become more future proof. But even that is subject to failure, so what do you do?
- ehsankia 7y agoIt obviously depend on the language and your tooling too, but I find that it's generally much easier to refactor an under-designed code to add functionality, than trying to refactor a code that was over-designed but doesn't fit the spec anymore. For example, in Python, there's no need to go for a class when a simple function does the job. And once you do need a class, it's fairly trivial to swap one for the other, especially in a good IDE.
- flowerlad 7y agoI can tell you what I do: I allow the total cost to increase by no more than 10% for future proofing. It is all about controlling cost. It is not justifiable to spend more than 10% of the time for future proofing because you have no idea what the future is going to be. Of course if you do have some idea about requirements coming in the near future then it may be justifiable to spend more.
- jchanimal 7y agoIt’s more than a flat cost estimate. It’s more about risk mitigation. Trying to add multi-tenancy to a system that wasn’t designed for it, can mean a major rewrite. Building the system from the beginning with the idea that there could be multiple tenants may cost 10% more, but the opportunity cost of not being able to address the multi-tenant market could be much bigger.
- jonahx 7y agoPreface: I was also very glad to see this called out specifically, and think it's a great rule. That said... > Note how Google does NOT say "make sure the code is properly architected". is not accurate. The very first paragraph on the same page is: > Design > The most important thing to cover in a review is the overall design of the CL. Do the interactions of various pieces of code in the CL make sense? Does this change belong in your codebase, or in a library? Does it integrate well with the rest of your system? Is now a good time to add this functionality? Emphasis mine. But that sure sounds like they care about proper architecture. They just also care about avoiding over-engineering. They're not mutually exclusive.
- flowerlad 7y agoBut that sure sounds like they care about proper architecture. Of course they care about architecture—I don’t think anyone implied otherwise. But at good companies like Google good architecture is a given. Top developers often fall into the pit of over-engineering, and almost never under-architect. So at top companies code reviewers have to be vigilant about over-engineering and rarely have to worry about under-architecting.
- deepaksurti 7y agoSo for anyone, including me, when looking out for the next job should be: - Is `good architecture` a given in this team? - What are they doing to avoid `over engineering`? What are the specific questions one can ask to find out the answers to the above two?
- kortilla 7y ago>But at good companies like Google good architecture is a given. Absolutely not. Google’s interview process and inflow of fresh graduates does not bode well for good architecture. Having spent time at G and FB, I can certainly tell you that employees at both are no better at architecting code in a sane way than SWEs at other companies. Code architecture requires experience. Google does not.
- 7y ago
- deleted 7y ago[deleted]
- CalChris 7y agoThis is for a code review which would be well after architecture and be the entirely wrong place to systematically question architecture.
- codeGreene 7y agoTo be honest, after reading this document, I believe it contain MUCH more information than just how to conduct a code review.
- LanceH 7y agoMy #1,2, and 3 rules are to get the code in the right place. You could vaguely say that is architecture related--is it in the main app, a library, etc... Those decisions can be tough for junior developers and can often be disputed by seniors (we don't need a library yet.) "Architecture" is very vague and used a lot of different ways.
- MichaelMoser123 7y ago# This is for a code review which would be well after architecture and be the entirely wrong place to systematically question architecture. I think that is part of the scrum philosophy: management can question whatever it wants and whenever it wants to; if the timeframe explodes that that is the blame of the developer.
- underwater 7y agoI've been pushing the idea that if you're getting meaningful feedback on design (and over design) in your PR reviews than you've failed. That stuff should be shaken out long before you have working, complete code.
- trgn 7y agoYes! Engineers should collaborate on the overall approach to a code change/addition with the reviewers before any PR is submitted. Those early discussions are extremely important since teams will converge to an appropriate solution quicker because the process is much more informal. These frequent early discussions also build team spirit, and if you're so lucky, individuals will start to click and the team will start writing code and designing software in similar ways. These early discussion are generally more jovial. The actual review can focus on details, and, if feedback was gathered and put into practice early, nobody will mind the occasional nitpick about punctuation and naming. These will feel earned, even welcome, the PR being a final touch of polish.
- pjm331 7y agoOut of curiosity how do those conversations happen before the PR? Asking because we might have a different process for PRs or a different definition of what it means to submit a PR - on many teams I’ve been on it’s been encouraged to publish a WIP PR in order to facilitate these conversations. GitHub recently added a feature to formalize that but before this we used labels such as “WIP” and “Ready for review”
- trgn 7y agoIn person, or for a remote job, with screensharing and online chats. In my current job, it generally starts with the implementer reaching out and saying, I want to achieve xyz and think about doing abc. That's how the conversation gets started. Granted, sometimes you need code examples already ready-made to gather meaningful feedback, but that can be personal branch, one-off patch, gist, WIP PR ... feedback can happen over slack, simple chat, comments on the WIP PR ... As long as the final ready-for-review PR doesn't catch the reviewers off guard in terms of overal design/approach.
- dwd 7y agoFor a proper discussion I'm surprised no one has mentioned this principle from Extreme Programming possibly predates Google as a company. https://ronjeffries.com/xprog/articles/practices/pracnotneed/ https://ronjeffries.com/xprog/articles/practices/pracnotneed...
- jorblumesea 7y agoThat's kinda funny, because their interviews are exactly the opposite. Toy problems you'd never see in the real world, crazy abstracted solutions, O(n) demands, almost encyclopedic knowledge of data structures and algos. Do as I say, not as I do I guess.
- username90 7y agoMaking code execute efficiently is not over-engineering, it is saving many millions of dollars and often a requirement to even launch.
- Shaanie 7y agoI suppose you just wanted to complain, but in case you're serious: interviews and code reviews fulfill entirely different purposes. Choosing the right data structure may or may not be over-engineering, it depends on the application. Google absolutely want developers that are capable of over-engineering.
- kortilla 7y ago>are capable of over-engineering. Anyone can over-engineer. It’s not a compliment nor a desirable quality.
- jorblumesea 7y ago> Google absolutely want developers that are capable of over-engineering. Why, so, when they get on the job, they need to be told not to over-engineer?
- kccqzy 7y agoDoes Google really practice what they preach though? Just recently I was looking at Angular, Google's web frontend framework. Services, modules, directives, angular-specific markup. Coming from React, I find this grossly over-engineered.
- iamchrisle 7y agoGoogler here. In general, yes we do practice it. They are guidelines, not rules tho. Is Angular grossly over-engineered? Depends on how you look at it. Any code's complexity reflects the complexity of its use case and Angular, for example, needs to fit A LOT of use cases at Google. So yea, sometimes, I think it can be a big hammer for a small nail. In other cases, all that abstraction helps. I guess that's the nature of how code eventually matures over time.
- kortilla 7y ago> I guess that's the nature of how code eventually matures over time. Not really. It’s a sign of too many responsibilities being given to one project. Feature bloat is a real thing. As code matures it shouldn’t be getting more and more abstraction and APIs, it should be stabilizing with fewer changes. If the former is happening, it’s a sign it needs to be broken into multiple projects.
- dgellow 7y ago> If the former is happening, it’s a sign it needs to be broken into multiple projects. Breaking something in smaller projects also has its own associated costs.
- Nimitz14 7y agoYeah, based on my experience with android and tensorflow they fail spectacularly at it.
- jf22 7y agoReact does must less than Angular...
- de_watcher 7y agoSometimes you can generalize the problem a bit to get a shorter solution.
- sorokod 7y agoPretty much by definition you will be covering a wider surface of possible inputs and behaviours and so acquiring complexity
- Tyr42 7y agoHere's a fun example from math which requires generalizing to get a good solution to. Suppose you have a 2^n x 2^n sized courtyard. You have one 1x1 statue, and unlimited L pieces (2x2 with a corner missing). You would like to have a layout which places the statue in one of the centermost tiles, and fill the rest with L pieces. ---------------- Solution ---------------- Define a layout function with allows for the empty tile to be in any corner. This is trivial for 2x2, as it's just an L peice. By tiling these squares, you can solve for the next size up. Put three with the hole in the center and fill in with an L piece to get the bigger square. At the end, take 4 2^(n-1) squares and put the holes in the middle. Add one L and you are done. What was the point of that? By generalizing your solve function to have more outputs, it lets you build up a structure from it, in which it's easy to solve your actual goal.
- sorokod 7y agoAnd complex analysis informs number theory. An example from software development might be more useful.
- mytailorisrich 7y agoOverall architecture should have been discussed and agreed before writing any code, not at code review.
- LandR 7y agoIn an ideal world, yes. In practice this almost never happens. Architecture evolves as the code is written. I don't think this is a good way to do things, but it's mostly what happens IME.
- jwl 7y agoYes, I don't think I have ever seen a premature optimisation actually ending up being beneficial when the time came to add new features to the project.
- lostmyoldone 7y agoPremature optimization rarely helps, but well though out flexibility/decoupling in core system components has had a significant positive effect on velocity down the line, and lack of the latter has all been shown to be disastrous. I do believe though that's really hard to discuss effectively as there seems to be no good, and common definition on what over engineering actually is, except in retrospect. I've seen teams where they were so "good" at avoiding over engineering and "architecture astronauts" that thousand line functions with a Byzantine labyrinth of conditional was preferred to even the most basic of design. With that said, what would you consider over engineering of the kind that never works, and in what kind of systems?
- hello_moto 7y agoOver-engineered is not a trap in BigCo, it is simply the by-product of KPI/Goal/Impact driven. People tend to do it to get promo/bonus.
- Smotko 7y agoI have worked on a Google project as a contractor and I can confirm that over-engineering was our biggest problem. I was also partially to blame, but at the time I did not know better.
- afpx 7y agoIt’s an old person thing. We learned to program way before the internet, when one only had a language reference, some vague requirements, and a lot of time. Things took much longer, and communication tools didn’t exist. Maintainability was once a big deal. These days, it’s easier to do a search, grab a snippet, and paste. There’s little need for reusability. Extrapolating a bit, I imagine in a few years, we’ll have one big ‘library’ of functions built into our IDE (built, and copyrighted by Google or Microsoft, of course). Programming used to be skilled labor, but now it’s kind of dumbed down for higher productivity. A natural evolution. Personally, I’m trying to re-train myself for the more modern rapid-fire programming. I’m not completely convinced that it results in better products, but it does feel good to be constantly committing.
- cjfd 7y ago> Over-engineering is a trap that very good developers fall into all too often. I would not call them 'very good developers' in that case.
- DrScientist 7y agoDefer all decisions until you have to make them. Leave the code in a state where it's understandable. ie don't over specify - not just simple things like interfaces, but also things like premature decomposition, that's exposed at the higher level for configurability. While on the face of it, having everything as small interacting functions means to change any function is quite easy, changing overall behaviour might be very hard - hard to understand all the moving parts, and even worse, if the code split is wrong for future needs, very hard to change.
- krona 7y agoThis only works at Google because most of the code is rewritten every few years [1]. The dynamics between tech debt and over-engineering in such a setting are not really representative of the rest of the industry. 1: https://arxiv.org/pdf/1702.01715.pdf https://arxiv.org/pdf/1702.01715.pdf
- ChrisCinelli 7y agoWhen you have very smart people, it may be difficult for those people to write simple stupid code. Smart people like to write delightful, sophisticated and elegant solutions that will address problems and use cases that the code will never go through. A library may be an exception. I used to be one of them. "Growing up" I realized that striving for simplicity and adding just complexity when necessary is the ultimate sophistication. Good engineering values and a shared vision of what "doing a good job" means helps out a lot. If we prioritize "development speed" and "quality from a user prospective", extrinsic complexity clearly becomes a burden. Instead, if these values are not shared, the main motivation for smart people may easily become "ego" and showing off code that may look clever and supporting a lot more use cases but most likely it did not need to be written, wasting time and adding unnecessary complexity with the risk of introducing cognitive overload.
- rhizome 7y agoWhen you have very smart people, it may be difficult for those people to write simple stupid code. If they can't write idiomatically, are they "very smart?" Code is a communication medium as much as a functional one, and if someone doesn't understand you when you're talking to them, such that you have to say, "well I use a cutting edge grammar, aren't you familiar with Lakoffian generative semantics? Imre Lakatos has a sick presentation on Vimeo," you're not maintaining communication skills. I imagine this could be what "unshared values" means. The way I see it, it's like driving: people who don't stop at stop signs, who don't use turn signals, who don't know right-of-way rules are not good drivers. These things are a part of driving just as much as knowing where you're going and trying to run the Nurburgring in under 8 minutes. They also demonstrate a concern and respect for the health and wellbeing of your fellow drivers (not to mention pedestrians, cyclists, etc.). This is not a matter of education, age, or maturity, it's manners. It's saying "please," and "thank you." It's parenting yourself if your actual parents didn't teach you. I'm not sure any of that can be chalked up to surmounting ego. "Ego" is just an excuse and probably not based on actual psychological concepts anyway. the main motivation for smart people may easily become "ego" and showing off code that may look clever and supporting a lot more use cases but most likely it did not need to be written, wasting time and adding unnecessary complexity with the risk of introducing cognitive overload. "Very smart?" ;)
- afpx 7y agoBut, we should also mention that Google also has some of the best software designers and architects in the world. You can't get a well-engineered library without someone experienced at design leading it. All of Google's open source projects have significant design. And, if you look at the commit logs, you can see that their designs didn't emerge from thousands of little pushes. Someone led those projects and enforced design criteria from the beginning. [1]: https://opensource.google.com/ https://opensource.google.com/
- SkyBelow 7y agoThe problem comes in when you can anticipate a problem and also anticipate not being given the resources to solve the problem then when you do have those resources now. For example, had a user who wanted a document produced in one specific format. They promised this was what they wanted and it wouldn't change. It changed 5 times as we were nearing release. So I over-engineered it to allow a template to produce the document that can easily be changed (compared to the release process for a code change). It ended up being used numerous times, despite the problem were given clearly states an unchanging template.
- omnimike 7y agoI would argue that as soon as they asked to change the format the first time then the template solution is no long over engineering as it has been demonstrated that the format can and will change. In general I wouldn’t call it over engineering if the problem you are trying to solve occurred in he past and you have reason to suspect it will occur again. The problem with over engineering is solving problems that have never (and may never) occur.
- jf22 7y agoIf the user did not have that problem wouldn't you have wasted engineering time?
- SkyBelow 7y agoIt was a calculate risk, same as many others, and by far not the most wasteful event of that year even had it not been needed.
- anshumanravi 7y agoLooks like Magento2 could make use of this, every part of it is too over-engineered, adding un-necessary complexity to make it look like enterprise ready application
- gwd 7y ago> where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system. I agree with this when it's internal interfaces. When you have public interfaces that you expect to have to support in a backwards-compatible manner for (ideally) years in the future, it's worth taking some time to think about how people might want to extend it in the future.
- alexhutcheson 7y agoWithin a monorepo like most code at Google, there isn't as much of a bright line between public and internal interfaces. If you don't get a public interface quite right on the first try, but it still has <~100 call sites, it's really easy to just make the change and fix all the callers. If your library has become popular and has substantially more call sites, then it's hard but still doable to fix. This assumes that the interface is a function or method call within a binary. If it's an RPC, then making changes is much trickier, since you can't assume that both sides of the call were built at the same commit. This requires a lot of thought to make sure any changes to your RPC messages are both backwards- and forwards-compatible.
- z3t4 7y agoToo bad the Google developer API's didn't go through that review. 200 lines of code (LOC) for a simple "Hello world" and that is using libraries that are probably a million LOC. Which could probably just be a HTTP GET, and run with a simple curl command.
- growtofill 7y agoAny particular APIs you have in mind? I find App Engine’s ‘getting started’ examples quite sane while they are ‘hello world’.
- z3t4 7y agoTry for example updating a column in a spreadsheet. Or uploading a file to Google drive.