23

I work for a small wholesale company as a developer. It is just me and one other. I am way younger than him and he is considered my "senior". I put that in quotes because I soon realized what his level of coding is. I really don't want to sound condescending or rude in any way and really don't want to offend him as we've become pretty good friends out of working hours. But, I feel like his way of doing things is impacting the quality of our software in a big way.

I have 5 years of experience and consider myself knowledgeable enough to spot bad code, practices and code principles not being followed. Every time I try to point out stuff he is doing wrong I get dismissed and ridiculed by him as in "What do you know about coding? I've been doing this for X years longer" which is ridiculous. Mind you, I am trying to present the issue as non offensive or rude as possible.
"Hey man, this is running a bit slow, could we maybe try and run this async so the UI doesn't freeze up?" The answer I get is "Who cares", "It doesn't matter", "They can wait" and so on.

I really don't want to affect our friendship but this is really starting to impact the final product.
How can I handle this properly ?

Edit:

Some of the problems and bad practices I found out are:

  • Concatenating queries - no parameterized queries (big no no)
  • Really bad code writing - You could see TextBox1, ComboBox17 ... ButtonN for days with no real context nor connection. Only hours of digging even to find what they all do.
  • No real error handling - users might get errors for typing text into numerical fields that will crash the whole app or simply display an unhandled exception

I am really not trying to be the young hotshot, just trying to follow some normal code practices and give out a polished product.

Secondary Edit:

We are not selling our software. It is used as an internal development program where we try to simplify and automate as many processes in our retail chain as possible. Most of the people working with the software will be people not really digitally educated thus input errors are really a common thing. Also, image their reaction when a random error pops up on the screen if they misplace a , or a . and then everything crashes.

Seems like this is some critical info. I worked for this company before him. He was later employed but was considered more experienced (by people who know nothing of coding) and appointed as my senior. Which is bs. The project we are currently working on started when he got employed.

Hopefully final Edit:

I actually took the advice from down below and started documenting everything that I found(including end user feedback) and in some cases wrote test code that shows the improvement we could achieve, then asked for a team meeting and presented it. All of the data was presented in the format:
this can be improved -> why we should improve it -> how we can improve it -> benefit over the current state. This seemed to click with both my manager(especially) and co-worker. This has lessened the dismissive behavior as my co-worker has slowly been opening up to new ideas and different approaches to problems. We also both agreed to scheduled code reviews for new and already existing modules which is great! Hopefully this will continue and translate into even better practices. Thank you guys for the shared wisdom !

3

11 Answers 11

41

Write great code. Participate in organized code review. Try to remember to focus on the highest priorities -- infinite speedup of 1% of the program takes infinite effort and still produces only a 1% speedup; don't optimize the wrong things, or optimize when you still have serious bugs to fix. Set an example.

Correcting your cow-orker's coding style, if it is appropriate, is the manager's responsibility, not yours.

You can suggest to your manager that specific performance or security issues be added to the backlog of open work items and prioritized along with functional issues.

Per Dean Inge: "There are two kinds of fool. One says 'this is old, and therefore good.' The other says 'this is new, and therefore better.'"

Per Steve Boies: "Make it work. Make it good. Make it great." Implied: In that order.

(And whatever you think of your colleague's coding style: It could be much worse. I worked with one guy who thought C statements should have their semicolon delimiters at the start of the statement rather than the end. Particularly bad because, at the time, the debugger we were using highlighted the line with the semicolon, so in his code it always pointed to the wrong statement. I think when he left we promptly reformatted all his code to get it back to normal...)

2
  • 2
    Comments have been moved to chat; please do not continue the discussion here. Before posting a comment below this one, please review the purposes of comments. Comments that do not request clarification or suggest improvements usually belong as an answer, on The Workplace Meta, or in The Workplace Chat. Comments continuing discussion may be removed.
    – Kilisi
    Jul 30, 2023 at 4:14
  • I remember hearing some ancient tales of why leading your lines with a semicolon made sense in certain older Unix environments but I never got any specifics.
    – Mark C
    Aug 10, 2023 at 14:55
23

So, you think your colleagues style of Coding sucks... Why?

"Because it doesn't follow standards"

Why?

"Because it's hard to read"

Why?

"Because I don't like it"

Why?....

Now - I don't mean this is a negative sense - here is the point I'm about to make...

Where is the business case as to why it needs to be changed?

"Your code is difficult to read" is not a business case.

"The formatting of these variables and the lack of naming convention for objects means that should our Lead developer be unavailable - there is severe business risk if a critical bug is reported in the time it will take to resolve the issue"

"The UI is slow" isn't a business case either.

"Research shows that over 50% of users will close and never use an app if the load times are longer then 3 seconds. Our BAU load times are already 2 seconds - if we have any load spikes, we potentially will be loosing 50% of our customers"

Then there is the flipside...

Once you've presented the business case (to your management not your co-worker) - you have to have some solutions.

"Best practices for naming conventions are XYZ, it would take maybe X hours of effort at most to bring all the objects in-line with this convention - this would drastically increase code readability and reduce the business risk in the event that coworker isn't available"

2
  • 1
    Comments have been moved to chat; please do not continue the discussion here. Before posting a comment below this one, please review the purposes of comments. Comments that do not request clarification or suggest improvements usually belong as an answer, on The Workplace Meta, or in The Workplace Chat. Comments continuing discussion may be removed.
    – Kilisi
    Jul 30, 2023 at 4:14
  • 1
    I don't know about a business case, but some people are ok with creating and selling garbage and some are not. It is not so much a question of a business case but what you are willing to put up with. Aug 18, 2023 at 20:33
22

The problem is that you and your coworker have an insurmountable impedance mismatch. He's just an employee who does the bare minimum to collect a paycheck, whereas you are a professional who builds software with thought and care. He doesn't care about coding standards or doing things right or improving his skills, whereas that viewpoint is an anathema to you.

I have worked with such people in the past; they ultimately grind you down and break your will to be a good developer, because the most difficult thing to fight is apathy. When you know that every attempt you make at improving the software that both of you work on, will be undermined by your coworker's lack of care... it becomes difficult to justify making those attempts. When you know that your suggestions will be dismissed solely due to tenure (which is not the same as seniority)... you'll stop making them. Your desire to practice good software development will constantly be at war with your knowledge that what you're doing ultimately doesn't matter, and so you'll lose respect for your craft - and for yourself. And that leads to a dark and sad place.

I know this from very personal experience. I wish I didn't, and I really wish I'd realised it much earlier, but it is what it is. And with that knowledge I suggest the only thing that worked for me:

Find somewhere else to work, that is filled with people who care about building software in the same way that you do.

This is a drastic suggestion, but I think it's probably the correct one for you. With 5 years' experience you're not a wet-behind-the-ears junior sweeping in with the conviction of youth, you're an experienced developer who is able to "smell" the difference between good and bad code. You deserve better, and there are far more companies than you'd imagine where your attitude is the normal, respected one, where your suggestions for writing better code will actually be listened too, and where you'll be able to learn from people who share your drive and desire to Do It Right. It's not always easy to find such places, but given your experience it shouldn't be too difficult.

For the sake of your career - and your health - consider whether you want to continue to be a professional, or whether you want to become just another employee.

6
  • If the circumstances were different I would agree. But, as things are now I would not quit this job for any number of reasons. I do not really have any problems with any other part of the company. The collective is great in and out. The company helped me when I needed it the most and I would feel like I am betraying them by leaving. I am just trying to ensure the best possible quality of the product we output.
    – CodeJunkie
    Jul 28, 2023 at 8:41
  • 11
    Please don't fall into the loyalty trap (I've been there too). The only thing your employer cares is whether you make more money than you cost them, if that ever changes you'll very quickly find how loyal they are to you (spoiler: not very). Businesses operate on facts not feelings, loyalty is not a fact, and you need to make decisions on your career based on what will factually be best for you in the long term.
    – Ian Kemp
    Jul 28, 2023 at 10:15
  • 3
    "He's just an employee who does the bare minimum to collect a paycheck" - he might not see it like that. He just consideres other things important. Jul 28, 2023 at 11:13
  • @ThorbjørnRavnAndersen It's entirely possible coworker is a normal programmer while OP is a prima-donna. But still a decent answer that these styles don't work together. Jul 28, 2023 at 22:39
  • @OwenReynolds Agree that the team is not working well. It could be really interesting to hear the senior developers version of this story. Jul 29, 2023 at 0:32
8

You can't teach someone who doesn't want to be taught. As for how to deal with the current situation, are there objective quality measures you can agree to with both your coworker and your manager? Some simple ones which I'd consider table stakes:

  • Any SQL query involving (directly or indirectly) user input must be parameterised.
  • If it doesn't pass the automated tests, it doesn't get merged.
  • The tests include automated formatting (or verifying that a format has been followed). Choose from any of the probably several popular formatters applicable to the programming languages you use, then stick to it. Only override a default if you all agree it's bad/not applicable.
  • If it's been developed by a single person, it doesn't get merged until at least one other person understands it. That is, all code is either reviewed or pair programmed.
  • Any time a user reports seeing a crash, that is considered a bug to be fixed, including an automated regression test to ensure it doesn't happen again.

As a side note, there are plenty of jobs where a quality mindset will be appreciated. If you show aptitude and interest during an interview and this is appreciated by the technical interviewer, that might be a good sign…

5

I read the situation slightly differently than others around here. There are a few key clues in the question:

  • This is an internal software
  • The company is a wholesaler, not a software company
  • There's just two of you, and you're relatively new there, which means he has probably done this for years on his own

This is a combination with consequences. Just like presentability and hygiene standards start to lax when you stay at home alone for long periods of time, so do coding standards erode when you're the sole developer on a single project for years at a time. Especially if everyone else around you is non-technical.

In other words, there has never been any pressure to make the code any better, so he has, gradually over time, settled down on the path of least resistance.

What's the point of giving variables pretty names if you're the only one who ever works on this and knows everything anyway? What's the point of making it faster, when nobody has complained about the speed? What's the point of fixing a crash on a wrong input when Jenny (the only user for said field) knows not to input wrong characters there? Etc.

Do you even use source control and a bugtracker?

When you've worked at several different places, a pattern starts to emerge - the smaller the company, the more lax the standards. That's because all those high standards only start to matter when you need to coordinate multiple people. The more people, the more the chaos starts to cost you. A single person can perfectly navigate the chaos they themselves have created.

So what can you do about it? Well, nothing systematic anyway. Your team and project are still tiny so there still is very little pressure from the outside for any of this to change. And it's unlikely that the improvements you make will be felt much by the users (if at all). At this point these issues bug you, but that's pretty much it. Nobody else cares.

However - the uncaring goes both ways. In such small internal teams you're not usually strongly beholden to fixing just the things on the bugtracker. If you feel like it, you can just take a part of code that bugs you and fix it - just like that. Well, make sure that you're regular tasks aren't too delayed by this side-project, but as long as the schedule doesn't suffer too much, you should be fine.

Well, at least that's how it's been in most such places where I've worked. All the non-technical people have really no idea how long things take and are usually already used to their requests taking bafflingly long time. So they won't care. Even if someone notices, you tell them that this improves the code maintainability, and they'll shrug and accept it. Except for your colleague, nobody has any way of verifying if what you're saying is true. They just trust you (but also - try not to betray this trust).

As for your colleague - well, don't redo fresh code that they've just written. But if you take something that has already been sitting there for months or years and improve it - it should be fine. Don't hide this from them - they already know ho you feel about it, and they don't need to feel like you're trying to go behind their back. Be open about it - "Hey, this crash here for the wrong input just bugs me bad. Imma fix it, it's simple." There's no need to justify beyond "this bugs me" - because at this point, that's really all that it is anyway. Nobody else cares. But "keep a hand on the pulse" anyway. Make sure he doesn't feel resentful about your changes. Ideally you want a shrug and a "Sure, if it makes you feel better...". You say that you're good friends outside of work and that should help tons.

Will this help them start coding better? Not immediately, I think, but it will start to rub off. Again, that friendship will come in handy. Seeing that you like the code in a particular way, they should eventually start doing that too, just to make you happier. And maybe along the way they'll notice a trick or two that they will like and adopt themselves. Maybe they'll also notice some practices that make it easier for you two to cooperate and adopt those.

I don't think your colleague is stubborn. There has simply never been any reason to do any differently than they are doing.

But beware - if you stay for too long in this place, you too will start to "sink to the bottom". The pull is strong. It's not necessarily a problem for your career, but it can make a larger cultural shock when you eventually do change jobs.

10
  • 2
    I worked at this company longer than him. He was recently employed and "appointed" as my senior. And the project we are currently working on has started since his employment.
    – CodeJunkie
    Jul 28, 2023 at 11:38
  • @CodeJunkie Ahh. That does change things. However then I wonder what is previous employment was like? It could be that he came from a similar place. Hmm... how big is the tech department in your company? You say that it's a small wholesaler. Are there any more devs besides you two?
    – Vilx-
    Jul 28, 2023 at 11:50
  • 1
    @CodeJunkie - Wait, I just noticed... he's ridiculing you? Is that like a good-natured ridiculing like between friends or actually mean? If it's the latter, have you tried telling/showing them that their words hurt and have they changed their behavior? If yes and no, then it's starting to smell like they know their incompetence/laziness, but are trying to cover it up with social strategies. I don't want to accuse them of that just yet though. I've not been there, I don't know what the situation is actually like. Accusations like that can ruin actual friendships.
    – Vilx-
    Jul 28, 2023 at 11:58
  • 1
    True it depends a lot on the dev and the application. If they develop slowly enough, years. If they churn out code like nobody’s business, it could easily be months not years, especially if the code is complex and sloppy.
    – bob
    Jul 31, 2023 at 2:01
  • 1
    @CodeJunkie - As long as there's just two of you, and no other technical boss above, I'm afraid there's very little you can do. It'll be just your word against theirs. And they're supposed to be your senior... Still, I think that answering "Who cares?" with "I care" might be one possible avenue. Sure, it'll just result in a shrug and "Well, you fix it then" - but at least you've got the green light to fix something.
    – Vilx-
    Aug 1, 2023 at 12:50
4

Concatenating queries - no parameterized queries (big no no)

Not universal. I don't bother doing parameteraized queries for stuff that doesn't have user input. Can it be a problem in the future? Sure, but they've modified my query at that point so they're responsible for that.

If it is exposed, then understand the threat vector and code monitoring systems to detect SQL injection attacks, and then let them know the attacks that are being prevented. Sometimes they need to see it happen before they want to fix it. Or maybe you realize the user input was actually cleaned up and nobody hacks it for months, so then you learn something new without being antagonistic.

Really bad code writing - You could see TextBox1, ComboBox17 ... ButtonN for days with no real context nor connection. Only hours of digging even to find what they all do.

This happens, even with my own "perfectly" documented code. Honestly, if it works, and you've figured it out, add your own comments to the variables, and come back in a month. I'll guarantee that you'll still spend much longer than expected to figure things out again. The point is to shorten this "reverse engineering" cycle.

I just caught myself switching between double quotes and single quotes when declaring Python strings... I... don't care anymore so it stays.

No real error handling - users might get errors for typing text into numerical fields that will crash the whole app or simply display an unhandled exception

Log it, and learn how to build reports and monitoring systems for these, instead of insisting that they need to be fixed. A business is about making informed decisions. Maybe they really don't care about users seeing stack traces. Maybe even the users don't care. Most people don't even read error messages anyways, so it may bother you, but I can guarantee you that 99% of users just gloss over anything that has a red X and don't process a single word of it.

This is actually a critical learning experience. Learning how to work with people that has a different perspective and value, and ultimately ending up with a workable relationship, is much better than going type-A and demanding anything from anyone, or sitting in the corner and crying about it.

4
  • 3
    "TextBox1, ComboBox17" - I would expect to see this in code that was generated by a GUI builder. Jul 28, 2023 at 11:14
  • I agree heartily about learning to work with people with different ideas of how to do things, but some of these aren’t different ideas, they’re generally considered wrong (e.g. variables whose names have no resemblance to their function). And concatenating queries with variables not involving user input is still harmful. It wastes the time of security minded devs who will see it and spend time tracing thru the code to confirm there’s not currently a vulnerability, and it normalizes query construction in an unsafe manner for Junior devs who will read the code and may not yet know better.
    – bob
    Jul 30, 2023 at 17:05
  • If it truly is necessary to build a query this way, I’d expect a highly-salient comment block in front of the code in question making it very clear what’s being done and why, and what conditions must be maintained for this code to remain secure. In addition it would be a good idea to include this code in routine automated and manual pen testing since it is at risk for introduction of security vulnerabilities by future maintainers.
    – bob
    Jul 30, 2023 at 17:08
  • This is a code review and doesn't even touch the original issue here.
    – red-shield
    Aug 9, 2023 at 5:14
2

I can relate with your concern regarding this, splitting work and social relation is hard for me. Given you have expressed your concern with him and he dismissed it, probably it was not effective. The other option is of course reporting to your manager. Which can be very tricky since if you suddenly report stuff and only mentioning stuff about your co-worker, it would feel like you are attacking the co-worker.

What I would do, is just let it be until you really had to own it. Yes your concern is valid but, as you are also just his peer and his junior too, it is quite a challenging place to do changes. Just document stuff, and when your manager asked you regarding work and performance, just mention the reason in an objective way if your performance are being questioned. If nothing happens even after your performance suffer, maybe it is true that no one really cares. Then it is up to you to stay and try to change a stagnant lake, or maybe you decide to look for other opportunity.

If your manager cares, it should open the conversation regarding how to improve stuff like you mentioned, just try to be as objective as possible (which is very hard, cause your perspective vs others' perspective is different, probably you already know this well after your exchange with your co-worker)

2

My guess would be that this person has worked alone on this thing for quite some time and now you've come to the show and criticize everything he has done (at least that is what he feels).

If that is correct, you don't have an issue tracker, because there is no need when you work alone. There might also not be version control if you are really lucky. Documentation is probably also lacking.

Just fix that. Suggest moving to github or one of it's clones, as it has both, and then use it for development - including using Markdown to document in-code. When something happens enter an issue, and collaborate on how and when to fix it (if at all). Don't work on anything that does not have an issue so you can document it.

When "master" breaks after a bad push or similar, consider protecting it and introduce pull requests for features - issues can very frequently be used to create branches. This allows you to peer review his code (and he yours) so you can teach each other.

When a broken deployment happens, suggest getting tools to help to avoid it. Leverage Github Actions to make reproducible builds instead of just doing it on your own workstation. Introduce static code checkers and let them catch these things.

Do not suggest rewriting production code just because you don't think it is best practice - it works. Document it instead and influence new code written.

2
  • Do not suggest rewriting production code just because you don't think it is best practice - it works. Document it instead and influence new code written. - It is the bare minimum of what people would consider working code. It always bites us in the back when it comes to user feedback.
    – CodeJunkie
    Aug 1, 2023 at 12:50
  • If you cannot make the new hire be productive, tell your boss. This may boil down to an ultimatum of you or your colleague leaving. You probably should figure out if you are willing to be the one leaving. Aug 1, 2023 at 13:42
1

As I understand it, you are making tools for internal use, not for sale, and therefore the senior coder probably has a perfectly reasonable expectation that quality doesn't matter much, this code may never be seen again, and therefore getting it out the door quickly is better than taking the time to make it high quality.

Whereas your perspective is that as a more junior developer, writing sloppy code is not advancing your career or helping you start your career with good coding standards.

As such, I would approach it that way. Have a sit down chat where you explain that you're trying to get your career started, and you'd really like the opportunity to make and meet some coding standards that you expect future employers will want to see. Ask if it's alright if you redo some code to add async functionality or improve variable names just because you would like the practice.

Might be he comes back and says "No" or "there's no time for that", in which case, welp, maybe time to brush up the resume and start looking, but if I was in his shoes I could see that as a perfectly reasonable request. He wants to write sloppy code for sloppy projects that management just wants out the door and into a few hands quickly to achieve some very specific goal, but you're trying to build a career.

Anecdotally, I do find myself being the senior developer in a team where everyone else is a fresh-out-of-college junior, and I try to bear in mind that they are all building their careers. I try to be available for help without stepping on them, and encourage them to keep to good coding standards and explore code organization concepts that they might find helpful later, even if it's a bit excessive for what a given project actually needs to make it out the door. But your guy might simply not have thought of that perspective, so try bringing it up from that angle.

Not "this code sucks" but "I would like the practice of polishing this because I think that will be good for my career."

-1

You have a subjective opinion on what should be provided and how things should be coded the "right" way and also that the code should be developed to this standard.

He has his own subjective opinion that the standard mentioned is unnecessary to get the job done (i.e. They can wait).

I'm sure there are tradeoffs in his mind that are weighted differently than how you see them.

If you accept that all opinions are subjective then neither his opinions or your opinions can be the "correct" one with regard to this issue.

Given that neither opinion his or yours is "correct" then you can imagine that the only factor left for comparison is his opinion carries with it many more years of experience than your opinion. Yet here you are presenting your subjective opinion to him as if it is somehow objectively superior when that is not possible.

Certainly you could judge modern practices, quicker code, safer code as "better" and "necessary" but there are people who judge an opinion coming from someone with more experience as better. But since its all subjective there isn't a right answer no matter how many people might say that your subjective opinion is objectively correct.

I suggest if you want to keep the work relationship good tell him your feelings of why you would prefer things be done a certain way and why you think its important and listen to his reasoning for the way he does things and his reasons for doing it the way he is and see if there is a compromise that works well given both of your preferences.

5
  • 2
    In this case I would strongly disagree with you. This is not really about a difference in opinion per se it is just plain bad code. There is no reason to implement something as insecure as concat queries when user input is present.
    – CodeJunkie
    Jul 28, 2023 at 19:49
  • It's ok to disagree, we all have "preferences". But there may actually be plenty of reasons to do it his way. Maybe he is right. Maybe they can wait and maybe those security vulnerabilities won't present a problem. Maybe that time he should be skilling up his coding is spent having sex with hot babes. His priorities might just be different. There is more to life than code quality. And it might be those external factors that influence his decision. I think most of us can agree, sex with hot babes is more important than good code.
    – Joseph U.
    Jul 29, 2023 at 23:10
  • I would have to disagree again. What any of us do in our private time should not affect work and the other way around. I couldn't care less if he was talking to aliens in his spare time. He has plenty of time to learn new things at work. In fact, most of the IT industry learns stuff on the go. If someone is busy partying and that affects his or her work efficiency regularly on a level described in the post, that person is not a responsible worker.
    – CodeJunkie
    Jul 30, 2023 at 14:20
  • These are all just arguments as to why you think that you are "right". But the point is no matter how you slice it and dice it this is just your opinion based on your value judgements. He has his own opinions based on his own value judgements. Where you are failing is you have this sense that your values are superior. I suspect this issue bothers you and not so much him. So he may be the one who has the better (if there was a better) way of looking at things that you might want to learn from.
    – Joseph U.
    Jul 31, 2023 at 1:02
  • I get what you are saying, but I think the displayed behavior is not beneficial in any way. There is no upside to a dip in work efficiency because of irresponsible behavior outside of work hours. That is not an opinion, it is a fact. Again, There is no reason to implement something as insecure as concat queries when user input is present. There is nothing that can justify this kind of code in production where you know input errors are bound to happen. SQL injection attacks died decades ago.
    – CodeJunkie
    Jul 31, 2023 at 6:34
-1

You could address these with a mix of neutral code review comments and automated tests, ideally which run automatically as part of the build or post-checkin trigger:

  • For "Concatenating queries": write an automated test which SQL injects a drop table on your primary or largest table. [Edit from feedback: destructive automated tests should only be run against environments it's OK to destroy, and this assumes running automated tests of security features of your own application in those environments is clearly part of your job.]
  • For "Really bad code writing" (TextBox1, ...): in code review, add comment for every single one of these asking "what is this for? Should it be named to indicate that?"
  • For "No real error handling": automated tests which put in realistically possible bad inputs and expect/assert the app doesn't crash.
1
  • Testing should always be non-destructive unless you have specific written approval beforehand. Same applies to any sort of security testing. You can get in legal hot water really quickly if you do things wrong.
    – bob
    Jul 30, 2023 at 17:23

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .