• MagicShel@lemmy.zip
    link
    fedilink
    English
    arrow-up
    16
    ·
    edit-2
    19 hours ago

    It depends very much on whether it violates SOLID. I do strongly encourage my teammates to use new language features when appropriate, but I would never fail a PR for it, just mention “here’s a better way to do this in the future.”

    But if someone is violating the architectural integrity of the application, that’s something else. If a field isn’t final when it could be, or is public when it could be private, or uses a class that technically works but is intended for a different purpose, or just adds unnecessary complexity, I won’t approve those until they are addressed.

    That being said, you do have to pick battles. The current app I’m working on is a bit of a mess, architecturally, and sometimes I’ll work with someone to try to adopt better standards and it turns out doing something the right way is just too far afield. Just this week I spent an hour trying to help someone get it right before conceding we should just do it quick and dirty and address the structural problems in a more deliberate way once the underlying structure was ready to support that.

    Coding is as much art as science. Sometimes we like code to look different, but at the end of the day if something is isolated and passes tests, it’s fine for now and can be rewritten in the future should that become necessary.