Refactoring Legacy Code: STEP BY STEP (Part 1)

Поделиться
HTML-код
  • Опубликовано: 15 сен 2024

Комментарии • 62

  • @HuyVu-vi4ut
    @HuyVu-vi4ut 3 года назад +28

    Man! this video has really good content but the way you show the screen of code is difficult to read. Pls change it to normal screen as many other youtuber if you can. Thank you

  •  3 года назад +12

    I had actually never thought of doing something like approval testing as a stop gap measure until you refractor the code enough to be able to test it properly. Very useful tip indeed.

  • @gonzalowaszczuk638
    @gonzalowaszczuk638 3 года назад +9

    I heard it called "Snapshot testing" in other articles/resources online (instead of "approval testing")

    • @edgeeffect
      @edgeeffect Год назад +1

      Thanks so much for adding more words to my Google-fu. :)

  • @ruixue6955
    @ruixue6955 Год назад +1

    2:36 refactor technique: *approval testing* aka. *characterisation testing*
    3:23
    4:40 what happens when you run *approval tests*
    4:43 captures the output from the code that you call
    5:50 approval test is a very useful tool during refactoring when the whole point is to avoid changing the behaviour of the code
    6:06 other thing need to do: run approval code with *test coverage*
    7:46 step 1 of refactoring in the code demo: *reduce clutter*
    8:40 rename method arguments
    9:29
    9:43 rename method
    10:50 IMPORTANT practical tip: focus back the simple steps and refocus on the task at hand
    11:23
    14:34 commit with small refactor
    16:01 takeaway: *work in small tiny steps*

  • @MatteoRegazzi
    @MatteoRegazzi 4 года назад +9

    Thanks for the video Dave. I usually love to see this kind of video also on my iPad beside my laptop but this way I cannot see code, it reminds me I’m ... ehm ... getting old :-). Great topic.

    • @ContinuousDelivery
      @ContinuousDelivery  4 года назад +4

      Yes, I was torn between coding in "presentation mode" and at a more normal resolution. I thought that there was some value in seeing the "shape of the code" which you tend to loose in presentation mode.
      I confess that I didn't think about watching this one on an iPad :(

    • @MatteoRegazzi
      @MatteoRegazzi 4 года назад +3

      @@ContinuousDelivery I also prefer the standard mode especially during refactoring legacy because the shape can reveal structure: clouds of code can reveal logical blocks. Luckily I have also a desktop monitor beside my laptop :-D

    • @Layarion
      @Layarion 3 года назад

      @@ContinuousDelivery ya this is a problem i face when I make video-game tutorials on YT as well. I usually just end up erraring on the side of small screens by setting my desktop to 720p and record at that, then call it done. one thing i hate about my approach though, is YT doesn't treat videos under 2k resolution with the same respect - meaning they give less bitrate ratios to those videos.
      aka, under 2k res videos have noticeably more compression.

  • @Mozartenhimer
    @Mozartenhimer 3 года назад +10

    I wish I had seen this 2 years ago. Sadly I've learned a lot of this the hard way.

  • @George-or3uv
    @George-or3uv 3 года назад +1

    I’ve been looking for RUclips videos like this.

  • @ruixue6955
    @ruixue6955 Год назад +2

    Just notices today that approval test is the also called characterisation test, which was discussed in Working Effectively with Legacy Code :) shame on me that I didn't realise the importance of it when reading the book :D

  • @JohnWasinger
    @JohnWasinger 3 года назад +2

    Check in comments that I hate:
    “Fixed re-indexing problem of the stem bolt conflagers. Fixed mixed use of tabs and spaces in the source code to improve readability.”
    Thank you for fixing the code’s readability but please make that a separate (no change in functionality) check in.

    • @Glinkis
      @Glinkis 3 года назад +1

      Yes, each commit should ideally be described by a single short sentence.

  • @NilsElHimoud
    @NilsElHimoud 4 года назад +2

    Thanks for the hint to approval tests.

    • @ContinuousDelivery
      @ContinuousDelivery  4 года назад +2

      Pleased to be of help, I missed their value for a long while.

  • @MarekAndreansky
    @MarekAndreansky 3 года назад +8

    The slightly tilted text editor is not fun to read.

    • @ContinuousDelivery
      @ContinuousDelivery  3 года назад +1

      Sorry about that

    • @MarekAndreansky
      @MarekAndreansky 3 года назад +1

      @@ContinuousDelivery Its fine, I got used to it after reading for a bit. Thank you for the interesting content!

  • @Layarion
    @Layarion 3 года назад +1

    13:19 I feel like you should have left in that block saying "dead, left it here for compatibility reasons" or something - because someone else looking at the block might be wondering why it's still there.

    • @TheOceanLoader
      @TheOceanLoader 2 года назад

      Yeah I agree - mark it as "obsolete".

  • @MattHeard
    @MattHeard 4 года назад +6

    Thanks for the video.
    I think that this example class had quite a shallow tree of conditional branches, so the video didn't quite demonstrate some of the additional steps that are sometimes necessary with characterisation testing. Getting 85% line coverage with a single test implies that most of the code is quite linear, but often legacy classes have too many responsibilities and too many conditional branches. I've done characterisation testing where I can only get 10% line coverage with the first test, with diminishing returns for subsequent tests.
    Any advice on how to determine what kinds of differences in input to use when trying to get greater coverage before de-cluttering?
    What about when the code is highly coupled to something unreliable like a database or a network connection?
    Thanks again for the video. 🙇‍♂️

    • @ContinuousDelivery
      @ContinuousDelivery  4 года назад +3

      Inevitably any code where I can demonstrate the techniques in a reasonable amount of time will be simpler than a real, nasty production system.
      I picked this piece of code as a compromise, it is bad code, it is complex enough to require some discipline to change it safely, but it is enough to demonstrate how I tackle this kind of problem in an hour or so.
      The coverage I got surprised me too, it was better than I expected.
      My approach to approval testing is to do a few tests that push the code through a few different paths. This code didn't need more than one.
      The next videos in this mini-series will, I hope, demonstrate the techniques, as I say in the video this one really only covers the pre-work.
      Keep an eye on my channel for the next two videos and let me know what you think.

    • @MatteoRegazzi
      @MatteoRegazzi 4 года назад

      Good point. I think you have to read code not covered, then try to test your assumptions adding tests and variations on input. Maybe the component you're working on isn't SRP compliant, maybe it's just convoluted, these are some reasons you are reafactoring (of course, in order to get the code readable/tetstable/maintanable). Seems to be a chicken-egg problem, but the first characterization test (or a Golden Master test) will be your chicken, so you can start to test your assumptions having a sefety net, even if with it is a net with very wide links that you are going to shrink.

    • @MattHeard
      @MattHeard 4 года назад +2

      @@ContinuousDelivery Thanks for the reply. I'm really looking forward to the next two videos.

    • @ContinuousDelivery
      @ContinuousDelivery  4 года назад +2

      Yes, exactly. Actually in this case I did that by varying the content of the XML file, but stupidly, I didn't record that part of the exercise. Doh!
      I started off with lower coverage, I think it was 30 - 40%, and then bounced between the code, to try and identify what input data may push the code down different paths, tweaked the XML and tried the test again. All the time tracking the coverage.

    • @MattHeard
      @MattHeard 4 года назад +2

      @@ContinuousDelivery I think this intuitive input-tweaking is the hard part of characterization testing that I find difficult to teach. I would love to see a video where you mess around with test inputs for 10 minutes. 😄 It would be very informative.

  • @OthmanAlikhan
    @OthmanAlikhan 3 года назад +1

    Thanks for the video, very practical advise =)

  • @Warpgatez
    @Warpgatez Год назад +1

    This video is really well done minus the slanted preview screen. It’s disorienting and the animated background isn’t needed. More distracting than anything. It would be better to just have the full IDE take the screen and your camera a little smaller in the bottom right over the IDE.

  • @Eamo-21
    @Eamo-21 3 года назад

    This is very useful. Thanks Dave

  • @Layarion
    @Layarion 3 года назад

    CD, i'm a dumb C# noob and aside from fiddling on a personal project and Unity, that's my only experience.
    My point being, I wanted to try and do this myself with the code you provided on Git before watching the video...but i download the ZIP, and have no idea how to even navigate the file structure, find what i'm looking for, or even what i'm looking for without a VisualStudio project file to open.

  • @JorgeEscobarMX
    @JorgeEscobarMX 2 года назад +1

    Code text is way too small. But a great video so far,.

  • @kinddata
    @kinddata 4 года назад +2

    If am using an untestable legacy platform like vb6 & stored procedures. Should I bake a self tests into the main code?

    • @ContinuousDelivery
      @ContinuousDelivery  4 года назад +2

      There are some references to unit testing VB6 so maybe worth bit if a trawl with Google. I found this on a quick look brainbaking.com/post/2016/12/vb6-unit-testing/
      What I have done in the past, when working with older tech, is to write my own, simple version of (x)Unit. It isn't all that hard if you keep it simple.
      I have seen some unit/functional tests used for Stored-Procs, not ideal because the DB is there in the back, but better than nothing.
      Good luck!

    • @kinddata
      @kinddata 4 года назад +1

      @@ContinuousDelivery thank you. Its inspiring to see what others have done with vb6. I think I will try make one class called TestPoint inside the main code, and get a test runner to call it often via, activeX createobject.
      I look foward to your next video.

  • @matthewtrow5698
    @matthewtrow5698 2 года назад +1

    Great video, but there's a few other options to consider:
    1. Run away.
    2. Hide and hope someone else picks up the end of the stick with poo on it
    3. Look for a new job
    4. Rebuild it from scratch
    4 is the best option, if you are afforded that luxury - you are then in the realm of "green field" development, everyone's favourite, plus you can make the same mistakes all over again ... (joking ... I think) ...

    • @edgeeffect
      @edgeeffect Год назад +1

      3 is the best option, if you do 4 they'll only break it again.

    • @rich63113
      @rich63113 Год назад

      Rebuilding small chunks of code is usually fine - but rebuilding a legacy application is almost always a giant expensive failure - because yeah - because any legacy application has a ton of hard fought fixes in it, and undocumented features.

  • @ajmeyer66
    @ajmeyer66 2 года назад +1

    When I think of "legacy" code I tend to think about older languages like "COBOL",BASIC,"DBL/DIBOL". Procedural style languages without the notion of "Objects/Classes". That is a whole other world. The current language I am working with (Synergex DBL) does have the notion of "Classes" but the code base was developed prior to that feature and essentially doesn't use it. Some newer code that has been introduced does use "classes" but 99% is simply subroutines and lots of goto's. Very difficult to refactor and generate "tests".

    • @edgeeffect
      @edgeeffect Год назад +1

      I'm dealing with "PHP-3 style" PHP-5 written by amateurs.... I DREAM of a nice, simple bit of CoBOL to get my teeth into... I used to LOVE CoBOL!

    • @MatheusAugustoGames
      @MatheusAugustoGames Год назад

      Characterization tests may be the way to go in your situation, given the lack of structured programming. What tooling do you have available to you? Testing and code coverage are great tools to have on your toolbelt. Depending on your refactoring tooling, reengineering may be the way to go. Also remember to use static analysis tools -- specially on older codebases!

    • @ajmeyer66
      @ajmeyer66 Год назад

      @@MatheusAugustoGames Unfortunately not any really. Code is essentially run in a Linux Terminal screen, old "Green Screen" VT terminal emulation. Just a compiler and linker, multiple code base instances, development, testing, user acceptance testing, production. Most developers used VI editor to do the actually coding, VERY old School.
      Since I am a contract programmer my mandate is to simply work on modifications requested and to do it as quickly as possible, no "redesigning" the world. Not worth it to me to rock the boat, a few more years to retirement so not much desire to fight the bureaucracy anymore.

    • @MatheusAugustoGames
      @MatheusAugustoGames Год назад

      @@ajmeyer66 That's very unfortunate for the coders that are gonna come after you're gone. I bet that if you tried, your talents could be used to make their lives easier by leaving the code in a better state than when you arrived.

    • @ajmeyer66
      @ajmeyer66 Год назад

      @@MatheusAugustoGames I do my best to refactor as much as I can. I have also built a few "Utility" functions and created documents to describe how to utilize the utility functions. Whether or not the utilize the tools or not is out of my control. We are however developing some "AutoGen" scripts that will read the source code and then build "wrapper" programs that can then call their built up library of functions. This will allow us to create in-house test harness programs that can call the utility functions via the "wrapper" program and pass test parameters in and evaluate the responses given back.

  • @diegopego1069
    @diegopego1069 4 года назад

    Thanks for the video!

  • @Farren246
    @Farren246 3 года назад

    Great to view older videos and see your own continuous improvement. Lots of background hiss in this one, poor green screen and poor voice quality, some fixed by 5 months and all fixed in recent weeks.

    • @ContinuousDelivery
      @ContinuousDelivery  3 года назад

      Yes, learning lots about video, sound and editing on this journey :)

  • @wvvwwwvvw
    @wvvwwwvvw 2 года назад

    I wonder whether I would make the old code base as a branch in git?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +1

      I wouldn't, refactoring is behaviour-preserving, and is best done in very small steps. So you should be safe to release at any point, or not very far from a version that was.

  • @11rchitwood
    @11rchitwood 3 года назад

    Do you only commit when the tests pass?

  • @MrAbrazildo
    @MrAbrazildo 3 года назад +1

    7:30, I use tab = 2.
    9:10, what is xPathString? You just deleted the comment. 10:15, don't delete, just put it at the right side. 11:00, mm... I wouldn't do that. 11:11, omg... I know Java is awkward, but the comment was saying where things were placed. 13:52 and 14:01, somebody put this man in jail! :)
    12:25, I would permanently highlight the 'hasChildren' 1st, or if this doesn't exist in the IDE, I would use the find recurse, to spot every occurrence of it.
    12:45, "equal to" and "has it" are 2 different things to me. That's why I would not delete the comment.
    - Here's the proof of a DRY violation: there's the hasChildren f(), but the programmer forgot it, because the previous f() was too big.
    - 13:17: at least in compiled languages, deleting (safer: turning it into a comment, actually) getXPathString() would reveal all calls to this "f()".
    How C++ is elegant compared to this s**! You could write:
    11:20, jsonString += Str1 + Str2 + ... StrN.
    13:00, return jsonString.substr (blabla) + "]". Period!
    13:40, String tagetString = shortXPath.equals("") ? "//toc" : "//".

  • @istovall2624
    @istovall2624 2 года назад

    Ctrl+shift+a
    Del

  • @ContinuousDelivery
    @ContinuousDelivery  3 года назад

    If you'd like to follow along, the code that I started with can be found here: shitcode.net/348

    • @ContinuousDelivery
      @ContinuousDelivery  Год назад

      @@gezenews I don't think these have gone out of date. My production values have improved a bit since I recorded these, but the content is still good I think.

  • @tristanstricker967
    @tristanstricker967 2 года назад

    Code is tiny. Please make bigger

  • @vrjb100
    @vrjb100 2 года назад

    Don't refactor code in this way. When you did keep up your knowledge up-to-date, 99% of so called seniors don't, then you would know the use of a json library does the same but better. Because you don't know the existence of a library doesn't mean you should write one yourself.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +1

      I am afraid that you are missing the point. This exercise has nothing to do with what the code does, this is how you tackle bad code, to reshape it to be better, and testable. This is about the skills of being a software developer, it is equally applicable to the code that people that assemble 3rd party components together, as it is for the people that build custom systems, or build the pieces that you assemble together.

    • @edgeeffect
      @edgeeffect Год назад

      " keep up your knowledge up-to-date, 99% of so called seniors don't"... that made me laugh SO loud.