Software Developers: Am I right to be annoyed about this?

July 27, 2017 9:40 AM

A developer or team outside of ours was pulled in to work on some new development in our app. Their code is completely inconsistent with ours. Am I correct to be supremely annoyed?

So, you may sense that I'm not that experienced, since if I were I would know the answer to this question. I've been away from working on this app for a while, and now I've come back to find this. It's a java application, and the new development was to add a component with basically the same functionality as the rest of the app, but for a different client type.

What I'm seeing so far is totally different variable naming conventions (variables preceded by an underscore, which is not supposed to be done in java anyhow), and completely different techniques for calling the database. A lot of variables being declared as 'final', which I've only every seen used rarely. There's lots more; I just don't have the language to describe what it is.

I'm feeling really angry and I don't know what to do about it. It feels like our app is split in two now; it completely lacks consistency. Adding to my anger is the fact that I'm having a hard time understanding the new code. I've worked on five applications on three different in this job, and I've been glad and proud of the way that none of us write needlessly complex, 'smartass' code - no offence intended to those who enjoy being clever, but we all have to work together to get our jobs done, guys. Let's make it easy on each other.

Am I being unreasonable?
If not, is there anything I can do about this (ie. try to prevent this from happening in future) or do I just suck it up? Thanks.

posted by kitcat to Computers & Internet (20 answers total)

Response by poster: * I meant to say that I've worked on three different teams here
posted by kitcat at 9:44 AM on July 27, 2017

Totally justified, omfg. Was this person a senior level developer? If not, who is the lead dev on this project? This is something they need to take a look at.
posted by Hermione Granger at 9:58 AM on July 27, 2017 [2 favorites]

You're not being unreasonable, but it would be unreasonable to expect anyone to conform to your standards without clearly documenting those standards. And that's a whole process whereby the engineering org comes together and agrees on those, or it's dictatorial from a manager, and it's all very political and leads to you spending less time coding and more time approving other people's code and blame being thrown all over. At least, that's what I've seen.

I would suggest a frank and friendly conversation where you assume that you don't know why it was written that way, and you would like to ask. I know you're correct, as you're saying the same sort of things that I would. But the fact is that they think they're right, too, and insisting you're the expert is what makes things political.

Alternatively, just use the code, but consider it debt, and put tickets in the backlog to refactor the code. You can even state, in that ticket (as acceptance criteria) exactly what standards the refactored code should meet. That can also make a case to management that they need to be the bad guys; if you have estimated backlog items that state you'll spend three sprints on code debt because they decided to outsource to a team without standards, and you can state why that's crucial, they have to face the fact that they made this happen. They may be inclined to force standardization across teams, and you can be the good guy who helps everyone with documented guidance.
posted by Pacrand at 10:00 AM on July 27, 2017 [17 favorites]

It's good and important to code the various modules of applications in a consistent way. So I think you are justified in being annoyed that that goal has been interrupted.

In "Effective Java", Joshua Bloch advocates making everything that you possibly can final. But this style of programming isn't often used in applications that use frameworks like Spring or Hibernate, in my experience.
posted by thelonius at 10:07 AM on July 27, 2017 [1 favorite]

Ask questions. Just be careful to not be aggressive or rude with your questions.
Approach it with curiosity. Faking curiosity is hard, so I try for genuine curiosity. I always view opportunities like this as a chance to learn some more complex topics (if they're truly writing needlessly complex code) or at least build a more solid understanding of how that person thinks about problems (it makes working through issues with them in the future much easier if you understand how they think). Even if they're wrong, their line of thinking can still be enlightening.

Just a point - even if they have 30 years of industry experience, there's a good chance they don't know or understand what they're doing wrong. Perhaps they come from a C++ background, or perhaps the last team they worked with demanded they do things this way, or maybe they just never had a positive influence teach them anything different. There are so many non-malicious reasons this could have happened that don't mean the person who did it is a complete and utter moron worthy of your scorn.

How do you avoid this in the future? Most teams have coding standards and code reviews to avoid these exact issues.
posted by Krop Tor at 10:08 AM on July 27, 2017 [1 favorite]

Code standards, and a gatekeeper at the final merge into the repo -- if it doesn't pass testing/code standards, the release gets pushed.
posted by dreamling at 10:10 AM on July 27, 2017

it would be unreasonable to expect anyone to conform to your standards without clearly documenting those standards

I woludn't expect a developer unfamiliar with the code base to understand all of your coding standards without such a document, but if you've been reasonably consistent, then they can and should pick up the large strokes simply by looking at the existing code. That they haven't is a red flag that they don't work well on a team, moreso than the code they have contributed itself.
posted by kindall at 10:10 AM on July 27, 2017

Response by poster: So, should I bring this up in our next team meeting? Privately with our (non-technical) manager? The only other long term member on our team is a fellow in his '60s who is extremely passive, probably has extreme social anxiety, and it's very hard to get even a three word answer from him. It's unlikely he will state an opinion, so there is no one to back me up. Maybe someone could give me a script? Honestly, I have only 4 years in this industry and I'm a woman who looks like she's about 12 years younger than she really is.
posted by kitcat at 10:16 AM on July 27, 2017

As a former developer and now a management person, I have a little bit of advice. Remember that your company's (or team's or whatever) purpose is delivering value to your customers/clients/whatever you call them. You're not hired to produce the most beautiful and elegant code, you are hired to deliver value and solve problems. In a well-functioning engineering team, you can do both. And can definitely make the argument that bad or inconsistent code will slow things down in the long run due to tech debt (an important point that many non-technical managers don't really grok). But you do not want to come across as trying to obstruct progress toward the business goals in the name of code elegance. Look for solutions that will improve the code quality without impeding progress. For example, simply saying that no other team is allowed to contribute code might be seen as obstructionist. Writing up style guidelines and requesting that other teams follow them is more productive and collaborative. A lot of it is about framing, especially if the management is not very technical. Go out of your way to not sound like you're saying "these guys did something useful for the client, but I don't like the color of paint they used, so they have to stop."
posted by primethyme at 10:29 AM on July 27, 2017 [15 favorites]

Perhaps focus on the issue of maintainability - if you and the other people on the team aren't familiar with whatever they did differently in dB access, etc, but are going to be expected to fix issues and/or enhance the functionality, that's sort of a problem.
posted by thelonius at 10:40 AM on July 27, 2017 [2 favorites]

Yes it's unreasonable to be "supremely angry", annoyed and frustrated sure, but the intensity is not likely to be productive. If the new team is still around perhaps broach the topic of consistency and standards directly in a very non-confrontational way. Ask about the underscores, which are a bit of a standard in a very different realm. If there were not explicit coding standards provided to the new team prior to their effort, well some it depends on how much they needed to review the existing code before they began. Not a few code bases are a glorious mess, just one of those things.
posted by sammyo at 11:24 AM on July 27, 2017 [2 favorites]

Without knowing your team it's hard to say how to approach it, but if you say something in your team meeting, please be careful not to say it in a way that is either cruel or embarrassing to the team members doing things incorrectly. This is a lot harder than it sounds.
I personally would let things that are done be done, and talk about it from a "moving forward" direction. This is going to require some re-framing from your perspective. You can't think about it as "this code done this way bothers me." Instead you need to be thinking "My team could be more effective if things were done this way."

How I would approach code reviews:
Hey, we all make mistakes/don't always understand what we're working on/could use more exposure to what other team members are doing/have something to learn from each other, so why don't we start reviewing each other's code before it's added to the main repository?

How I would approach coding standards:
It's sometimes hard to switch between sections of code when they're inconsistent or unfamiliar. Can we all agree on some standards for style, naming, and readability, so that we have one less thing slowing us down?

I would make sure that you are prepared to lead the conversation in defining the code reviews, standards, and enforcement. Have a good, clear idea of what you want (I like short checklists) but definitely approach it as a collaboration and not dictation. Look at how other teams do it, and use that as a basis. Be prepared to give up on some, or all, of your ideas, because your team does not like them.
I personally would do them one at a time. I would start with code reviews since you can kind of get both for the value of one there. After a few weeks I would bring up coding standards, since you'll likely start to see them form as a product of (good) code reviews anyway.

I would definitely run what I plan to do and why by my team lead first. I would absolutely be pitching the difficulties as a business-impacting issues like primethyme suggested. In that conversation, I would be prepared with examples of where I, or another team member, was slowed down or blocked by the lack of standards and reviews. Since your manager is non-technical, it may also be useful to have some basic research that proves there is something to be gained.
posted by Krop Tor at 12:11 PM on July 27, 2017 [2 favorites]

It sounds like you don't actually have a sound basis for preferring one style of coding over the other, and without that, you don't really have any room to complain.

If you want coding standards, than talk about coding standards as a group and formalize them, but being angry because someone decided to ignore your completely arbitrary set of coding conventions in favor of a different completely arbitrary set of coding conventions is pointless.
posted by empath at 12:42 PM on July 27, 2017 [1 favorite]

As primeThyme mentioned, "technical debt". Read up on it. The fact that you can list specifics (leading underscores, "final" variables, 2nd design of db access) gives you credibility.

Analogy: you run a fleet of delivery vans, delivering information, goods and services to your customers. For reasons your boss knows you have Chevrolet delivery vans. The consultant bought a Ford van. It's easy to see why - it's what he knows, it's easier for him to be productive with what he knows. But he has left you with a time bomb! Now you'll have to learn how it differs from your Chevy's, learn how to maintain it, etc. He has doubled your work.

Propose that you'll need to gradually convert the foreign stuff over, reduce the technical debt. Doing so is more work in the next few quarters, but in two or three years, you'll have half the code base, and therefore half the work required to keep your project from grinding to a halt. See "big ball of mud", "Turing Tar Pit", "Accidental Complexity".

He has "complected" the system. It's going to cost you to simplify it, but that'll be cheaper than constantly paying for the duplicated functionality, the cost of maintaining 2 software architectures.
posted by at at 12:47 PM on July 27, 2017 [1 favorite]

A couple thoughts:

1. It's certainly true that the polite thing for him to do is to adapt to the conventions of the codebase he's working in. Ignoring those conventions is rude, regardless of whether they are written down.

2. Some of the examples you gave were cases where the choice of convention matters relatively little (how often to specify things as final, naming conventions for identifiers.) In cases like this, a lot of programmers don't take the responsibility to be polite and adopt the prevailing convention. Sometimes, they don't care at all about those decisions, and if you just point out that everyone else standardized on camel-case and they standardized on underscores and you care about the difference, they will change to accommodate you. Other times, they are very opinionated and care a lot about their particular favorite convention.

3. If you get really annoyed by this you will be annoyed a lot in your career as a programmer. It's up to you to figure out how much you're willing to fight about it.

4. The kinds of things you mentioned shouldn't preclude you understanding his code. If you don't understand what they wrote, then try to understand it first before trying to address it with them, or else they won't take your opinion seriously. If his work is so bad as to be really incomprehensible, then tackle the comprehensibility first, and then separately address style and correctness.

My suggestion in this case, assuming that the code is comprehensible, your conventions are visible, and this person was just not making any effort to follow them, is to mention them and (if possible) propose a patch to fix their code to follow the conventions. They will either thank you, not care, or refuse to accept the patch. In the third case then you can figure out whether you want to try to involve management or your coworkers to coerce them into writing better code.

Definitely don't go straight over their head without talking to them, or be really visibly angry with them over a first offense of this sort. That's not kind and it probably won't get them to do better work in the future, which is your goal, right?

My message to them would be something like "Hey, the existing code in this repository all does X and Y, and your code accomplishes a similar thing but with A and B. X and Y are typical for reason blah blah blah -- do you mind if I change it over to use X and Y?"
posted by value of information at 3:51 PM on July 27, 2017 [1 favorite]

you do not want to come across as trying to obstruct progress toward the business goals in the name of code elegance

one man's 'elegance' is another man's 'this section really needs loose coupling because it is a likely point of change in the medium-term.'

so if the new code impacts coupling poorly, for example, get that in the backlog. same as mentioned above for 'style' elements.
posted by j_curiouser at 5:29 PM on July 27, 2017

Here's one possibility to consider before you charge in: Some of their changes may have solved critical problems in your code that you aren't aware of. Until you can explain why they did things the way that they did, you don't know whether they made pointless changes that merely increase confusion, or purposeful, necessary changes. You might say, "You completely changed how we access the database!!", and they might respond, "Yeah, and the code we added doesn't have SQL injection vulnerabilities and character set conversion problems like your code does."

That would be the first question I'd ask them: "Did you use a different style just because it's what you're used to, or did you see problems in the existing code that you were trying to avoid?"

Once you've got the answer to that question, you don't have to suck it up. Maintaining code consistency is a well-known problem with well-known solutions that others have outlined above. It's not something that will ever happen automatically, though; it's a constant battle with entropy. It's work, with its own set of required knowledge and skills. If that's what you'd enjoy spending your time on and feel is important, then you'll need to put the effort into learning the cat-herding skills required.

One way or another, this is a point at which you have to put in the work to learn and grow as a programmer: Either your technical skill and ability to understand and appreciate new coding styles will grow, or your social/managerial/process skills and ability to keep other programmers on the straight and narrow will grow. (Or, ideally, both.)

Anger won't protect you from what you're afraid of. Growing your technical and social skills will.
posted by clawsoon at 2:06 AM on July 28, 2017 [1 favorite]

You gotta ask.

I'm having a hard time understanding the new code

"Hey person, do you have any free time in the next week or so to go over your additions/modifications to this application? I'm having a hard time understanding the new code and it'd be really helpful to get your perspective before any future development happens. I'm specifically curious about some things like your approach to database access and variable naming conventions and why they might make more sense than what (we|I)'ve done in the past?"
posted by czytm at 2:01 PM on July 28, 2017 [1 favorite]

And then if you don't like their response(s):

"Ah see, we chose to do a, b, and c because of x, y, and z. To me that makes a little bit more sense for the project, is there something I'm missing?"
posted by czytm at 2:03 PM on July 28, 2017 [1 favorite]

It is hard to say without seeing the code and the difference between them. I have worked on enough projects started by less-experienced developers, where some of what you describe could be code I added; particularly marking members as final in Java code, and using a database abstraction layer instead of building SQL commands via concatenation and pulling individual values from rowsets. I have also seen a number of developers who had decades of experience and built code with the same problems. There are lots of ways to build experience, but experience is not a magic cure for making silly mistakes.

Yet. Your feelings are your feelings, and they are legitimate. The question is really about how to handle those feelings, and that can be tricky, moreso from your self-description. You would have a much higher chance of being perceived as incompetent, where the same response from a person stereotypically associated with coding would get a pass.

Asking about why specific choices were made is one of the safer options. But avoid asking about too many things in a short period! That would sound like a rant, and makes you easier to dismiss even if every point is valid. Things like the different ways of accessing the database are good candidates for this approach, especially if it is using package B vs the existing code using package A. Maybe A is an older version, maybe B has a specific feature, or maybe B is just what the other person/team is used to. Things like marking members as final or prefixing private members with an underscore are more contained within that section of code, and will not get as much benefit from asking questions.

If the project does not have a style guide, getting one will help reduce future style conflicts. Pitch this as reducing everyone's cognitive burden by getting rid of the mixups with camelCase vs underscore_split.

Something else to consider: is it now really two apps? Maybe it is easier to make it so, than to reconcile the differences.
posted by jraenar at 2:16 PM on July 28, 2017