Thank you so much for sharing! Please make a playlist out of this series of videos to make it easier for people to follow, and also useful if you share whether we need to follow the order watching or they are independent of each other.
Hi Jahan, Thank you for watching the video and for taking the time to comment. There is a Playlist called "Tutorials" which includes the three videos in this series. It will be easier to follow the process, if you watch Part 1, Part 2 and then the final video on Testing. I hope you find them useful.
In the middle of the video you discover something you think is a bug, then fix it. In a real legacy code situation that's a really dangerous thing to do. It might be a bug, or it might not. Even if it is a bug, customers might rely on that bug being there. I got really uncomfortable until you explained you would in real life go and talk to users and find out if they wanted this bug to be there or not. Later on you discover that it's not actually a bug, it's a fix that removes a trailing comma. You constructed a test case for a degenerate case where no trailing comma was needed, and that's why you thought it was a bug. If this were real legacy code then a more probable explanation is that the degenerate case never comes up. There's always at least one element. I'm speculating of course, but isn't there a danger this bug you found and fixed is really unimportant to users? I don't want people to think that refactoring is all about finding and fixing unimportant issues! At one point you take the output the code actually produces, and paste it into your test. You are careful to explain that this isn't proper TDD and you shouldn't do that. I interpret that differently. When you paste actual output into the expected value in an assert equals, what you are doing there is approval testing. It's a different style of TDD. It's not wrong, or bad, as you imply in this video. It has tradeoffs, and makes more sense in some situations than others. I would have liked you to have gone over to using a proper approval testing tool for this test. Later in the video you start editing the expected value and get it wrong. If you'd used the tool, you'd have just approved it and saved yourself the embarassment. At the start of the first video you created an approval test without really showing it to us - I'd like to have seen how you came up with the test data for that test case. At the end of the video you talk about refactoring being like clearing your workbench, that you use it to make the code in a state where you can start working with it. I'm a bit nonplussed here, actually. That's what you didn't show us! The code was testable once you put the very first approval test in place. Everything you did after that was to help you understand it, not to make it testable. As a refactoring exercise, I get it, you just wanted to show some horrible code and some techniques you could use to fix it. The second video had some nice heuristics and steps. Overall though, I think you're missing a purpose with your refactoring. It would really have helped to have a concrete new feature you wanted to add, that the current implementation was getting in the way of.
Emily thanks for the great feedback. The point of my video series was to try and demonstrate the uncertainty that is inherent in this kind of exercise. So I didn't rehearse it or plan it. I am not sure that I was clear enough on the trailing comma thing. I should have been more explicit that at that moment I was experimenting to see how the code worked before I decided how to proceed. This would have been clearer, perhaps, if I hadn't take the short-cut of not really committing to save time. There is no way that I would have committed the change with the line of code commented out! Yes, I hummed and harr'ed about showing the creation of the Approval test - I think that I will do another separate video on that sometime. I basically took the snippet of sample XML that was in the comments in the code, wrote a test based on that as the input, then measured coverage and added more XML that I guessed would increase the coverage until it did. Whether or not the code counts as "Testable" based only on the Approval tests is debatable I guess. Strictly I guess you are correct, but I suppose that I fall into the trap of the overloaded nature of the word "Test" in the context of TDD. What I really mean by "Testability" is "Designable Through Executable Specifications". Approval tests don't do that, which is why I reacted against them a bit when I first heard of them from you. I know see their value, but it is not the same thing that you get from TDD. I suppose that by sports analogy, Approval Tests are defensive tests and TDD tests are Offensive tests? You are probably right that the video would have been in better context if I had a planned feature to add. Anyway, thanks again for the feedback.
"Overall though, I think you're missing a purpose with your refactoring. It would really have helped to have a concrete new feature you wanted to add, that the current implementation was getting in the way of." Refactoring by the definition does not introduce any new features. We can clearly agree, that the initial code was unmaintainable and needed a refactor. It isn't a video about "why do we need to refactor". In that case maybe it would be good idea to show why some feature is easier to implement after the refactor. But in case of showing "how to refactor" we don''t need any specific feature to just see how to refactor in general. No matter what the reason of the refactor is, you probably will end up doing similar things.
I must be the only person who's been triggered by the fact this JSON producer isn't producing any JSON :-D Beside the joke on quote, this is a very valuable walkthrough. I'm always struggling when working on legacy code, as I want to remove the clutter and fix the obvious, as I can see in the code it is spattered with bugs. The hardest is always to convince people to allow me a few days to set up unit tests (they don't see the value of unit testing, as they're not using them as for yet). This approval testing praxis is very neat, will use it in the future!
10:55, in C++, I would create a macro called something like: PENDING_substr_in_closeJson, that would control the compilation of (compile or not) that line. So, if I forget about that, I can always come back and read the PENDING list, in some unforgettable place.
Looks like video might have some good information (your videos usually do) but had to give up due to the bizarre screen display which appears deliberately chosen to make it hard to read.
Challenge is when you have to refactor unreadable code without abstraction where everything is coupled and tests are impossible. This is much closer to gambling than programming.
Well this is meant to demonstrate how to do that. I'd disagree that this is "gambling" if you use refactoring techniques, then these are safe, so you can make small, incremental progress toward code that is less coupled and to where tests become possible.
@@ContinuousDelivery but unfortunately adelhi IDE has refactoring support with very limited functionality so it's Just string rename. This is why more gambling than something else :(
@@rolnikz Tools help, but aren't essential. The essential skill is to make change in very small steps. If you want some help on the steps, read Refactoring by Martin Fowler: amzn.to/30ntgaK or Refactoring Ruby Edition: amzn.to/2XGbuhi
Thank you so much for sharing!
Please make a playlist out of this series of videos to make it easier for people to follow, and also useful if you share whether we need to follow the order watching or they are independent of each other.
Hi Jahan,
Thank you for watching the video and for taking the time to comment. There is a Playlist called "Tutorials" which includes the three videos in this series.
It will be easier to follow the process, if you watch Part 1, Part 2 and then the final video on Testing.
I hope you find them useful.
I love your content mate, keep 'em coming
Thanks for the video, loved the series =)
I guess I am quite off topic but do anyone know of a good website to stream newly released series online?
@Kasen Thiago flixportal =)
@Beau Jamie thanks, I signed up and it seems to work :) Appreciate it !
@Kasen Thiago happy to help :D
In the middle of the video you discover something you think is a bug, then fix it. In a real legacy code situation that's a really dangerous thing to do. It might be a bug, or it might not. Even if it is a bug, customers might rely on that bug being there. I got really uncomfortable until you explained you would in real life go and talk to users and find out if they wanted this bug to be there or not.
Later on you discover that it's not actually a bug, it's a fix that removes a trailing comma. You constructed a test case for a degenerate case where no trailing comma was needed, and that's why you thought it was a bug. If this were real legacy code then a more probable explanation is that the degenerate case never comes up. There's always at least one element. I'm speculating of course, but isn't there a danger this bug you found and fixed is really unimportant to users? I don't want people to think that refactoring is all about finding and fixing unimportant issues!
At one point you take the output the code actually produces, and paste it into your test. You are careful to explain that this isn't proper TDD and you shouldn't do that. I interpret that differently. When you paste actual output into the expected value in an assert equals, what you are doing there is approval testing. It's a different style of TDD. It's not wrong, or bad, as you imply in this video. It has tradeoffs, and makes more sense in some situations than others. I would have liked you to have gone over to using a proper approval testing tool for this test. Later in the video you start editing the expected value and get it wrong. If you'd used the tool, you'd have just approved it and saved yourself the embarassment.
At the start of the first video you created an approval test without really showing it to us - I'd like to have seen how you came up with the test data for that test case. At the end of the video you talk about refactoring being like clearing your workbench, that you use it to make the code in a state where you can start working with it. I'm a bit nonplussed here, actually. That's what you didn't show us! The code was testable once you put the very first approval test in place. Everything you did after that was to help you understand it, not to make it testable.
As a refactoring exercise, I get it, you just wanted to show some horrible code and some techniques you could use to fix it. The second video had some nice heuristics and steps. Overall though, I think you're missing a purpose with your refactoring. It would really have helped to have a concrete new feature you wanted to add, that the current implementation was getting in the way of.
Emily thanks for the great feedback.
The point of my video series was to try and demonstrate the uncertainty that is inherent in this kind of exercise. So I didn't rehearse it or plan it. I am not sure that I was clear enough on the trailing comma thing. I should have been more explicit that at that moment I was experimenting to see how the code worked before I decided how to proceed. This would have been clearer, perhaps, if I hadn't take the short-cut of not really committing to save time. There is no way that I would have committed the change with the line of code commented out!
Yes, I hummed and harr'ed about showing the creation of the Approval test - I think that I will do another separate video on that sometime. I basically took the snippet of sample XML that was in the comments in the code, wrote a test based on that as the input, then measured coverage and added more XML that I guessed would increase the coverage until it did.
Whether or not the code counts as "Testable" based only on the Approval tests is debatable I guess. Strictly I guess you are correct, but I suppose that I fall into the trap of the overloaded nature of the word "Test" in the context of TDD. What I really mean by "Testability" is "Designable Through Executable Specifications". Approval tests don't do that, which is why I reacted against them a bit when I first heard of them from you. I know see their value, but it is not the same thing that you get from TDD. I suppose that by sports analogy, Approval Tests are defensive tests and TDD tests are Offensive tests?
You are probably right that the video would have been in better context if I had a planned feature to add.
Anyway, thanks again for the feedback.
"Overall though, I think you're missing a purpose with your refactoring. It would really have helped to have a concrete new feature you wanted to add, that the current implementation was getting in the way of."
Refactoring by the definition does not introduce any new features. We can clearly agree, that the initial code was unmaintainable and needed a refactor.
It isn't a video about "why do we need to refactor". In that case maybe it would be good idea to show why some feature is easier to implement after the refactor. But in case of showing "how to refactor" we don''t need any specific feature to just see how to refactor in general. No matter what the reason of the refactor is, you probably will end up doing similar things.
Nice to see Emily being involved for such a long time. Now 3 years later she finally has her own channel.
I must be the only person who's been triggered by the fact this JSON producer isn't producing any JSON :-D
Beside the joke on quote, this is a very valuable walkthrough. I'm always struggling when working on legacy code, as I want to remove the clutter and fix the obvious, as I can see in the code it is spattered with bugs. The hardest is always to convince people to allow me a few days to set up unit tests (they don't see the value of unit testing, as they're not using them as for yet).
This approval testing praxis is very neat, will use it in the future!
Was the unedited version of the video mentioned in 21:35 ever released?
You can find a longer-form version on my training site here: courses.cd.training/courses/refactoring-tutorial
10:55, in C++, I would create a macro called something like: PENDING_substr_in_closeJson, that would control the compilation of (compile or not) that line. So, if I forget about that, I can always come back and read the PENDING list, in some unforgettable place.
it's amazing how malleable code can be in the hands of a professional engineer, thanks a ton.
Thanks
@6.22 The name of the first method is "shouldTranslateEmptyXMLToJson()" which I think is misleading as I think the test process some XML?
Looks like video might have some good information (your videos usually do) but had to give up due to the bizarre screen display which appears deliberately chosen to make it hard to read.
Can you post a link to the original code so we can try the refactoring ourselves?
I won't set up a repo, but the code that I started with can be found here: shitcode.net/348
This is the reality of legacy code. He didn't jump deep into the rabbit's hole.
Challenge is when you have to refactor unreadable code without abstraction where everything is coupled and tests are impossible. This is much closer to gambling than programming.
Well this is meant to demonstrate how to do that. I'd disagree that this is "gambling" if you use refactoring techniques, then these are safe, so you can make small, incremental progress toward code that is less coupled and to where tests become possible.
@@ContinuousDelivery but unfortunately adelhi IDE has refactoring support with very limited functionality so it's Just string rename. This is why more gambling than something else :(
@@rolnikz Tools help, but aren't essential. The essential skill is to make change in very small steps. If you want some help on the steps, read Refactoring by Martin Fowler: amzn.to/30ntgaK or Refactoring Ruby Edition: amzn.to/2XGbuhi
I love your content mate, keep 'em coming