5 ms·

I find myself somewhat reluctantly agreeing. While I think the SOLID principles and similar things are helpful tools for thinking, I think they should in genera

I find myself somewhat reluctantly agreeing. While I think the SOLID principles and similar things are helpful tools for thinking, I think they should in general take a back seat to the simple principle of Less Code. I’ve seen so many over-complicated code nightmares that were created in the name of “separation of concerns”, or some such, that my spine tingles anytime someone starts talking about some abstract design principles.

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 ago
  One 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 ago
    people 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 ago
      started 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 ago
        Yeah 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 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 ago
One 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 ago
  people 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 ago
    started 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 ago
      Yeah 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