List

Refactoring Live: Primitive Obsession

Refactoring Live: Primitive Obsession

by James Dabbs

In the video "Refactoring Live: Primitive Obsession" presented at RailsConf 2019, James Dabbs discusses the importance of refactoring code to avoid primitive obsession, which refers to the practice of using primitive data types to represent business objects. The session focuses on real coding practices aimed at simplifying and enhancing code structures in Ruby on Rails applications. Dabbs emphasizes that effective refactoring should not alter the observable behavior of the system but should aim to make the code more understandable and maintainable. Key points discussed include:

  • Definition of Refactoring: Refactoring involves changing the internal structure of software to make it easier to understand and modify without changing its external behavior.
  • Primitive Obsession: This is identified as the tendency to use primitive types like strings to represent complex domain concepts, thereby limiting the expressiveness and usability of the code.
  • Approach to Refactoring:
    • Begin by identifying code smells related to primitive obsession.
    • Use specific refactoring recipes, such as replacing primitives with objects and employing polymorphism to enhance code flexibility.
  • Example Implementation:
    • Dabbs demonstrates this by live coding a Rails application managing student records, where he refactors how grades are represented. Initially, grades were treated as strings, leading to incorrect logic when handling letter grades like C+ or B+.
    • He illustrates the development process, transitioning to a proper Grade class to encapsulate the logic relevant to grading, making comparisons easier and more accurate.
  • Iterative Refactoring: The live coding segment highlights a step-by-step approach to refactoring, ensuring that tests are run frequently to maintain code integrity and prevent regressions throughout the changes.
  • Final Takeaways:
    • The session concludes by encouraging developers to integrate refactoring into their daily practices to prevent technical debt accumulation.
    • Development should harmonize with business requirements—refactoring is essential anytime a new feature requires changes to existing code, and it should be a continuous process rather than an afterthought.

RailsConf 2019 - Refactoring Live: Primitive Obsession by James Dabbs

_______________________________________________________________________________________________

Cloud 66 - Pain Free Rails Deployments
Cloud 66 for Rails acts like your in-house DevOps team to build, deploy and maintain your Rails applications on any cloud or server.

Get $100 Cloud 66 Free Credits with the code: RailsConf-19
($100 Cloud 66 Free Credits, for the new user only, valid till 31st December 2019)

Link to the website: https://cloud66.com/rails?utm_source=-&utm_medium=-&utm_campaign=RailsConf19
Link to sign up: https://app.cloud66.com/users/sign_in?utm_source=-&utm_medium=-&utm_campaign=RailsConf19
_______________________________________________________________________________________________

Let's roll up our sleeves and clean up some smelly code. In this session, we'll dig in to Primitive Obsession - what happens when our domain logic is all wrapped up in primitive data types? And most importantly, how do we untangle it?

#railsconf #confreaks

RailsConf 2019

00:00:21.570 real quick getting started I'd like to thank and could you all help me thank the the sponsors for making this possible and the organizers and staff
00:00:28.169 for actually making this whole thing happen a real quick round of applause
00:00:33.379 thanks for all y'all hey everybody I'm James I'm here today to do some coding
00:00:40.440 with you specifically we're gonna be talking about refactoring I'll say that most of what I know about refactoring
00:00:46.470 came to me by way of this clicker that works good it's going super well okay
00:00:52.559 it came to me by way of these people who
00:00:57.780 wrote this book and these people who wrote this book it is my sincere hope
00:01:04.289 that most of the concepts that we're talking about today are not entirely new to you if you have maybe tried to use
00:01:10.800 some of these if they haven't really permeated your day-to-day work if you haven't found the time to practice or if had trouble putting them into practice
00:01:16.410 in a rails application of scale that this talk is for you how many people
00:01:21.690 here have seen this talk get a whiff of this from rails comp 16 good number nine
00:01:27.179 okay great if you're watching this on video at home or otherwise have control of time feel
00:01:33.060 free to pause me go watch that and come back in like a minute and a half when we
00:01:38.220 do a quick recap for anyone who's stuck in the here-and-now refactoring what we're talking about
00:01:43.380 today here's a definition it's a change made to the internal structure of software to make it easier to understand
00:01:49.500 and cheaper to modify without changing its observable behavior so a couple important things there it is purposeful
00:01:55.950 right we are making a change to make something easier and it does not change a durable behavior right I'm not
00:02:02.250 altering any of the contracts any user experience in the process of doing a refactor that's the idea the big
00:02:08.340 question is okay how do you how do you actually do it and and here's the hardest part of the whole thing if you
00:02:16.379 have code that is in production if your customers are happy if your servers are happy if your air monitoring services
00:02:22.500 are happy do not touch that code there is really nothing you can do to improve
00:02:27.750 it and there's a whole lot you can do to make it worse what you need to do is wait and that's SuperDuper hard but wait
00:02:35.880 until you have a new requirement right once you have a new requirement you can ask this question is your code open for
00:02:43.260 that new requirement and when I say open that means something very specific it doesn't really mean do I see how I can
00:02:49.590 add this and I think that I wanted reduce any bugs it means can I add this new feature by only adding lines by not
00:02:57.210 changing any existing definitions or methods in my code at all right is it really open
00:03:03.830 if it is that's fantastic you're done it's easy make the easy change move on to the next thing all right
00:03:10.350 similarly if it isn't but you see how to make it open that's fantastic you're done the
00:03:15.690 interesting part is what happens when you need to make a change but your code resists that change somehow and in a way
00:03:22.950 that makes it not obvious how to proceed right in that case the refactoring books
00:03:29.160 say what you do is you pick the code smell closest to your new requirement
00:03:34.610 and this doesn't again mean some vague sorry it write a code smells very
00:03:41.610 specific thing it's not just something you don't like about your code we'll talk about one in a second but once you
00:03:48.240 find the code smell you apply a curative recipe for that code smell one of the key things about code smells is that
00:03:53.370 each one comes with a handful of curative recipes of some process that you can apply to fix it so you do this
00:03:59.070 over and over again eventually your code is open you make the change and you're happy right sorry today we're going to
00:04:08.460 be talking about a particular code smell called primitive obsession and primitive obsession is the tendency to use
00:04:15.530 primitive types to represent your business objects there's a real easy thing to do in rails you know very often
00:04:21.600 you'll you'll do something like write a user model that has a phone number that's a string all right that's a very reasonable thing to do in your database
00:04:27.390 a phone number is a string but the temptation is to then think of a phone number as being the same as a string and when you do that you end up writing code
00:04:33.600 that looks like this when what you really need to express is that you know your phone number is local right like
00:04:38.700 concepts in your problem domain so we'll look at some concrete examples the
00:04:44.490 question is when you find that you have this problem what do you do and the nice thing is there a handful of recipes
00:04:49.630 we're gonna look at two in particular we're gonna look at replace primitive with object which has this picture and Fowler's refactoring book it's got some
00:04:56.710 steps there broadly make a class make sure you didn't make any syntax errors use the class make sure the tests pass
00:05:02.910 and a more complicated one we're gonna look at replace conditional with
00:05:08.470 polymorphism which looks like this good luck here's kind of what it does to
00:05:14.770 code if you can see this you start with some complicated conditional with lots of branches and you end up with several
00:05:19.810 different classes with simple interfaces and relatively simpler methods that's
00:05:25.030 kind of the idea and it has some steps right it's create classes for your behavior create a factory function to
00:05:30.789 pick the right class yada-yada-yada the point is not to memorize any of this right there are 20 some-odd code smells
00:05:37.120 and 60 some-odd refactoring recipes familiarize with yourself with them and then go look them up when you need them
00:05:42.430 right we're gonna dig into these to give you a feel for what they look like exactly but really like don't bother
00:05:48.039 trying to commit this to memory we're gonna try and dig into this in a real actual rails application so we are going
00:05:54.909 to be doing some live coding everybody ready here we go okay so i've cooked up
00:06:02.919 this little rails out this is a sorry it's a it's an app for managing college records grades enrollments and stuff a
00:06:09.880 few little domain concepts i'm gonna have to stand by the laptop some domain concepts a section is kind of the core
00:06:17.020 applicator core object where we talk about a section is a particular class right like math 101 running in some term
00:06:23.530 fall 2019 in some room with some instructor students are enrolled in the class and the enrollment is that join
00:06:30.250 record where we record the students grade in that class a couple responsible
00:06:35.289 I guess sections also do have student assistants here one of the core
00:06:41.289 responsibilities of section is to decide whether or not it should admit a particular student and it does some kind
00:06:46.810 of straightforward stuff but checking role and everything but the interesting bit is it verifies that the student has
00:06:53.560 gotten grades and all of the prerequisites for the course and that the grades meet the minimum requirements to admit them in
00:06:59.010 I'm a core part of the system okay if you can't quite see over here on the right what I've got running is guard set
00:07:05.640 up with r-spec so what you'll see as I change files I'm going to save this section file over on the right that's
00:07:11.610 going to run the unit tests for the section model and then assuming they pass it'll run the full spec suite and
00:07:17.580 so I'm going to make a lot of changes but I'm gonna pretty much ignore that unless it turns red so if it turns red
00:07:22.830 will like this front part of the room please yell at me very loudly and make sure we stop and address it but otherwise we're like yeah you got me
00:07:28.530 okay otherwise we're not gonna worry about at all I encourage you to look away okay so one of the other things
00:07:36.510 this app does is we have transcripts a transcript is largely responsible for computing a student's GPA it does that
00:07:42.750 by pulling their enrollment grades looking them up the appropriate scores for them and doing some other stuff it's
00:07:49.200 not terribly interesting or important you can kind of see right here that right now like our system it works fine
00:07:55.740 but it only understands whole letter grades right a B C and D so that's fine
00:08:02.370 until it isn't but let's say we have a new requirement we would like our app to support plus
00:08:07.800 minus grades we'll work a little bit on defining some acceptance criteria but
00:08:13.500 let's say when we do that work we end with with a couple specs this one in the
00:08:18.900 transcript that broadly says if I have a student who's got all B pluses I can compute their GPA appropriately this one
00:08:25.590 fails for not interested reasons so I'm just gonna skip it for now you'll see it running as pending in the specs and then
00:08:31.500 in the section spec this one that's somewhat more interesting so I'm going
00:08:37.380 to focus on this for just a second this spec says if I have a section that requires you to pass a prereq with a C
00:08:44.250 and a student who has passed that prereq with a C+ they should be admitted into
00:08:49.380 the course right reasonable requirement this does fail and nobody yelled at me
00:08:56.940 please keep me honest it does fail but for sort of uninteresting reasons it fails because I have a validation on
00:09:03.920 enrollment that says that that's not a reasonable grade I'm gonna cheat just a smidge and add these just so we get to
00:09:10.500 the the interesting failure this one will fail don't yell this time sorry if I say it's gonna fail don't you
00:09:19.170 know you got it so we would like for this to pass but it doesn't and it's for
00:09:25.620 sort of an interesting reason let's dig into the section implementation just a
00:09:32.190 little bit the issue here is that when I
00:09:37.740 get to the point of checking my requirements my minimum grade is a C
00:09:42.870 like you would expect and the grade the student has a C+ which means they should be admitted but check out that logic on
00:09:48.600 the next line sorry I have Y numbers hidden for width reasons check out the
00:09:54.000 the line right below that the problem here is that when I compare grades I ask is the grade that I have before the
00:10:01.350 grade that I have to have right and that works pretty well because a comes before C and B comes before C but the problem
00:10:09.300 is C+ does not come before see any more than aardwolf comes before a write like
00:10:16.080 these are strings and if you take a look at the enrollment grades that we have
00:10:26.090 right I have written these down in sort of ascending or descending grade order but if I ask Ruby to sort them I get
00:10:33.630 something very different back right the core problem is that I have treated my grades as strings and that was fine to a
00:10:40.680 point but now I'm understanding that the behavior of a string is different from the behavior of a grade right so I have
00:10:45.990 this primitive obsession that I have to untangle before I can move forward with this requirement so that's that's what we're gonna work on doing that said I'm
00:10:53.610 not gonna try to actual actually fix the requirement for a bit yet so I'm gonna comment this spec back out I expected to
00:11:00.180 fail and I'm just gonna work on refactoring towards making it open okay
00:11:05.330 thank you there you go
00:11:10.400 okay so now understanding that I have a primitive accession I turned towards my recipes and I'm going to apply extract
00:11:18.500 object for this value right the recipe says what I want to end up with is I want to end up with an object like a new
00:11:25.130 class that just has a field that wraps the grade the code that I want to see
00:11:30.350 look something like this I'll wrap the grade I'll ask for its name back right super minimal change
00:11:37.390 that's easy enough to implement I need a new grade class I'm gonna initialize it
00:11:42.650 with a name which I store and I want an add a reader for that name that's it
00:11:49.279 alright but with that implemented I can change these lines and it should work
00:11:54.560 exactly as it did before that's it we've done a refactoring that is the entire
00:12:00.050 thing right and that doesn't feel like very much because it's not but the nice thing is I've made a small but
00:12:06.410 measurable step and I've accumulated basically no-risk along the way right I'm gonna take several hundred more of
00:12:11.779 these and by the end I hope to have something really happy with but that I've never was scared about the entire
00:12:16.790 time right so this isn't much of a win but it does give me a place to start
00:12:22.370 looking towards hanging some behavior off an object that I own but isn't a string right so I'm gonna carry on and
00:12:29.930 extract that comparison out to an object on the grade class again the code that I want to end up with like this lesson and
00:12:35.540 is hugely confusing and hurts my brain to think about I want to end up with some code that looks like this right as
00:12:42.950 my grade at least this minimum grade and that's super easy to add right I own the
00:12:48.680 grade class so if I have some other grade I'm gonna resist the temptation to
00:12:55.670 fix the problem and instead I'm just gonna move the logic into place right so
00:13:02.120 once that's in place I can translate that line and the spec should still pass again I'm gonna hide these out of the
00:13:08.120 way they're not interesting most of the time I'm really focused on the code change here and that starts to feel like
00:13:15.290 a win right I am now comparing grades as great objects and I could go through my
00:13:20.900 system and every where I make a grade comparison I could replace it not going to do that it's a small enough system that's not terribly hard to do I
00:13:26.829 just kind of have one other call site so I can make a similar translation but
00:13:39.610 what does that really bought me right I could I could do this sort of thing throughout the app but the problem is
00:13:46.059 I'm still treating a grade as strings everywhere throughout the core of the app and just at the boundary is where I
00:13:51.189 need to compare them I'm converting them up to objects what I really like to do is make sure that my system always talked about the objects in my system
00:13:57.759 right like I want all of these like every time there's a variable named grade I want for it to be grade and then
00:14:03.220 if I update lots of grades I'm confident my system works right so I want to start moving in that direction
00:14:09.939 let's take this as a particular example I'd like to end up in a world where user
00:14:15.519 dot grades returns great objects I'm not comfortable making that change
00:14:20.799 though because I don't know what all calls user dot grades yet so first I want to insulate myself from that change
00:14:26.319 a little bit and I'm going to do that by adding this
00:14:36.140 so currently grades is a hash I'm gonna convert all the values to
00:14:42.170 grades now this will fail okay my tell me why so the the tricky bit here is I
00:14:50.630 have mason converted to grades twice right initially I transform the values then when I pull the values out of a
00:14:56.390 hash I pass them through a grade again so this is not a safe change I need to revert it I'm gonna back it back out no
00:15:01.790 but I'm gonna head that way what I need to be able to do this is I need a method of taking a value that might be a string
00:15:08.329 and might be a grade and ending up with a grade there's a couple ways to do this but one that I particularly like is to
00:15:14.060 add a factory so when given a value I
00:15:21.740 gonna turn the value if the value is a grade and otherwise I'll assume it's a
00:15:29.750 string and make a new grade from that value right so this works exactly like
00:15:34.790 new except when handed a grade I just get the same object back with that in
00:15:44.209 place I can update my calls to new with four confident that if something upstream changes to pass a grade down
00:15:50.870 instead everything should continue to work as it did before and then I'll make that
00:15:56.029 change upstream in fact I'm going to try to use for consistently instead of new
00:16:02.050 so that later when user changes upstream from that I'm also insulated for that change right this is just kind of adding
00:16:08.390 a shim layer so that you can be permissive of what you accept in in up
00:16:14.329 jim code once this is in place and I know that the grades received from here
00:16:19.670 are great objects I no longer need these updates and I can start simplifying this
00:16:25.899 this block a lot I think all of these
00:16:31.399 changes are safe
00:16:36.870 I'm gonna do the same thing in the schedule the exact same concepts right instead of mooing up a new great object
00:16:42.690 every time I just want to convert then I want to take the upstream grades in this
00:16:49.260 case that come from a method on self and transform them and finally I don't think
00:17:03.840 I need that indirection anymore I can move there now an interesting
00:17:14.760 thing has happened at this point if I take a look I've sort of made a couple
00:17:20.070 of the call sites to grades safe let's take a look if you notice there are only three call sites to grades in my
00:17:26.490 application to that I have immediately cast two objects after calling them and one inspects so at this point I'm pretty
00:17:34.980 confident that my app wouldn't change if I change the behavior of that method but I should think about what the actual
00:17:40.020 specification says right like the specs say that I get string like objects back it's totally reasonable here to change
00:17:47.010 the specs to reflect the change that I want in the code but I think it's also reasonable say like my grades are string
00:17:53.730 like objects and these specs has written should still pass I can do that by defining an equals method and saying if
00:18:03.210 the thing that I'm comparing to is a grade then I check my name against the
00:18:08.730 other grades name and otherwise just do kind of the default thing so I think
00:18:16.440 with that change the specs should pass and all of my call sites are safe whether or not I returned grades or
00:18:21.900 strings and I think at that point I am comfortable making the change to the user grades method to only return grade
00:18:29.250 objects and all the specs should still pass once that's done I no longer need
00:18:37.020 these shim layers and now we really feel like I've gotten somewhere if you look
00:18:42.840 at our implementation of section as far as section understands the world grades are great objects
00:18:49.250 right it's not ubiquitous throughout the app but I have layer upon layer pulled it back and kind of pushed the strings
00:18:55.250 out towards the boundaries thinking back
00:19:00.500 towards a requirement right my grades need to understand strings if you think a little bit about the application right
00:19:07.220 now we're still our grazing underhand scores we're still treating a grade as a
00:19:12.320 string in the place where we compute scores in the transcript so we'll want to do a similar treatment here right but
00:19:18.770 I think we've seen the pattern in managing the tools what I want is I want to be able to write not take the string
00:19:25.880 grade look it up in a hash I want to be able to get a grade and ask it for its score now it won't quite be able to do
00:19:33.080 that right enrollment is an active record object grade as a string column but I can at least kind of convert that
00:19:40.340 here for now and it feels like I should be able to write that so let's take that as a goal right to do this
00:19:46.580 great objects will need to understand their score right now knowledge of
00:19:51.710 scores is wrapped up in the transcript which is obviously in retrospect the wrong place for it like this this prefix is a clue the grade scores kind of
00:19:59.060 belong on grades gonna change name to scores but with those here it's easy
00:20:05.990 enough to write the score method the score of a grade is well go look it up
00:20:11.410 now I don't love that it's possible to create a grade that when you ask it for a score it'll blow up and I want to fix
00:20:17.480 that but it's not required yet so I'm not gonna touch it I'm just trying to get this line sorry in the transcript
00:20:26.260 this line safe and I think at this point grades have a score method so this should be safe to change I will do that
00:20:33.850 inspection still pass they failed did
00:20:39.110 you all see it while I was doing it by any chance grades score yeah I called it scores I
00:20:46.910 looked up in score all right that aside
00:20:58.840 okay so that line is now safe as well and I'm not referencing grade scores
00:21:04.580 anywhere so I can delete that as well so this is really good progress but I think
00:21:10.519 where we're at kind of the hardest point and and also one of the key points here if we take a look across the application
00:21:16.600 at the places that we are converting a string to a grade it's happening in two places here in the transcript we have an
00:21:23.690 enrollment break an active record object that's giving us a string and similarly in grades I pull the users history and
00:21:31.369 it's still an enrollment record that's still treating grade as a string if you look at that active record object we
00:21:39.320 noticed that enrollment has its own separate notion of grades in fact comparing enrollment and grades together
00:21:45.700 there's a latent bug here right enrollments can record that a grade was an F but my transcript has no idea how
00:21:52.279 to handle s whatsoever right the fact this logic was written in multiple places has invited a bug I'm gonna call
00:21:57.769 that a bug and just add this although not prompted by a specific requirement
00:22:04.149 but in any case like we need to unify enrollment with the rest of the system right this one's harder though because
00:22:10.850 we don't have a grades method to shim it's provided to us by active record all right so we're gonna have to do
00:22:16.220 something a little bit fancier here but but similarly what I want to what I want to write is not like I don't want
00:22:21.799 enrollment to have its own notion of grades I want to write something that says well the grade for this record is
00:22:27.679 one of the grades right for me to treat that field as an object in my domain I'm
00:22:34.249 gonna have to do something special but active record supports this I just have to tell it to serialize the grade field
00:22:40.700 as a great object right so this is a goal comment that out for now there's a
00:22:47.059 couple pieces that we're gonna have to implement to make this work one will need a grade all method but that is easy
00:22:53.059 enough to add right I've got my list of scores that I know about let's just map
00:22:58.700 over it these are immutable so I would memorize
00:23:05.400 that although it's not strictly necessary secondly if you're not
00:23:12.240 familiar with serialized the the contract that active record expects of you is that the class that you provide to serialize should respond to load
00:23:19.080 which will get a string and need to produce not i've string or nil and dump which is similar but will need to
00:23:24.780 produce the value to serialize to the database so we just need to define those
00:23:29.930 load of value if we don't if the value is nil we return otherwise convert it to
00:23:36.690 a grade and dump is exactly the same
00:23:41.820 except we need the the serialized value right though in this case the name of the grade so with those in place on
00:23:51.300 commenting these two lines should work but we have to think a little bit there's one change that's latent here well it's not clear we are changing the
00:23:58.170 contract of the grade method on this object so we have to think about are there other places that are calling grade we saw that we shim them several
00:24:05.760 of them but I haven't necessarily audited the whole app to make sure that we have this is a point where it's
00:24:12.000 really nice to have a good test suite that we can lean on so I might run this and just see okay well what still blows
00:24:18.240 up there otherwise you can have to go line by line and find every caller in
00:24:25.470 this case we should have just a couple so these are coming from the grades
00:24:31.950 controller which used to return a string and now is returning name corresponding to a string probably not terribly
00:24:39.060 surprising right in my grades controller I used to return an a grade which was a
00:24:44.640 string and now I'm getting back kind of a JSON defied grade object we could
00:24:50.430 easily add dot name here and that's totally fair but I think the better observation is to say really I don't
00:24:57.090 anywhere that I'm serializing grade in my API I don't want to change anything I just want to produce Jason values
00:25:03.600 they're just the name similarly was with value objects that represent primitive types it's often reasonable to define a conversion method
00:25:10.860 back to the base type I think with that change all of our tests should pass
00:25:17.929 and they do okay so the nice thing here is at this point now everything in my
00:25:25.320 system that talks about grades uses these objects so if I want to make improvements to how grades work I have one pile to look at and I can start to
00:25:36.090 fix it so one change that I would like
00:25:43.500 to make although it's not strictly required it seems on to me to pass in the name and ignore the score like I
00:25:49.350 really want to think of grades as having a score that I pass in that is a change
00:25:58.620 to the API for new so I need to make that a default argument if I want to break stuff and one of the ways that I
00:26:05.399 have found super helpful to give myself confidence that I am free to make changes to that method is especially if
00:26:11.190 I have a factory method is to make the new method protected right my
00:26:16.350 expectation is that anytime any wants grade they're calling for I can make that explicit by making new a protected
00:26:22.230 method on the class and then when I do that I feel much freer to change these implementations here it's easy enough to
00:26:31.679 create a grade with the score I made a rating over the list of scores here something slightly different needs to
00:26:37.260 happen right if I'm if I'm asking you for a great object that represents be something has to know that Abby is a 3.0
00:26:43.850 but really I think I think that all should be the source of truth about what grades exist in my system and I think
00:26:50.159 four should delegate to all so I would tend to want to write all find the particular grade that you want or maybe
00:26:59.760 raise a particular hopefully helpful error message at that point grades have
00:27:09.570 a score and so I can I can think about
00:27:15.659 comparing them pretty easily right Ruby gives us good tools for this if I want to compare two things I include comparable I def a spaceship operator
00:27:24.230 and now since grade objects have scores I can easily express that the way you compare to
00:27:30.480 another thing is you compare the scores of the two things as grades in this case
00:27:37.140 I'll have to rescue from an argument error if I'm given something that isn't grade like right but with that in place
00:27:48.419 so if you're not familiar comfortable what that does is that gives you greater than greater than or equal to equal to for free
00:27:53.880 I don't need my equal two method anymore and finally and most interestingly my at
00:28:00.419 least method becomes a lot easier to write right instead of comparing names which I know won't work
00:28:05.490 with C pluses for instance I can write this and well that looks very similar it is a conceptually very different thing
00:28:11.910 instead of saying my name as a string is before some other string I'm saying me
00:28:17.220 as a grade I'm bigger than this other grade alright so what are we changed
00:28:27.660 here I contend to you nothing I contend
00:28:32.910 that nothing about the way the system behaves has been updated at all but now if you want to look at our transcript
00:28:39.179 spec you comment that back in and our section spec and comment this back in
00:28:44.990 for those to pass we need a B+ and a c-plus so they are failing but I can add
00:28:53.070 a B+ and a c-plus and all the specs
00:29:02.730 should pass all right and so this is what I mean we refactored until it was open for the
00:29:08.669 requirement and actually implementing the requirement took two lines right if you've heard makes change these didn't
00:29:14.460 make the easy change this I believe this is what that means how we feel it we did
00:29:22.530 for one more cool enthusiasm thank you
00:29:27.690 let's take a look at that grades controller right now when you're checking if you are allowed to change
00:29:33.250 grades we use the same method in both places it's this can of great update grades method which checks your role in
00:29:40.870 a couple ways Verne admin you can if your teacher you can if you're the instructor for section and if your student you can't update grade student
00:29:46.450 is the implicit third rule I'd like to introduce a new requirement I'll point out the the specs for this are pretty
00:29:52.390 good and I'm confident in them so I'll just add this new spec the new spec basically says if you're a student
00:29:58.780 assistant you should be able to record new grades you can't record your own grade though and you can't update other
00:30:05.350 grades right so these fail right I'll be
00:30:11.080 done when these are passing to do this
00:30:17.670 it's easy enough to copy and paste and create a can create grades method and then start changing conditionals right
00:30:23.080 but my requirement is to add a new role to my system that has new behaviors and if it were open I'd be able to do that
00:30:29.320 by adding a new role to my system that has new behaviors right the problem is I've tried to represent roles as strings
00:30:34.810 and so every time I need to make a decision about how they behave I have to write it right there because I don't have an object to hang that behavior off
00:30:40.990 of that's another primitive obsession the way I'm going to fix this is by replacing these calls with polymorphism
00:30:47.710 I want to end up with something that looks like grab the role for that user and ask it if they can update right so
00:31:00.130 over in the section I'll start to add that again the there's a recipe here it says define a base class for these roles
00:31:15.210 define a factory method that returns the
00:31:21.460 right kind of thing
00:31:27.230 copy the method over that you want to extract update it appropriately for its
00:31:37.259 new surroundings in this case I want to ask can you update and I don't have current user I have user and then wire
00:31:47.789 that class in right so I want to write this on my section object I'll define a
00:31:52.830 role for a user which returns a role
00:32:08.740 that's a big enough changes I tend to like to run at once just wiring it in without changing return value to make sure I didn't introduce any errors and
00:32:14.590 then running the whole thing but with
00:32:19.929 that done assuming the specs all pass I think we can focus on the roll itself the idea is I want to move this
00:32:25.870 conditional up and give myself different kinds of roles so I'll start by extracting an admin role and the way and
00:32:35.020 admin role works is admins can update right that's that's the business
00:32:40.630 requirement to make that work I need to make sure that the factory returns an
00:32:46.360 admin role when appropriate but doesn't
00:32:52.419 behave any differently in other cases once that's true I never hit that branch
00:33:00.820 so I can just throw an error there just to make sure for myself I'm gonna
00:33:07.570 extract this new section user bit just so I don't have to keep typing it right
00:33:13.780 I'm trying to pick the right type of object but the object should all behave the same so that seems fair to me
00:33:19.179 and I can continue extracting by just defining other roles of other types
00:33:24.450 right a teacher role behaves this way it's just this logic you get a teacher
00:33:37.299 role under these cases
00:33:45.660 and with that in place I shouldn't hit this branch ever and
00:33:53.290 finally well like while the last role does work I think in my domain what's really going on there is there is a
00:33:58.420 student role which cannot ever update
00:34:14.500 and so with that in place I wouldn't implement an update method on the base class at all or would have it raise some descriptive error saying sub clashes and
00:34:20.470 implement this something like that but that's that's my extracted hierarchy
00:34:25.659 right now let's pop back over the grades controller and inline these methods
00:34:32.280 right this is how can update grades currently behaves I want to create a new
00:34:40.500 create check the problem is for creating grades as an assistant I'll need to make
00:34:47.710 sure that are not ratings for myself so I'll have to pass that in right this is this is the required change but with
00:34:53.049 this hierarchy it's easy enough to add if I want a create method everywhere I
00:34:59.010 can add it to the base class by default the implementation is to check can I
00:35:04.599 update all grades carte-blanche right and with that in place and without the
00:35:12.579 syntax error that I saved we should be able to make this change right now the behavior is changed we've just kind of
00:35:17.680 declared these like pull them out separately and again I have not changed
00:35:22.990 a bit about how the system behaves at this point but now if you're handed this and the new requirements they are straightforward like almost trivially
00:35:29.740 easy to implement right like there's nothing to do but the right thing which is if you want an assistant role well
00:35:35.500 make one define how it works right if
00:35:44.650 you're an assistant you can add a grade as long as you aren't the user you're adding a grade for and specify the
00:35:53.230 conditions under which that is your role in this case it's if the section assistants include the user
00:36:06.520 with that change I believe the specs should pass and again our implementation
00:36:15.490 of the feature was five lines of addition well no math right ten lines of
00:36:24.070 added code didn't have to touch anything else right we made it open and then we made the change cool that's it we did it
00:36:31.960 we got through it how we feeling good unnecessary thank
00:36:37.330 you so some takeaways from working this
00:36:43.420 way I think moving towards developing in this sort of mode makes a couple things
00:36:49.690 easy that used to be really hard you can get started without knowing what the code has to look like when you're done it's it is literally a follow your nose
00:36:55.810 sort of thing you could like as long as our tests were green we could have stopped and shipped production at any
00:37:01.930 point in time right are you going on vacation are you picking up some new tasks are you going to lunch doesn't matter it's good to go right it may not
00:37:08.530 have finished it may not have gotten where you wanted to but realistically like this is a software we're never done
00:37:13.990 being comfortable with intermediate steps as being finished products as well is an important thing there are some
00:37:20.770 hard things that are still hard one of the key bits here was I got quick feedback I'll actually - I'll talk the
00:37:26.980 reason in some depth I got quick feedback that may be hard for you I would highly recommend setting up guard
00:37:33.280 in spring you want to run a small set of tests on small objects that don't have to talk to a database refactoring will
00:37:38.680 help you get there some other things that are hard working clickers trusting
00:37:45.520 your coverage right there were some points where it was really helpful to say like oh if something broke I'll see it right no silver bullets here right
00:37:52.450 more tests but also consider some in production testing especially I found
00:37:57.940 some science experiments to be valuable where you can push up a tentative change
00:38:03.100 to production and get some notifications if it actually would have changed anything so you can be kind of confident that you didn't break behavior if you're
00:38:11.290 refactoring a lot you'll want to push hundreds of changes to production that don't change anything and if you are
00:38:16.630 asking a human who verifies your work to manually regression test every time both gonna be miserable don't do that to
00:38:21.700 people if you have q a's work together to develop a set of regression tests that you're both confident and also give
00:38:28.450 them some insight into your development process right like refactoring is a process for de-risking you know the
00:38:34.270 development process but a lot of times if you lob code over the wall people won't see that isolate yourself from
00:38:41.560 rael semantic where you can but don't be afraid to use it when necessary alright like active record is the boundary between the
00:38:46.870 database and your world that you are creating teach it about your world so that you don't have to worry about
00:38:52.330 database things weirdly none of this stuff gets harder
00:38:57.430 in larger apps except finding all of the places that you call a method that's the
00:39:02.650 one thing right so if you want to make that easier on yourself use big long descriptive overly verbose names don't
00:39:09.100 get fancy with dynamic sending and consider maybe like making a new method switching the callers or in like
00:39:14.230 deprecating the old one instead of just deleting or rewriting immediately in place but if I could leave you with like
00:39:20.860 one big takeaway from this how many people have expressed a sentiment like this right like we have some debt that
00:39:26.500 we'd love to pay off but we have to ship some features and we're too busy right like I know I have totally said that plenty of times more and more I have
00:39:33.640 come to realize that I think that is exactly the wrong perspective on the one hand if you do not need to change your
00:39:39.550 product there is no reason that you can change your code right and I mean like insufficient performance is a reasonably
00:39:45.130 that is a new requirement but on the other hand if you need to change your code why would you ever do that in a way
00:39:51.100 that doesn't make it easier to change right it's not the refactoring is the thing that you could to do after your product is done
00:39:56.260 refactoring should be the thing that helps your product be easy to make right so try folding it into your day to day
00:40:01.600 process other takeaways like read these books they're great practice practice practice if your company can bring in
00:40:08.800 Sandy Mets for a workshop like do that it's amazing if you want to work at a company that does that we're hiring I
00:40:14.890 took that picture at work the other day there were dolphins we also were hiring
00:40:20.290 an Austin if that's your thing go go forth and reach Hector
00:40:32.500 and that's my time but I'll be hanging out and happy to chat afterwards