If you're concerned about it you should actually read the code and offer suggestions (sounds like a great time to push for code review!). It may be 3000 lines out of need. Deciding that just because there are 3000 lines means it's wrong or bad is arbitrary.

[Edited based on updates]

You say that the code's speed is not relevant. At this point it sounds like it's just ugly code. The best course of action since you've already given them suggestions (since you're not their supervisor, etc.) would probably be to simply accept it and move on. If you ever need to work on their code it sounds like it's divided up enough that it could be easily broken into multiple functions, but as-is it works and you may never need to touch it.

Work on what your bosses want you to work on and make suggestions and improvements where you can fit them. If you try to fix everything wrong you see all at once you'll stress yourself out for no good reason.

I'd focus on the maintainability issue.

Depending on circumstances, a 1,000 line function that does one thing and is well-documented can be more readable and maintainable than a 20 line function that defers every decision to a ten calls deep stack of utility functions that each had special cases grafted on over time as requirements changed (oddly specific rant, I know).

The checklist for having big functions:

1. Remove the need for users of the function to understand it completely: there should be documentation that treats the function as a black box and only describes its behaviour. User code does not get to rely on anything that isn't documented in this specification, which needs to be an explicit point in code reviews for caller functions.




2. Automate verification of the function. All current use cases should exist as unit tests, so if it ever becomes necessary to modify that function, you can do so quickly with the confidence that nothing else breaks as a result.

Length of a function often correlates with how easy it is to understand, but that is not a hard and fast rule.

I deleted my previous answer because it was irrelevant after the latest edits.

Let's see what we have:

1. Your boss is not programming-literate.


2. Your colleague might leave soon and you'll inherit the project.


3. Your boss wants results yesterday.


4. Your new project is going to be a big, steaming pile of unmentionables.

Cool. Let's start with the boss. He doesn't understand programming, but he understands time and money. He also wants things Cheap, or you'd have more resources, and Fast. You must explain to him what he's getting himself into:

You've already said he doesn't care about quality - which is fine. It's his decision to make. What he doesn't understand is that bad quality prohibits fast work. Eventually you hit a brick wall, the technical debt catches up with you, you just can't write any more code that doesn't break existing stuff. This is where burnout and updated CVs are found. He must understand that software development is a marathon, not a sprint race.

At this point you can mention that feature you rolled out last month, which was rolled out pretty quickly but you're still fixing bugs about it.

How do I react professional to that?

"Great, thanks!"

Now, if you feel the code is sub-optimal, regardless of the number of lines, you can test it on the side to see if it meets any performance requirements. This gives you meaningful, actionable information.

If you think the code follows poor practices, you can bring this up in code-review.

I have worked on legacy code before where the entire website is handled in a single file that is approximately 100,000 lines of code. That's right. Everything about the site is done in a single file, single function. It got to a point where adding or modifying something meant you scrolled all the way to the bottom and simply modified the output buffer to change things. Like if someone said they wanted to change a sentence, we do a regex to search the sentence and simply replaced it with the new sentence.

We eventually got to the point where it became so bloated only a few people were "experts" at modifying the output buffer. It was ultimately decided to simply toss the file, and redo the entire site with a modern approach.

I think that is what will happen here. Maintain the 3k function, and if he goes, simply toss the code. That's what I would do, rather than waste time trying to convince someone something is better. It works, is what the argument is and that might be true. Without a boss who knows code or having a good soft-skill, you probably won't get far with trying to convince your equal co-worker to change.

Preventing problems is cheaper than waiting for them to happen and then solving them. Your boss likes cheap.

Ask your boss if he expects your code to be used for a long time and if changes are likely if customers pay for them.

In the likely event that you get a yes for both then suggest that you would like to have new code you write reviewed by your peers. It won't take them long and the extra time will be paid tenfold because errors are way cheaper to fix the earlier they are detected. An error found in code which is fresh in the mind is easier to fix than one found by the client with the usual less than helpful error reports from clients. Assure him that you won't be asking for much code review, maybe once each pair of weeks. If he asks if you are unsure about the quality of your code assure him that such is not the case at all. But 6 eyes have a wider view than 2 and code review is a standard industry practice because its benefits far outweigh the minimal costs.

If he goes with it, when your pals find errors in your code be sure to mention it to your boss. Mention how much more time would you have had to spend debugging the issue if the code had gone into production. Or the loss of client confidence. Mention how much this team work improves the product even if you work on different projects.

If he goes for this step he will easily make himself the next one: having everyone's code reviewed.

If he is not willing to accept someone volunteering to have his code reviewed then there is no chance you'd get him to let you review your pal's code.

You've said on one hand that each developer is responsible for their own code, and yet you wish to report one of your colleagues to your manager for not working to your standards.

If the code works as expected, there's nothing that needs to be said about it until there's a time when it's a problem or there's a time when coding standards are set for the team as a whole.

A long method is definitely a code smell, but it does not conclusively indicate that something is wrong. In fact, I would argue that you shouldn't break a method apart purely on length, that is arbitrary. For example, I've seen some long methods for distinct ETL (Extract/Transform/Load) tasks where the length is really driven by the amount of data.

Don't report it to the boss at this point. Find a tangible reason why the method can be improved, then communicate that to the developer in a constructive way.

From what you are describing, you have a mountain to climb and a team to drag up it.

I don't think I would specifically talk about the 1k line method, I would start by bringing up best practices with your boss in a 1:1. Ask him if the team has any coding guidelines or best practices that they follow. Assuming the answer is no, gather some links to some articles on best practices for whatever programming language you are using. I try to stay with coding guides from big companies... companies everyone will have heard of like google, Microsoft, etc. and start with their coding guides.

Bring those to your boss along with some articles about how implementing best practices helps... what are the benefits, etc. Don't bring your message, you are the messenger. You bring glad tidings of ways to be more efficient, save money, have fewer defects, and the list goes on...

I'm thinking your boss would react better to that approach. Then once you get him hooked (hopefully), you have a team discussion about them and let's start following them. (I would throw in some procedural best practices like code reviews and the like also, not just coding guidelines.) Then if you can get them to buy in this far, then you start applying those guidelines and reviewing new code (and old code as you run into it).

"Hey, I found this large method that according to best practice ABC, we should split up into smaller methods that have a single responsibility, etc." and go from there.

To be honest, I doubt you will get far with any of this but this is how I would approach it.

You have two points here:

• In your current position, it's not your problem, you said that your boss ask this coworker to finish the code fast, so he did, leaving behind those 3000 lines with a lot of technical debt, you can mention this to your boss (he doesn't understand programming, but the concept of debt is something that everyone knows) just explain that while the code works, if something breaks or the coworker who write it leaves the company it will take time to understand what he did, he might understand but probably even if he understand that, he will say that is not a problem now, because it's not a problem, at least not yours, what bring us to the next point.




• So, the coworker leave the company and now is your problem (you mention that there are 3 developers, so even in this case could not be your problem), well, (I can talk from my experience) do not touch it unless is necessary, talk to your boss again, remember him that first talk, but don't start refactoring that code until your boss approves or the s#!t hits the fan, because if you start to refactor that code, it might be easy but there is a high probability that it will take you time that your boss expect to focus on something else.

A few things to take in consideration, even if its a small company with a boss that don't understand how software development works:

• As other answer mention, while your boss don't know about development, he know about business (I hope so) so the best way to get him understand why code reviews, unit testing and other practices are necessary is to tell him that these are investments, less bugs, less production downtime, etc.


• I don't know who is the senior in your team, if is the one with the 3000 lines, you might have a hard time convincing your boss that what he is doing is not the best approach. This applies to if you are the one with less experience.


• You might want to talk with your coworker, and try to understand why he did that, he mention that is optimized this could mean that the code works, as you says, and that is fast (you might not take speed relevant but for business sometimes it's a huge deal) but he knows that it has technical debt and he plans to fix that, he just follow your boss command to finish that fast.

I think the getting hit by a bus example might be the best.

I would go to your boss, and say "I have concerns about the codes maintainability, if the employee gets hit by a bus or otherwise unavailable it will take me ## weeks to figure it out. Are you ok with me being unavailable to work on other project for ## weeks if this project ever need maintenance if other person is not available.?"

Let your boss make the final decision on which is more important for this project "it s done" or "its maintainable"?

For all we know the boss could be yay, the customer will have to pay us 2x as much because if they want the modifications done they will have to pay for it.

This means that the most important thing to the boss is what will the client pay for.

Don't worry about it. It's not your problem (right now). It's this guys responsibility to maintain the code, not yours. Don't touch it. If your boss asks you to touch it, whether now or later, tell them why you won't.

If your colleague leaves, you buy a book about refactoring. That's the point where you tell your boss this function isn't maintainable.

If the boss doesn't know anything about programming then I wouldn't bother telling him. By the sounds of it, your co-worker will do that for himself, especially since he's bragging to you.

The code needs to work, that's all your boss cares about. Of course, size and speed are important factors if it's a large project but if it's small or medium sized, working is good enough.

Ask your co-worker to either show you the code running or for the code itself so you can check it.

What is the function trying to do? How complex is that task? What optimisation category did they aim for (speed, accuracy, usability)? Without any of that information it will be hard to make a judgement. You can always take a look at the code. If there are inefficiencies, point them out like 'The code is good! Quickie [timesaver/performance booster/whatever], you can always do X to avoid [bottleneck]'

I try to use a static analyzer to do this. Feedback from a computer can't be wrong, and that way you don't have to personally confront coworkers. You just have to agree on code rules or take a basic set of existing rules.