6 ms·
> were created in the name of “separation of concerns” when I hear "separation of concerns", I now immediately ask (internally or externally) "do we need to be
by lowercased 7y ago
> were created in the name of “separation of concerns”
when I hear "separation of concerns", I now immediately ask (internally or externally) "do we need to be concerned about this at all? right now or even years in the future?"
Recent project: untangling something that was written 2 years ago as "best enterprise engineering practices". That dude is gone, no one else on the team understands it because it's insanely over-engineered. This is literally "send a notification email", and there's about 38 core files, another 25 'helper' files, ~20 templates and a custom template parser/nested template thing, and a few dozen node/npm dependencies. This is a 'microservice' running on lambda. literally, someone needs to be able to send a notification that "X is ready", and it's this behemoth that no one understands. They do understand that all the templates and logic were taken away from each caller's context - so all the various teams that used to be in control of their own notification messaging now have to wait for someone to dig in to this other codebase which is basically unmaintained now.
It's got its own custom date parser, because... why not? And there have been random "wrong dates" being sent out for months because... no one can quite trace down the root problem. I finally did, and I'm not saying "no one ever can", but it's under no one's direct ownership, and the people that have tried to take it over have just been overwhelmed with trying to trace through it, make it testable, etc.
And last week I heard rumblings that another team is "rebuilding notifications".
It's as much a failure of management/oversight as it is "overengineering" in general, but at each review step (so I was told), things "looked good", in isolation. No one quite pieced together that this was an unwieldy behemoth until after this guy was gone. :/ (no doubt an extremely typical story, but one which, by now, I would have hoped would be 'less typical').
- Maro 7y agoOne way to keep this from happening is code reviews. Don't show me a presentation, don't show me blocks of architecture, let's open Github and start looking at the code. Nothing formal, just a 10 minute walkthrough and discussion. Unfortunately in many Eng cultures the EMs stop doing this, or do it too late. In my mind this simple idea is similar to the Toyota Production System's "Go and See for Yourself (Genchi Genbutsu)".
- falsedan 7y agopeople feel bad about publishing a PR and being told "don't do it like this" and get defensive, argue about the sunk cost, throw references justifying why their approach is Good Actually, accuse the reviewer of not being serious about software engineering, etc. The real way to project success is egoless programming.
- lowercased 7y agostarted on this project a few months back and have had PRs blocked with stuff like "in this test file, just use one assertion, don't need to use two" "there's too many comments here" "i don't like the extra line breaks in this file" "bookAdminId needs to be bookAdministratorId" these are generally comments just on test files. I've struggled to deal with these because... well... ego is part of it, but when there are bigger structural issues, and the focus is on - imo - minutia (without much acknowledgement that these are, indeed, minor points), it's troubling. To then be told after a few weeks that you're "not delivering" is doubly so. It's hard to take 'ego' out of this altogether, but thanks for reminder.
- falsedan 7y agoYeah this sucks. the egoless approach is to suck it up and do the changes, but also to raise with the team the difference between feedback for implementing the feature & expressing opinions. One thing I liked about Review Board (!) was making comments as 'Issue' (to make it clear this had to be fixed before merging) vs. not (making it just opinion/non-critical feedback). Though in a team of pedants you would find every comment being marked as an issue
- lowercased 7y agowhat's "review board"? also, it's triply bothersome because... i've been doing this > 20 years. that certainly doesn't mean I don't make mistakes, and am never wrong, but it does get tiresome to have whitespace arguments take up time 2 decades in. On top of this, there's actually an automated linter that we all adhere to, but these are 'extra' style things that someone likes to focus on outside of the linter. It's just wasteful of the company's time, more than anything else.
- pjc50 7y agoI wonder how much of this is driven by the interview culture of forcing people to write complex solutions from scratch in isolation.
- draw_down 7y agoAll of that is bad, of course, but it does not strike me as a code or technology problem. Why was that allowed to be built? Who reviewed the code? Was the design discussed or vetted at any point? Answer to last question is almost certainly no, so why not? Et cetera.
- mdpopescu 7y agoBeware Chesterton's fence. [1] Is the network always reliable? Are the messages supposed to be at-most-once, at-least-once, or exactly-once? What happens if power is off when the service is supposed to send the email, and then it comes back: should the email be sent anyway? What if there are five thousand emails waiting to be sent? What if "X is not ready" at the moment, but there's a message waiting to be sent? A lot of that complication might be because, like Spolsky says [2], it "fixes that bug that Nancy had when she tried to install the thing on a computer that didn’t have Internet Explorer". [1] https://abovethelaw.com/2014/01/the-fallacy-of-chestertons-fence/ https://abovethelaw.com/2014/01/the-fallacy-of-chestertons-f... [2] https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/ https://www.joelonsoftware.com/2000/04/06/things-you-should-...