My senior coworker rejects my code on the basis of not being optimal

[-]

cs-brydev@reddit

>he just rejects it outright because it works slower than it could be programmed to work. It's hard to give a definite response here without more context, but this might not be a legitimate reason for rejecting code. If there is a performance requirement it should be added to the ticket requirements with some metric guidelines. If he wants to add that, that's fair, but he can't just arbitrarily claim the code isn't optimized enough. On real-world projects "good enough" is often sufficient. In your situation, if I were in his shoes I would tell you exactly what the performance requirements are, to force myself to give an objective guideline. But honestly what's more likely to happen is I'll accept your code, deploy it, then create a new ticket for later to go back and improve the performance of that code. That will let your code get deployed already but also give him a chance to consider what the priority should be. It's pretty common to just add a backlog ticket for code improvements like that and just come back to it during down time. That's better than holding everything up.

Reply

[-]

timwaaagh@reddit

We write non optimal code all the time. Unless it actually causes a performance problem it doesn't matter. In fact trying to do everything in an optimal way produces unreadable unmaintainable garbage.

Reply

[-]

kapiteh@reddit

Unreadable code has nothing to do with the speed at which it runs. He has an algorithmic problem not a coding problem.

Reply

[-]

timwaaagh@reddit

I always wonder whether people who hold this opinion have experience optimizing things or not. Here's an example from my game. First I had units stored in a list. Collision detection was done through iterating through the list, then iterating that through a list again. Initially I thought I'd get away with it but unfortunately it turned out to be a performance bottleneck. I put all the units in a custom data structure based on the location of the unit so I only needed to check a few of them for each unit. That's a custom data structure with pointers etc that's quite complex instead of a simple list. This code may not be unreadable but it's harder to read. Of course this is just one example but usually the optimal solution is significantly more complicated, harder to read and more difficult than the easy one. Now imagine if I did that to my entire codebase. Quicksort vs bubblesort is another example.

Reply

[-]

lostinspaz@reddit

turns out that things called “functions” used in conjunction with something called “comments”, can make even very complex code understandable enough.

Reply

[-]

timwaaagh@reddit

I would encourage using those, especially functions.

Reply

[-]

UdPropheticCatgirl@reddit

“Harder to read” is impossible to quantify and if the application is performance constrained, performance should always precede it. And the Bubble vs Quicksort argument always feels weird, Quick and Mergesort get used specifically because they are easy to read especially compared to Introsort which is usually the actually correct algorithm to use.

Reply

[-]

kapiteh@reddit

Again you’re talking about the optimizing the algorithm Which has nothing to do with how the code is written or maintained Good code reads like sentences from a book, uses common patterns with logical unit modularity and self documents Unless you have a novel algorithm to solve a problem, the. It needs to be proven and understood (like quick sort is not intuitive like you said) Unintuitive doesn’t mean unmaintainable Until you get to that point, you are just modifying existing algorithms to optimize them to your use case

Reply

[-]

timwaaagh@reddit

Code is ultimately just a way to write down algorithms. in this way, algorithms affect code. the algorithm chosen determines how to write the code, how much code is necessary and its cognitive complexity. in the example, my previous code was a one liner, the new optimized algorithm resulted in pages of code. if i need to change anything i will need to understand quite possibly all of that. maintainablity can be understood as the amount of time it takes to do a certain change. This is why it is less maintainable than the previous code. this is also why you have best practices like 'kiss' and 'yagni', which despite trying to be edgy are important. I do agree with you that it is also important to pay attention to try to purely code level readability and write 'like a book' as you said (to a certain extent). it's just not all there's to it.

Reply

[-]

kapiteh@reddit

That’s not all maintenance is though Most of the time “maintenance” isn’t the time it takes to change a feature internally It’s how the changes to that feature affect the system as a whole, what it’s coupled to, how the moving parts now interact with each other, what it breaks and what is now deprecated or gets thrown out, how it effects the performance of the system A self contained feature or algorithm needs to be understood to be maintained, but that has nothing to do with the code itself A developer can fix bugs on an algorithm without fully understanding it just through good error handling and unit testing with correct input and expected results Once a low-level algorithm works there is rarely a need to go back and change it, as it should be designed to accomplish 1 task and only 1 task efficiently If you need another feature built on top of that, that algorithm will get abstracted into a “black box” eventually Like array.sort() Code like that gets written once and that’s it

Reply

[-]

timwaaagh@reddit

Well sure. maintainability is definitely also how much you break by changing something, because you will need to fix that too as part of your change. This has to do with coupling.I see performance as a separate concern. Deprecation is mostly relevant when upgrading libraries and the like. I don't know about 'the system as a whole' or 'moving parts'. I did not make the assumption we were talking about particularly 'low level' code. Op was telling us about a reviewer who is constantly rejecting prs because 'not optimal'. It could have been any kind of code that is susceptible to O(n) analysis. Which is most code. I guess we differ mostly on whether this code will change. I don't like to make the assumption that things will not change. Of course if you know it doesn't need changing ever then maintainability is not a concern. readability and the ability to understand code are still a concern even if it stays one way forever. it's quite possible that a developer who is fixing a bug in a part of the program using your low level algorithm will find himself stepping through your optimized code. Black boxes are a solution but most languages do not really facilitate this. I wish they did.

Reply

[-]

Iuciferic@reddit

I think in this case, as a general rule of thumb, it probably matters

Reply

[-]

iOSCaleb@reddit

Just for context, the difference between O(n^(2)) and O(n log n) is not small. If you have a list of just 1000 items to work on, an O(n^(2)) algorithm will take 1,000,000 steps, whereas an O(n log n) algorithm will take maybe 10,000 (depending on the log base). And that difference only gets worse as n grows. If you know you can do better, and your coworker is saying that he knows you can do better, do you really need him to hold your hand? Could he be nicer about it? Sure. Maybe he figures that if he holds you to a higher standard now, you'll hold yourself to a higher standard in the future.

Reply

[-]

Justkeepitanonymous@reddit (OP)

Oh I know that, absolutely. I am aware how much time complexity matters. I know he is right. But rejecting my solution somehow makes it look like I have spent my days twiddling my thumbs as opposed to actually working, putting time, effort and investigation. I would have at least appreciated “Okay I see you put a lot of work here, but this will be time-sensitive so look into reworking this using algorithm XYZ”. I mean, this is my first job as a developer ever. There has to be some middle ground where juniors get enncouraged to write code and the application not suffering from performance issues or amounting a lot of technical debt …. Like, you know, every company ever wants only experienced knowledgeable people, but we weren’t born experienced …. we still have to get there in some way.

Reply

[-]

lostinspaz@reddit

change your focus. this shouldnt be about you, it should be about what’s best for the company. Stop trying to get “my code” in, and focus on getting “the right code” in.

Reply

[-]

RiverRoll@reddit

Gettint the PR rejected and asking to do it better, which you say you can, will be the way you gain experience. You underestimate how fast code can degrade, when I started reviewing PR I was very lax at first and in 3 months I was already regretting it. More discouraging than getting a PR rejected is getting months worth of code that needs to be rewritten because it's becoming problematic.

Reply

[-]

Karyo_Ten@reddit

Was it GTA which had a O(n²) level loading code and the fix was easy and they became a laughing stock?

Reply

[-]

Kered13@reddit

You could try sketching up a design doc or pseudocode that lays out the code you're going to write and present it to your senior ahead of time. This would save you time of writing complete working code only to have it rejected and needing to start over from scratch. You don't need to spend a lot of time on it, just enough detail that he can give you the go-ahead to write the real code or show you where you can do better.

Reply

[-]

Dependent-Hour6575@reddit

Right and this is where training becomes very open-ended. I'd work with him and ask him for a bit more hand-holding now and also show them you're improving by taking classes on the side or something. There's lots of good material on Coursera or you can also do a class at a local college or something

Reply

[-]

iOSCaleb@reddit

>show them you're improving by taking classes on the side or something Nobody should have to take a class on the side in order to get a reviewer to give clear feedback.

Reply

[-]

Dependent-Hour6575@reddit

And providing clear feedback is what should be minimally done. That said, if there's things OP needs to learn to do a job, that's on OP and can come from working with their mentors to develop a plan. I love helping new hires, give them as much training as possible to do a job and feel welcomed, but I also have a responsibility to my family, to my own job, and to maintain my health. At some level, something's gotta give as otherwise people will encounter legit health issues. I left my last job of 8 years and took some time off before entrepreneurship as I legit had PTSD from the whole experience.

Reply

[-]

WreckfastOfChampions@reddit

> I would have at least appreciated “Okay I see you put a lot of work here, but this will be time-sensitive so look into reworking this using algorithm XYZ”. I'm genuinely not trying to be rude, there's just not a non-blunt way to put this, but you're asking to be coddled and if you expect that through your career you're going to be very disappointed. Your colleagues are not your second grade teacher. Your colleagues are not going to be looking to pat you on the back to sugar coat and soften the blow with a "good effort, I can see you tried really hard! gold star for that!" before asking you to improve something. If you expect every piece of criticism to be prefaced with a corresponding compliment to make sure you don't feel too bad, this is going to be a disappointing career for you. Your senior asking you to fix something without complimenting you for effort isn't rude or wrong. Effort is a basic expectation

Reply

[-]

Justkeepitanonymous@reddit (OP)

Thank you for this. Actually I spent more than 7 years at two different companies before current job which did congratulate based on effort, regardless of how non-productive the effort was. I’ve had hours upon hours of meetings with previous manager thanking us for work done which most people knew was unnecessary and served no purpose at all. Endless pizza parties and claps on the back for stuff any half-smart person with mediocre Google skills can do. Now I guess I spent too much time working like this and grew to expect it. I see it’s not the norm in this work.

Reply

[-]

N0_Context@reddit

Partially why I love tech is the prevalence of people on the spectrum who don't speak in that way and will just give it to you directly. Some people find it off putting but it's better that way imo.

Reply

[-]

WreckfastOfChampions@reddit

Now, don't get me wrong, even within that there are good and bad ways to provide that criticism. That is fair criticism of his methods. If he just hits the red buzzer and says "Nope, wrong, try again" that's obviously not the right way to provide feedback either. There's a middle ground. You're not wrong for expecting more from him, only trying to say temper your expectations for where he should get to.

Reply

[-]

SourcerorSoupreme@reddit

That's your ego speaking. Just grow up and ignore the noise. You already realize that feeling stems from your senior's attitude, but they are not technically misplaced, so why even spend your time and energy getting bothered instead of making it better.

Reply

[-]

iOSCaleb@reddit

1. Don’t take it personally. Even if this guy is the most miserable weasel of a person who’s 110% out to get you, you getting upset about having your pull request rejected doesn’t get you closer to your goal. 2. Any rejected pull request deserves the courtesy of a comment. If this guy rejected yours without a single comment as to why, and if he did it vindictively, that’s rude and unprofessional. But don’t assume it was vindictive — assume that the missing comment(s) was an honest mistake. 3. Ask for feedback on the PR. If he tells you what was wrong, great — go fix it. If he tells you to figure it out, give that your best shot. Start with easy stuff — are there standards for pull requests on your project, like including a description of the changes, the bug tracking number, etc? Did you follow those standards? Does the code build and run? (Hint: check out your pull request in a new branch so that you see exactly what the reviewer sees; if you forgot to add some files to a commit, you won’t see that if you use the original branch.) If you’ve gone through the basic stuff and still don’t know why the PR was rejected, you might ask your manager; explain what you’ve done and that the PR was rejected without any explanation and you’re not sure how to proceed. 4. Is this developer the only one who will review the PR? It’s not unusual to have multiple reviewers. If you can get someone else to review the PR — in addition to this senior developer, not instead of — they might be more forthcoming with feedback.

Reply

[-]

not_some_username@reddit

The comment is that it’s not optimal

Reply

[-]

un-hot@reddit

This is exactly what I was thinking, I'm struggling with a huge refactor at work because we've discovered that some feature from two years ago is O(n^(2)) under the right conditions. It's our highest priority bug because it's completely dependent on user behaviour and it can take our request time from ~0.04s to over 4s in some cases. I'd be pissed if the optimisations were little things but time complexity is no joke. The senior dev just sounds like they're concerned about scaling the application, and rightfully so.

Reply

[-]

iOSCaleb@reddit

Don’t be pissed if you get a bunch of comments about minor issues in your PR. It’s *so* much easier to fix those things before your PR is merged than afterward, when you’ll have to go through the whole branch/fix/review process again just to fix things that should’ve been addressed in the original PR. Just fix them now and thank your reviewers for their vigilance, or explain why you did it the way you did it it’s intentional.

Reply

[-]

Clandestinity@reddit

As I understand it, OP wasn't getting any comments, instead, their senior just outright rejected the PR. That to me is just not the way to go about it. Personally I have never gotten my PR rejected at work, not even at the very beginning when my code was absolutely horrendous. Instead, all I got was a large amount of comments telling me what the problem is.

Reply

[-]

ayy_md@reddit

That’s because you literally cannot reject a PR, unless he means the senior dev closed his PR? GitHub only allows you to leave comments or request changes (which requires you to comment).

Reply

[-]

iOSCaleb@reddit

Yes, I think I covered that elsewhere, and others have as well. In my comment above, I was responding to the first part of u/un-hot's \*I'd be pissed if the optimisations and PR comments were little things but time complexity is no joke.\* An outright, permanent rejection of a pull request would be pretty unusual unless the PR was unsolicited or the circumstances that led to its creation changed, but it's not at all uncommon for reviewers to ask the author to make certain changes before merging the PR.

Reply

[-]

AdMission1809@reddit

What does PR mean 😭

Reply

[-]

scritchz@reddit

PR stands for 'Pull Request': It's the process that changes should go through before being applied to the main branch (the main code base). During the PR, code will be reviewed, feedback may be given, and issues of the changes should be fixed. In most cases, (well-written) automated tests will help to find new bugs. After evaluating the changes, an authorized reviewer may either merge (accept) the changes into the main branch, or reject the changes altogether. The issue of time complexity of the code was mentioned during a PR; the reviewer rightfully called this issue out. However (according to OP) the way it was mentioned wasn't made in a nice or professional way, if at all. Ideally, both issues and good solutions would be commented in a PR, to give constructive feedback to the developer and fix bugs before they become costly issues. Overall, PRs serve as a small stop to (ideally) prevent the company from releasing buggy code, and serve to both the reviewer _and_ developer as learning opportunities.

Reply

[-]

Legion4444@reddit

Pull Request, in relation to git. In alot of workflows when you are doing work you do it on your own branch of the code, then you do a Pull Request to get it into the main branch. Someone reviews the PR for all the changes you are requesting to accomplish your goal/task. They review whether it actually completes your task, if it's optimal, obvious issues, etc.

Reply

[-]

rebelrexx858@reddit

Sure, but how big is the list 99% of the time? And what are the latency requirements? If it meets them, add a jira to make it better later, put it in backlog and prioritize when needed.

Reply

[-]

iOSCaleb@reddit

>Sure, but how big is the list 99% of the time? I obviously have no idea, and the OP hasn't told us. I don't even know if there \*is\* a list. The point I was making is that the difference in performance is potentially huge. OP has already admitted the they can do better. And on the other hand, if the size of the problem is big enough to matter 1% of the time, using a O(n^(2)) algorithm means that 1% of the time, you're going to have terrible performance. In many situations that's completely unacceptable. Whether it's actually unacceptable in the OP's situation or not, only the OP and their senior developer can really say.

Reply

[-]

Weasel_Town@reddit

Yeah, you can't know whether it matters that something is O(n\^2) rather than O(nlogn) without knowing what n is. There are tons of situations where n is a 1-digit number (pass/fail/error, freshman/sophomore/junior/senior, iPhone/android/other, etc). Then things like readability, not introducing yet more libraries, or shipping before we all die of old age matter more.

Reply

[-]

debugging_scribe@reddit

Juniors always need their hand held...

Reply

[-]

Anewpein@reddit

It's weird, like people new to the industry have to learn more and experience things. God forbid seniors in the industry actually do their job and train them rather than be condescending assholes.

Reply

[-]

MiAnClGr@reddit

What type of developer are you?

Reply

[-]

Key-Replacement-2483@reddit

and it is good you land on a job that someone in your team cares about you for good, and it is going to be beneficial in your learning journey. Another thing is you are working in an IT company or department that cares about code quality and optimization which is getting lesser in time

Reply

[-]

Character_Pitch_4096@reddit

You should consider outlining your approach and asking what better approaches are available. Especially if you know it can be improved. A senior coder may have bigger responsibilities and tighter deadlines. So a good precept to live by is "I am not owed any mentorship. I will work in such a way mentorship is easier to offer"

Reply

[-]

bubblezoid@reddit

He should be giving you a code review. At least in writing if not in a meeting where you can have back and forth. Just rejecting it out of hand with no comments or suggestions is completely useless.

Reply

[-]

wedgtomreader@reddit

Just ask him. If it’s his job to reject, then it’s also his job to work with you to meet his expectations.

Reply

[-]

wedgtomreader@reddit

Just ask him. If it’s his job to reject, then it’s also his job to work with you to meet his expectations.

Reply

[-]

Urodele@reddit

A good mentor will do what you’re hoping for - set aside time to walk you through what can be improved on and why. A good colleague will leave comments on your code and encourage you to reach out with any questions. A poor coworker will behave like your senior and provide no advice, logic, or comment. Try not to hate these people - they don’t deserve our time. Two things you can do: 1) Set up a meeting with the senior to go over your work and ask them questions about how to improve it. Hope they’ll get the picture, or do the same thing in the future. 2) go to your manager and ask for a mentor to help you with leveling up your skills. Manager may then go to the senior and tell them to be more helpful. Or you’ll get a different senior who is nicer! Your manager should be looking to help you succeed so they look better when your work satisfies the requirements their manager gave them. Tell them you’d like a technical mentor and they should respond appropriately!

Reply

[-]

johnyriff@reddit

I have a lot of respect for programmers and I know it's a pretty demanding job that requires a lot of skill built over years of experience. That stated, you guys have a far different culture with the meetings thing than most trades. For me as a HVACR tech I call a journeyman who tells me to fix the fucking thing, and make sure to do it like this. Meeting concluded lol.

Reply

[-]

Urodele@reddit

Your journeyman tells you “make sure to do it like this”. I would assume there are steps in that, rather than the implied “fix the thing, I don’t care how, just be sure it cools by the end.” Which is what the senior essentially did. When what he should have done was provide some steps.

Reply

[-]

Justkeepitanonymous@reddit (OP)

Thank you so much!

Reply

[-]

PervadingVictory@reddit

Exactly, you can tell people to improve constructively without coming off as condescending. His manager may be just using this a pretense to insult him.

Reply

[-]

Jesse_Grey@reddit

> But I do get dishearted when after I work hard on a task given to me by my senior he just rejects it outright because it works slower than it could be programmed to work. lol why? It's literally wrong, so he's supposed to reject it. Step your game up, ask for help, etc. Don't just sit there and whine.

Reply

[-]

y0y0dewey@reddit

In my experience something like this should’ve been caught in design, well before review. I would probably run the design by a senior dev before writing the code to make sure you’re on the same page ahead of time. Some upfront design is both beneficial to you as well as more respectful for everyone’s time (as opposed to rewriting something after a full implementation was cranked out). It’s not atypical for a problematic implementation that wasn’t discussed ahead of time to be outright rejected. I don’t know if your senior had any malicious intent but engaging with him earlier in the development process gives you some agreed upon standard to point back to if this happens again.

Reply

[-]

Chokesi@reddit

I think we've all encountered that one senior that behaves in such a way. I hope you have a mentor there to guide you at least? For what it's worth, at least you're completing tasks, pushing commits and creating pull-requests to the codebase. I commend you for that.

Reply

[-]

No_Help809@reddit

Theyre just gatekeeping. Colleagues do it all the time. They will block your solution for any reason while asking you to auto-merge theirs without question. Push back on them. Ask them to make a benchmark showing that the performance is not good enough for the expected load. Make it look like they're blocking forward progress.

Reply

[-]

Legitimate-Earth3266@reddit

Agreed!! Something I’ve learnt being a Jr was to push back this type of behaviour. Some people will give you constructive and insightful feedback. Others will be pedantic just to be assholes!

Reply

[-]

No_Help809@reddit

I genuinely appreciate hearing from people like you. I wish you the best in your career. I'm a "senior" (whatever that means) with 10 years of professional experience (once again, whatever that means). It is very refreshing to work with people who get things done and take a balanced and reasonable approach, instead of the politics and pedantry that plague this industry. There are some amazingly talented programmers out there with less years of experience. There is zero correlation. Thanks again!

Reply

[-]

throwaway6560192@reddit

If we know there is a more efficient solution, it isn't gatekeeping to not accept the inefficient one.

Reply

[-]

No_Help809@reddit

When you give me a ticket, if you don't give me a performance requirement, I am implementing the ticket at a reasonable pace with reasonable coding standards. If you don't tell me what the standards are, you will just be slowing the COMPANY and the USERS down later when you needlessly delay my PR. How would you like it if 17 people had a second opinion while the doctor was cutting you open on the surgery table? Go with the solution that gets it done within time constraints.

Reply

[-]

Frontal_Commando_89@reddit

don’t put your personal worth in your work - as a person and an engineer. no one’s above a little code review thrashing, just know that 1. you’ll get better, 2. it’s not a reflection of your worth

Reply

[-]

Rustam_Rustam@reddit

Bro, n log n time complexity usually means there is somewhere happens sorting then two pointers or binary search or constant operation. It depends. Check what you can sort to optimise your solution.

Reply

[-]

Bushwazi@reddit

You have to ask. Stop it.

Reply

[-]

nderflow@reddit

"The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming." -- Donald Knuth, 1974 Before discussing optimisation at all, the team should have an explicit performance goal. If you have no specific performance target, you have no way to know that your system is already performant enough. If you do have a performance target and you're not hitting it, the team should be profiling the code to find out which specific code must be worked on. Attempting to optimise everything is a naive approach to engineering a software system.

Reply

[-]

UdPropheticCatgirl@reddit

You should never introduce potential bottlenecks. And performance in userland always matters, without exceptions. Also I would interpret that quote to mean stuff like “It’s acceptable to introduce extra variable to stack instead of swapping with just two pointers” rather then “It’s okay to solve at O(N^2) even though it’s clearly O(NlogN) problem”.

Reply

[-]

nderflow@reddit

It's primarily a question of how you invest your engineering time. If your performance requirement is already met, you should invest engineering time in whatever else needs work. Such as reliability, cloud resource usage, usability etc. Algorithmic complexity is often important, but it's also necessary to remember that the theoretical machine on which complexity theory is based doesn't actually reflect real machines that well. For example, it ignores cache effects. Sometimes linear search over a vector will outperform a theoretically O(1) hash table lookup because the vector is more cache friendly, so provided N is small enough algorithmic complexity may lead you in the wrong direction.

Reply

[-]

UdPropheticCatgirl@reddit

> Such as reliability, cloud resource usage, usability etc. I would performance is actually tied to all three of those, especially the latter 2. > For example, it ignores cache effects. Sometimes linear search over a vector will outperform a theoretically O(1) hash table lookup because the vector is more cache friendly, so provided N is small enough algorithmic complexity may lead you in the wrong direction. While I agree with the general sentiment, even modern C++ compilers don’t always handle vectorization well, so while we should be cache friendly (minimize misses etc.) it’s still generally better to go by complexity unless you’re sure that it’s gonna run on CPUs with support of the SIMD required for stuff like this.

Reply

[-]

nderflow@reddit

You have the wrong end of the stick, so to speak with respect to my comment on vectors. A [vector](https://www.geeksforgeeks.org/vector-in-cpp-stl/) is a data structure. [Vectorization](https://en.m.wikipedia.org/wiki/Automatic_vectorization) is a different thing.

Reply

[-]

UdPropheticCatgirl@reddit

Yeah sorry I guess I misunderstood, but isn’t the break even point for the std umap implementation and vector on modern CPUs at about 8 32bit ints anyway? I would almost call it exception confirming the rule, and the whole thing is very system’s programming/C++ specific.

Reply

[-]

Mammoth_Loan_984@reddit

He’s right to reject your pull requests. Suboptimal code fucks up products. He’s wrong to blankly reject them without feedback. A competent senior should help mentor you, or at the very least, point you in the right direction. I’m a self taught IT guy, but I sometimes submit code changes to our dev team. They appreciate this because it makes their lives easier. When I’ve missed something obvious, they’ll usually provide feedback like “in this case, using X would be more optimal than Y. While we have done Y in the past, I wonder if this could be optimised with X?”. This both leads me down a path of self learning and results in a better product shopping to market - this is what true senior development relationships should look like with juniors.

Reply

[-]

Justkeepitanonymous@reddit (OP)

Thank you!

Reply

[-]

Dracux@reddit

If he is a good senior he will call you and show you how things are done, not just reject things, that horrible and not good for anyone.

Reply

[-]

BlackHeartSprinkles@reddit

He sounds like a mad mentor.

Reply

[-]

BlackHeartSprinkles@reddit

*bad

Reply

[-]

nderflow@reddit

And dangerous to know.

Reply

[-]

FuriousRageSE@reddit

I feel both words could be used interchangeable here ;P

Reply

[-]

bigtdaddy@reddit

No he doesn't, this guy sounds like a bad junior

Reply

[-]

moomooegg@reddit

What? He rejects your pull request without any feedback?

Reply

[-]

TokyoPaul9@reddit

Some people are teachers and some are not. Some seniors see training as part of their responsbility (I do). Some do not. Your responsibility is to solve the problems he points out. Code isn't fast enough? Talk to your network, read a book, watch a video. Figure it out--also as fast as possible. Your senior is your client, and clients just want solutions to their problems, like, right now, fast. The sooner you can see that, the better you will be. (Lord knows it took me forever.)

Reply

[-]

superbiker96@reddit

He's a shit senior. A real senior would help you solve the task together, and teach you the right way. Shooting at code is too easy.

Reply

[-]

BornAgainBlue@reddit

I'm a senior, you need to reach out to him. Don't be scared of senior devs.

Reply

[-]

testlabrat1729@reddit

you are taking things too much to heart. Your anxiety is not because of him rejecting your code. your anxiety is because you think that you might get fired because of bad coding. If you don't have that fear, you would gladly takes his opinions and improve your code performance. You might not be able to code better but your perseverance will make the difference. I think that Senior coworker would actually love if you use his algorithm. stop wasting time on that thinking and continue coding.

Reply

[-]

UpstairsNose1137@reddit

I'll tell you what I do as a junior. Before I get my code review, I feed the code I to gpt and ask it to optimise it. It has actually helped me a lot to reduce the time and space complexity as well as remove the duplicate code.

Reply

[-]

tcpukl@reddit

It's shouldn't be just flat reject. They should teach how to do it better.

Reply

[-]

rdeincognito@reddit

Your senior is incapable of improving your code, that's why he isn't helping. Instead, he creates a harder task to you that he himself couldn't fulfill. You'll find more of that type of people...

Reply

[-]

Original_Golf8647@reddit

When I started as a dev I remember it was for a small company and my manager (who was a dev too) merged all my code in, no matter how bad it was. He was more concerned with how quickly it could be done than quality. I'm not even sure he looked at it.

Reply

[-]

noodle-face@reddit

I'm a senior and I think there's two important thinks to realize here. 1. Code review rejections will happen - don't take it personally UNLESS.. 2. If the senior is going to flat out reject a code review, they need to provide both justification for this and positive feedback on what they'd like to see change. If it's like you wrote and they just denied it and said "not optimal", then they did a bad job code reviewing. If I were in your shoes I would ping them and ask them for more feedback. Be willing to listen to what they have to say and justify any changes you think you don't need to make.

Reply

[-]

magpupu2@reddit

How old is this senior? Some are just jerks trying to show others that they are better. I hate those types. Most are just insecure that the new guy can be self taught and now in the field when he have to get formal schooling and get loans etc. They are afraid to teach because if you become better they fear that their job can be handed to someone else. To me when I was learning, as long as you get the logic down, that is the key. You can always clean up the code once you get more experience. When I was in school, one of my professors gave us a project and after submitting it, he pulled me aside and asked me if I paid someone to do it as I am using functions that are not even in his curriculum. I just told him, the textbook that he was using will make my code longer and there are functions that do more than one thing at a time. I showed him the text book that I was using and that was that.

Reply

[-]

untouchable_0@reddit

Your senior could definitely be doing more to to help you learn, and should be helping advise you. However that is more a leads responsibility, and the senior may already be overworked. That being said, O(n^2) is not good. There are very rare instances where you cant find a better solution. You obviously know how to code. Now you have to learn to code better. And it will never stop.

Reply

[-]

Linkario86@reddit

I had 2 Seniors who sometimes pair programmed with me and it honestly was the most stressful shit. 5 years later, I still don't want to do it. They could've been a guiding hand watching, correcting, hinting better approaches. Instead I was told to write faster, stress the fuck out of me, pointed out every typo, which I made a lot of, because, well write faster, while not actually knowing what to do, basically dicdated me what to write while I had no clue what their intention was. How the fuck was I supposed to learn anything like that? Fuck em. Within the first year I looked for a better Job with better pay. I got one exactly a year after I started the first. Better team, nicer people, cooler and more technical Job. It was better. Not optimal but better. I quickly proofed an affinity for clean and readable code and architecture. I even documented the mess they had and made a proposition on how to make it better, which everyone liked, but wasn't really valued higher up. A friend of mine had an opportunity for me, so I switched Jobs again, after 1 year, increasing my income even more. Still a Junior they realised after less than half a year that Junior doesn't really fit my title, so they removed the "Junior" part and increased my pay further. Unfortunatly due to some bad decisions that company went down the gutter. When the Risk amd Legal teams began to jump off, so did I. A good decision as most of the Software Engineers were let go not long after, and I didn't have them to compete for Jobs around my area too. Now I guide others and took all those lessons with me. I think our Juniors and Apprentices like me for giving reasons and hints for improvements in my review rejections. I want them to improve but also think for themselves so I don't give them the Solutions outright, but hint where to look at next to get them where they need to be. TL;DR: get what you can from the current Job to move along to the next, if the current one sucks for various reasons. There is little risk in looking for a new Job while working. Nobody in your company has to know until you give your notice. Just take those lessons so you don't become the asshole.

Reply

[-]

hobbymostly@reddit

When I started as a junior I had a senior dev who would tear my PRs to shreds. I’m talking like 30 comments on it sometimes. He was known to do that to everyone’s PRs but I had to realize I’m not my code. Be humble enough to realize it’s not personal and you’ll grow much quicker.

Reply

[-]

punkouter23@reddit

I hate it too when I can’t get code done and have senior looking at every line. He means well. But it’s demoralizing so I like when I’m the only dev and I can get things done

Reply

[-]

TangerineWaves@reddit

It's okay! Please don't feel bad about code getting rejected during reviews. In fact when I have mentees under me, I usually judge their code even stricter than I do my other coworkers, because I want my mentees to learn while I'm available to help. I'm sure your senior would be happy to help walk you through it. Don't be afraid of him.

Reply

[-]

Afraid-Locksmith6566@reddit

To be honest, i would do the same on his place. A little not optimized code is not that big of a deal, for instance to take a median if it is not some very often used code - use the sort, why not, it wouldnt make sense to write some obsure algorithm just to save a little bit of time. But what you described is not some minor factor but a o(n^2) vs o(n log n) this is straight up wrong alghorithm used, it is like using bubble sort instead of quick sort. To show it on numbers for n = 1024 and log base = 2 n ^ 2 = 1024 * 1024 = 1 048 576 n log n = 1024 * 10 = 10 240 It is orders of magnitude faster to use n log n alghorithm.

Reply

[-]

Tall_Magician735@reddit

That's not how it should work, if he's going to say "this can be better" he has to take at least a little time to explain how. Generally speaking, too, we should be optimising for readability, not speed, if your way is obvious and clear to read, then that will generally be more desirable than faster code that is non-obvious to read. Not all the time of course, if you were making a 3D games engine or something, but generally readability is the target, not maximum optimisation.

Reply

[-]

Nnyoss@reddit

Are your MRs a lot of lines of code? If so, he might just be swamped with other work and not know the best way to answer all the issues. If it's a small MR, he should be able to walk you through some examples or explain it.

Reply

[-]

Justkeepitanonymous@reddit (OP)

What does “a lot of lines of code” mean? Whatever little tasks he gave me, I think none of my solutions contained more than 100 lines.

Reply

[-]

Nnyoss@reddit

I just know that when we had a jr developer at my last position, he would do a lot of research and come up with new structures or different patterns he found online. A lot of individuals on our team would avoid reviewing them because they didn't know where to start, so I would set down with him and help him refactor it.(which I actually enjoyed) Next thing you know 20 lines of code was reduced to 4 or functions were simplified to one domain. I dont know if this is the same in your case, though I've seen it quite often with newer programmers pushing code quickly. Either way, it sounds like the sr developer needs to be more transparent with you on what specific changes are needed and/or what he expects when you send an MR for review.

Reply

[-]

Lookin_for_Light@reddit

try asking him for one thing that you could improve on.

Reply

[-]

Familiar_Bill_786@reddit

Have you tried asking really hard where you can improve your code?

Reply

[-]

Justkeepitanonymous@reddit (OP)

Yeah and I don’t know if he really sucks at explaining things simply or is really set on securing his job so speaks this way on purpose.

Reply

[-]

datbeowulfisreal@reddit

Hi, let me tell you something. I have written comments like that in reviews too, on purpose. I sometimes want to see if the developer will find out whats wrong on their own first. I still write this politely of course and I say that I want them to think alone first. Then, as a next step, I will guide them. There are assholes in the industry but often people are just stressed out. There are also developers - like me - who have to get used to spending a significant amount of time mentoring. Give him a chance, be open and honest. If he turns out being an asshole, hunt for a new job. Developers can only get better by making mistakes. It's the best way to learn. There is absolutely no reason for you to be afraid to ask why this and that is.

Reply

[-]

teacherbooboo@reddit

i was never a junior :) because i came in at the right time when things drastically changed, so the old timers did not know how the new stuff worked. however, you just have to ask them, even if you just ask, "before i spend a week on this, i was thinking i would tackle this using xyz. how would you approach it? can you give me a direction to go? because i don't want to waste time so i though i should check with you first." as far as the efficient algorithms, you could just read a book or take a udemy course. normally you use pre-built tools for sorting and searching so you shouldn't be writing that code yourself ... but it does somewhat help to know the theory.

Reply

[-]

Justkeepitanonymous@reddit (OP)

I am currently (when I have the time) going through a course on DSA but I can’t really see the practical use of them each time so maybe I need a better course. Can you recommend one? Or a book?

Reply

[-]

teacherbooboo@reddit

most are the same... theoretical. one of my colleagues who teaches that course tells his students, "if you ever code one of these algorithms yourself for work outside of this class, i will hunt you down and kill you!" his point being the major companies, like oracle, pay very good people to embed the best algorithms they can in their products. alternatively, there should be a library you can use that has efficient sorts or searches already. for example, c# usually uses quicksort to sort things. what a dsa course is trying to teach you is WHY some things are inherently bad (or good), however, mostly you want to avoid writing sorts yourself ... in c# for example we use the LINQ library

Reply

[-]

dmazzoni@reddit

This is completely normal, but what you need to realize is that you're taking it personally, which is totally normal and human. But you need to learn to not think of it that way. Your senior isn't criticizing YOU, he's criticizing your code. And your code should be improved. Good code is important for the project, and learning to write good code is important for you. Also, feedback like this isn't just for junior coders! Experienced coders get critical feedback too. I've got 25 years of experience and it's still quite common for me to get lots of feedback on my PRs with suggestions of ways the code could be better. That's the whole point of code review. Code reviews aren't an opportunity for you to get kudos for working hard. They're an opportunity for you and your teammates to collaborate to get the best possible result. The point isn't whether O(n\^2) is or isn't good enough this time. The point is that in the future your senior wants you to be able to work on really important code where the difference between O(n\^2) and O(n log n) really matters a lot, because that can be the difference between a task taking minutes vs seconds. It will get better. After 20 - 30 PRs you'll get much better, and pretty soon your senior will start approving things with little to no feedback. That's how you'll know you've learned a lot. And you'll hopefully get positive feedback in other channels (like a 1:1 from your manager) so you'll know you're appreciated and valued, and you'll be able to more easily handle getting feedback in a code review.

Reply

[-]

lupuscapabilis@reddit

Everything is a learning experience. Even as a senior guy now, I send PRs to devs under me on my team, and I often learn things from them to optimize my code. I just take it, implement it, and remember it. You'll be experiencing that the rest of your career, so best to get used to it.

Reply

[-]

AutoModerator@reddit

On July 1st, a [change to Reddit's API pricing](https://www.reddit.com/r/reddit/comments/12qwagm/an_update_regarding_reddits_api/) will come into effect. [Several developers](https://www.reddit.com/r/redditisfun/comments/144gmfq/rif_will_shut_down_on_june_30_2023_in_response_to/) of commercial third-party apps have announced that this change will compel them to shut down their apps. At least [one accessibility-focused non-commercial third party app](https://www.reddit.com/r/DystopiaForReddit/comments/145e9sk/update_dystopia_will_continue_operating_for_free/) will continue to be available free of charge. If you want to express your strong disagreement with the API pricing change or with Reddit's response to the backlash, you may want to consider the following options: 1. Limiting your involvement with Reddit, or 2. Temporarily refraining from using Reddit 3. Cancelling your subscription of Reddit Premium as a way to voice your protest. *I am a bot, and this action was performed automatically. Please [contact the moderators of this subreddit](/message/compose/?to=/r/learnprogramming) if you have any questions or concerns.*

Reply

Reply to Post

108 Comments