I am unsure whether I am lazy or sensible
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
There is nothing intrinsically wrong with LINQ. My only concern would be that LINQ does a lot behind the scenes, and could be less performant than more explicit code I would measure the performance (resource usage + speed) of the old and the new code. If your code is more performant, you have new ammunition on your side, otherwise it's back to the drawing board...
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows. -- 6079 Smith W.
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There is nothing intrinsically wrong with LINQ. My only concern would be that LINQ does a lot behind the scenes, and could be less performant than more explicit code I would measure the performance (resource usage + speed) of the old and the new code. If your code is more performant, you have new ammunition on your side, otherwise it's back to the drawing board...
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows. -- 6079 Smith W.
In the end I did the performance measurement. The gist of it is that the code has no impact on overall operation (a very involved case took 30 seconds, while the bit I edited took 600ms, at worst, or less most time), and the compare the new and old code together, I was surprised that one bit I changed for speed was occasionally slower by 20ms, while an other bit, which I only changed for readability, was consistently 400ms faster.... Anyway, I posted my measurement, and stopped working on it, including not going to commit anything unless directed too....
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
I think developers lacks soft skills usually. Who is rude to comment the code should not be in the position of doing code reviews, or working in a team, but that should be addressed by your team leader/tech leader. Btw I don't think a code change without solving any issue (but only readability) should be approved, and readability is very subjective, so if not in agreement with tech lead you should not do that. Any code change could introduce bugs, so why taking the risk? I don't think I will accept your PR. I use tons of linters and static analysers to make sure devs follows coding guidelines, I'm pretty autistic there. For example in Typescript I disallow the use of "any", but many complain about that (not sure why). I like LINQ, it makes complex code super easy to read, so I would like to accept your PR, but to what risk? :) "Bad performance" in a 30sec task (so it'a not ASM or low-level stuff) that is called sporadically is *for sure* not because of linq, usually you can get a boost with a complete refactoring or different approach.
-
You could also just improve documentation/comments on the original code based on what you figured out. The comments could include the linq code as a pseudo code if that is easier to understand than the original.
100% agree with englebart here. You took the time to figure out the code, refactor and comment what you discovered so the future you can save time. LINQ is slower but, not that much slower. Compared to how much time future devs might spend trying to grok the code (again) the readability of LINQ is well worth it. Plus, the optimizers are getting better all the time, so the perf diff appears to be shrinking. Check it in. Now, did you mention adding unit tests?
-
I think developers lacks soft skills usually. Who is rude to comment the code should not be in the position of doing code reviews, or working in a team, but that should be addressed by your team leader/tech leader. Btw I don't think a code change without solving any issue (but only readability) should be approved, and readability is very subjective, so if not in agreement with tech lead you should not do that. Any code change could introduce bugs, so why taking the risk? I don't think I will accept your PR. I use tons of linters and static analysers to make sure devs follows coding guidelines, I'm pretty autistic there. For example in Typescript I disallow the use of "any", but many complain about that (not sure why). I like LINQ, it makes complex code super easy to read, so I would like to accept your PR, but to what risk? :) "Bad performance" in a 30sec task (so it'a not ASM or low-level stuff) that is called sporadically is *for sure* not because of linq, usually you can get a boost with a complete refactoring or different approach.
To be fair the ticket was mostly "investigate that code" and my finding was "the code is fine", my refactor was to help me understand what was going on. And I submitted it because I found it "better", but do nothing was an applicable outcome. Anyway, after a good night sleep, I got over it and did some measurement of the various code measurement on an expensive dataset. Where Cut took about 4 seconds, and Paste took about 31 seconds. Turns out that one little bit I thought was a performance improvement went from 1 milliseconds to, sometimes, 20 milliseconds. Peanuts, but definitely slower (I was puzzled). Also turns out, another bit I only refactored for clarity, went down from 600 milliseconds to 175 milliseconds (and that was for the "fast operation" that only took 4 seconds, hence 10% improvement). So those changes were a go in the end! ;) Oh, and I also demonstrated that LINQ as it was had no performance cost (this much was obvious, but helped reduce LINQ bad performance reputation a bit, I hope)
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
There was a piece of code we wanted to look at, to possibly remove it, or hunt a suspected bug. It turns out it seems hard to remove and and it seems to work. It's just that it fires multiple data event instead of just 1. Which is not really a big deal at all. So there is no really need to change anything. While studying the issue I did a few things to improve readability (although I suspect that people disagree with me about readability, even though I consistently reduce the number of line of code I refactor... I gave up on convincing anyone), and also, from a cursory glance (without measuring mind you) I intuited that performance would probably be better at best, or same. Anyway I made a review, and it's not so much that people asked me to measure the performance of my changes, is that I got the feeling they instantly dislike my changes (I used LINQ! damnation)(for be fair I could use a foreach loop instead).. anyway that just demotivated me totally about this whole change request. And maybe I misread it and they don't care just think this is a piece that need measurement before changes - also the guy commenting.. is not communicating very well, at least with me, either he dislike me or has autism or something. Anyway I am demotivated now, it's not really important, I don't really want to spend time justifying it, I rather just delete the change request and move on. But I look at my reaction.. And.. mm... well.. the question is in the title. I know. I think I will simply forget about the CL. If the commentor ask me again, I will replied I thought he didn't like the changes nor they seemed important but I could go ahead and measure the difference now, then!
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
If you ever meet someone who loves and embraces changes I think they're either worth getting cozier with or very much not. It's not always one or the other in all cases. But I'm pretty sure it's one or the other in any distinct case. The bigger point maybe being that people do generally dislike change. Everyone in the boat will have a degree of apprehension if you go messing with the bottom of it. Maybe even more so if they don't have the time to fully reassure themselves you're patching vs drilling.