I have a co-worker who said his 3000 line method is the most optimized possible. How do I react professionally to that?
Should I communicate this to the boss, who does not know anything about programing?
Note that we are a small team of only 3 programmers that are at the same level and each one...
Actually, not really. "lines" is not a meaningful measurement in most cases as simple formatting can greatly influence this. Can you count statements or operations instead?
On modern processors conditional branches can cost a lot of time. There can also be gains from organizing code to allow time for loads to complete before using the data. Loop unrolling is a common optimization that reduces conditional branches and allows more code overlap at the cost of increased code size. I would look at your colleague's code to see if that is what they have done before assuming 3000 lines cannot be optimal.
Do you mean perhaps refactored? Sorry for the constant questions, but we have to get a good picture to give a good answer. Also: Is this typical of your colleague or is it just the one method?
Even 1000 lines for a single method isn't really maintainable. it may be "optimized" and if that is critical then fine but if it isn't on the critical path, the method is garbage.
Of course the method probably does "every calculation", its 3000 lines long. You are misunderstanding; some code in say an airplane control panel may be critical it executes in 0.001seconds whilst say listing users in a database for a view isn't "critical" it executes in 0.001s. So basically, if the 3000 line method is believed to be "speed optimized" (and I bet it isn't regardless) I would leave it. Otherwise I'd rewrite it as not being maintainable. if its too hard to refactor the code is bad in the first place.
Let's not massively frame-challenge and pile on a user who made a first post. The post is emotional and lacks details - nothing that can't be fixed by the magic of editing and asking questions. Let's assume the method has no business being 3k lines long, as is the case for most methods of most software, and that the premise of the question is correct.
(1) If you are the person's superior, obviously fire them immediately and have the walked from the building. (2) if you are not the superior. it's a "non-question" ... just do your best, or, find a new place to work without idiots.
"Length of a method" has utterly no relation to speed of execution. Please don't comment if you know nothing about the topic under discussion.
The question is being completely ignored. I know the method is completely ugly and imposible to maintain. What I want to know is what to do about it, should I tell to my boss the problem or should I just ignored it as long as I don't have to touch it and it work "fine" (having in mind everything that y described in the original question including edit 1 and edit 2)
@angelhumberto , I have put in the correct answer. However, don't forget man - the decision is your boss's decision. Boss may listen to what you say (ie, in my answer) and then the Boss may say "I want to keep it like this." If so, don't be a whiner. Basically do what the boss says - make it work boss's way. You then do two things (1) demand massively more pay (you're the only competent person there, so you will never be let go) (2) then, once you get more pay, seek another job with better pay. Enjoy!
As I already said, the fact that is 3000 lines method is more than enough to be SURE it is not optimized as a whole concept, why? Simple, because you can not touch a 3000 line method whit out making the earth earthquake. If that's is not enough to you I had seen the code and there are completed block copy and pasted with some variation
Often, in business, code optimization is not a worthwhile pursuit. If I'm paying someone to write code and they choose to completely rewrite something that is proven to work then I have to question whether that was money well spent. Sometimes it is, sometimes it isn't. You should think about this before pursuing further. Maybe there's a business case for throwing that code away and rewriting it. However if that case doesn't lead to a lower costs than just leaving it in place I doubt you are going to get anywhere.
What is the cyclomatic complexity of the method? Having that number at least gives you something objective to talk about, and gives you a way to measure if refactoring is making improvements. It's widely accepted that higher complexity code is harder to understand and maintain without introducing bugs.
What is the life cycle of the code and who going to use it? If it code you are going to use internally, and therefore need to be more maintainable. Is the code staying, or is something needed for 6 months, and then be obsolete? If the code is being paid for by a client, when the client stops paying the code becomes obsolete. If its for a product line that is being discontinued, then maintainability may not matter. Your manager is the one who needs to make those decisions.
What did your coworker optimizer for? Optimizing for runtime speed, optimizing for space, and optimizing for maintainability typically yield different outcomes.
I was in a similar position. My tech lead wrote 300 line methods that did everything from validation through business logic to presentation. Simple changes required weeks. No shared validation meant tons of security issues. Every single field had SQL injection. No one cared when I raised concerns. In the eyes of the manager, the tech lead delivered functionality while I complain about hypotheticals. I regret trying to fix a broken company by myself. I should have accepted their prioritization, only filed practical issues ("Apostrophes break our app"), and found a better company sooner.
@thatotherguy 300 lines is not an excessively long method. It sounds like part of the problem is that the function wasn't long enough to do everything it should have, especially if it contained validation and SQL queries.
@jpmc26 No, it absolutely is excessively long. One function should never do all of these things. It should be <10 lines, something like pseudocode validate(request); fetch(request.id); doThingWithIt(thing); return toResponse(result);. This has huge, immediate, practical benefits, even when you ignore things like readability and testing.
@thatotherguy I'm not saying that the functionality could not be factored out. I'm saying that 300 lines to do everything you describe is ridiculously short, regardless of whether it's in a single function or spread out into more. Hence it was probably straight up missing code that needed to be there. The fact it was missing is a bigger problem than where it goes, even if spreading it out somewhat makes for better organization.
That said, I've absolutely written 300 line methods that really didn't have any good way to break them up further. That isn't particularly rare.
In other words, the length is not really the problem in the situation you describe. The poor abstraction, missing functionality, and bad security practices are the problems.
@thatotherguy As am I? Not sure why you think I'm not.
Unless you mean because I'm referring to the validation and SQL query building and execution? All those would happen per web request, meaning they'd be in individual request handling methods, with the exact details differing per request as well. A single request implementation maps to a single method in a typical web framework, and one could feasibly implement all the logic for it in that one method (despite the fact that would be a bad idea).
There was a pile of boilerplate setting up DB connections and defining classes and such, which is not counted. After that, you can do a SQL query and extract some data in 5-10 lines, right? Why would 300 not be plenty to fetch and compare user data, query several tables, do some logic, and populate a response?
@PatriciaShanahan modern compilers have been doing loop unrolling automatically for years, if not decades. And they do it better than programmers. Doing it manually is actually very likely to yield slower program.
Discussion on question by angel humbe…
Discussion on question by angel humbe…