She is the most convincing advocate for object oriented programming I have encountered and I learned a ton from her videos and book. I have really doubted OO programming but she shows that it can start a long relationship with your code that you can maintain and flourish and love. Most of the doubts I have had have boiled down to my experience not finishing and following through with refactors, and opting for faux-o, which as she argues is worse than procedural. She also argues that well written functional code and well written object oriented code are similar, which I find fascinating if true.
I’ve watched this numerous times over the past few years and I love it each time even more then before. Sandi is a great teacher and really explains the concepts in a way that makes sense.
I actually came to this talk after watch Brian Will's video on how OOP is embarrassing, and I felt like in order to get the full value of that video I needed to watch this. To be honest, Sandi is great fun to watch and she killed it.
Sandi, that was an awesome talk. I kept thinking about what you'd do next and every time you did something smarter than I would have thought. Like everytime, genius!
Stumbled upon this from a reddit post. Super-awesome that I have learnt the OOPS objective refactoring. This helped me to move the complex logics, calculations and assignments from the case when block with objects.
Really liked the way the examples are laid out in this talk. One of the most compelling arguments I've seen in favour of delaying abstraction in favour of duplication.
Such an amazing presentation with many important concepts effectively communicated, but... She does actually introduce multiple bugs into the code because she trusted poor quality existing tests. It's possible for quality to go infinitely negative for normal items and for it to get to 51 for Aged Brie. So take that as a lesson in placing too much trust in existing unit tests for spaghetti code and on rewriting a block of code instead of refactoring it.
She said she purposely did not Google what it was, and treated it like a prod issue where all she had was the code and the tests. Tests are the executable specification. If the tests are wrong, that's an entirely different matter.
Nice oop of course, however still there is this condition in the requirements: `Feel free to make any changes to the UpdateQuality method and add any new code as long as everything still works correctly. However, do not alter the Item class or Items property ..`
Sandi Metz rocks. I'm reading POODR (2nd ed) and trying to convince everyone in my organization to read it too (~50 devs working in Cobol-oriented java)
Looks like there's a bug that doesn't get caught by the tests after the first refactoring of normal_tick. If @days_remaining is 1 @quality should only be decremented by 1, but it gets decremented by 2. There's another bug in the first implementation of brie_tick that looks like it should have been caught by the test_brie_on_sell_date_near_max_qality but the test run that is shown doesn't show any failures. With the implementation shown, if @quality is 49 and @days_remaining is
Yes, that's what I was thinking. Sure it's nice to be able to refactor some messy code. But you can't refactor messy requirements. She should at least have added a few tests for those corner cases. If you refactor production code with the attitude "well the tests pass, I don't care for anything else", you're gonna have a bad time. Users will sometimes rely on "buggy" behavior. Often while refactoring code you'll find some bugs, but this way you'll create new bugs. If you look at the actual specs (github.com/NotMyself/GildedRose) there's a lot of corner cases that she missed or intentionally left out to get pretty code.
yes, the `backstage_tick` method has the same issue too i think - if `@quality` is 49 and `@days_remaining` is, say, 3, then `@quality` will get incremented to 52. the instructions for the kata say that quality should never increase beyond 50. still a great video tho
There is a problem. The point of the talk is to demonstrate how to reduce complexity with small and interchangeable objects, but she demonstrates something else. She successfully reduces the complexity for the following reason: she throws away the code, re-writes it from the tests and has fewer branches. That does not show how the small and interchangeable objects make it less complex. When she adds the factory class and the abstract class, the code becomes slightly more complex by introducing two new concepts and indirections. I'm not saying small and interchangeable objects are bad, I'm saying this talk did not really showed me why it's good. Nice presentation anyhow.
Depends on how you look at it. The original if-statement is a horror for anyone to understand. Like she mentioned, better-than-average programmers get stuck on the problem quite easily. In the new design, the only hard things are the relationships between the objects, and the role that each object plays. By explicitely telling that GildedRose is a factory class, most OO programmers already know what it does. (though it is hard if you're not used to OO). Then the only thing left is to find out the relationships between the objects, which is easy enough because there are very few methods, which consist of 4-5 lines at the max. Understanding the new design is not easy per se, but it is definitely easier for most programmers. The ones that are more experienced in OO have an even easier time because they know what a factory is and as such, know that the different Item subclasses are just there for the Open/Closed principle. Compare that with trying to understand the original if-statement and hopefully you can see that the "time to understanding" has been lowered from possibly hours to 2 or 3 minutes at most. She is not saying it is easy. The problem is hard and ugly, which that means the code will be hard. But she is saying it does not have to be ugly. That is what she's trying to show.
Wow what a coincidence! I was just reading the first chapters of her book and was thinking "Oh wow this really resembles what I'm reading about...". A happy accident! :)
The original Gilded Rose kata prohibits changing the Items property and the Item class. I'm not convinced that Sandi avoided changing those two things... She creates a new class and called it Item, and the Items property does not appear in her final code.
I believe there's a bug: take a normal item with quality=1 and a negative sell-by value. The quality is not zero, so it will be decremented. _Twice._ This makes the quality field negative, in violation of the requirements. The way to easily catch this: keep the old code around while refactoring. Have a test which runs both the old code and the new code for every interesting value-that is, with name equal to each string in the old code, with sell-by ranging from -2 to 12 or so (making sure to dodge off-by-one errors) and with quality ranging from -2 to 52 (dodging off-by-one errors around the 0-to-50 quality interval). The assertion is that the old code and new code produce the same answer.
Nice talk, but the first refactoring was erroneous. Obviously, a test-case with remaining days '1' was missing (as her refactored code would have reduced the quality by 2 in this case).
I'm under the impression it all went to shit when she went from small methods to small objects. I fail to see how she didn't contradict her own principle of the wrong abstraction is way more expensive than some duplication. I wonder if she'd still defend this today.
An intermediary class (one that both inherits from a superclass and has subclasses) is a subclass that could be functionally used in a program. Having such a class structure is dangerous since it has a tendency to break things in subclasses when code is changed in this intermediary class. This is why it's recommended to have an inheritance structure where the only classes that are functionally used in the program are the final - leaf - nodes. A tool you can use to enforce this behavior is to use abstract base classes that are only used to inherit from. To further enforce this you can also seal your final nodes to prevent subclassing ("sealed" keyword in C#, "final" in C++/Java/probably more).
A list of bugs in her refactored code: 1) items without a special case are treated as Sulfuras, not Normal. Easy fix. 2) normal items can be reduced below 0 quality if odd after expiry date. Once reduced below 0, will continue deeper into the negatives 3) aged brie can increase above 50 quality if even after expiry date. Once increased above 50, will continue higher 4) backstage pass can increase above 50 when days remaining is less than 10. This corrects after the concert occurs 5) Conjured items can be reduced below 0 if it begins with an odd number quality. Once reduced below 0, will continue deeper into the negatives 6) the goblin in the corner may get mad (he has the item class and she has basically remade her function into an item repackager). 1 is easy to fix, the no-match case should return a Normal object 2-5 is the same basic error just duplicated. It just needs a set to 0/50 end to the functions. 6 is a diplomacy check against your fellow programmer.
the point is to learn how to write tests around it, that's part of the kata. She didn't mention that she wrote those tests, they didn't just appear out of the blue.
Love this talk and the solution is pretty solid, but I still feel like it's overly complicated and you could accomplish the same thing using simple immutable functions instead of creating all these classes. Like, what's the point of even storing state in these classes for a simple `tick` method? Just have a look up table that grabs the appropriate function. You could even put in a default function for the Item class. It would look something like `lookup_tick_module(name).tick(quality, days_remaining)`
A 5000 line Active Record object? I think once you get past a couple hundred with AR, you're lost. Put the keyboard down and rethink what you're doing.
Once again, we ignore the possibility that our tests don't test every input and output. And why should they? If they did, we'd replace the tested function's implementation _with our tests_. Do not delete code you don't understand just because x tests pass. Aside from that, thanks for the perspective, Sandi.
she lost me when she said "Inheritance is not Evil"; we have to stay away from inheritance because we can't predict the future. We always make changes, therefore it's better to write smaller interfaces where you know what the requirements are and compose multiple interfaces together to get desired output you want.
I recently did this in C#, there's a few things I disagree with in Metz's approach, having separate classes for brie and concert tickets, how would that scale in a real world example? If you had thousands of types of food and cheeses, your going to add a new class for each one? Also one of the requirements is to not change or mutate the item class, but she has changed the design drastically, which violates the non functional requirements. I still believe that business logic should be in your domain services/presenters, not in the business entities. Putting business logic in your domain models directly gets out of control and ends up leading to longer term bad designs where the entities become god objects, and monolithic themselves. I prefer a clean abstraction that handles one thing (updating quality) maybe another abstraction that handles something like (updating expiration dates)
All fair points. I think one of the difficulties, though, is when you're brought in for tactical coding (like in her case). Ultimately, there's a deeper issue related to the entire architecture of this project, but that can't always be solved in the outset. It can be tempting to throw the baby out with the bathwater, but it's not always practical or possible to do so. After all, we live in the real world. Contracts have limits and budgets, and many times there are for more issues than you can address in one go. Often times, at the end of a tactical coding contract, you're not only providing the code as a result of your refactorings, but giving the client a clear path and plan forward on how to remedy the issues from a long-term perspective; educating the client is imperative. Not all clients will heed this, and you'll very often get a call back a while later to fix something that has gotten out of control again. But, ultimately, it's important to educate in addition to fixing the issue in the short-term.
No it's not. For me it is ironic how OO people are trying to make their code loosely coupled but they are missing the biggest problem which is right in front of them, it's the objects that promote strongly coupling data and operations that you can perform on that data. And there is no way you can escape this, it's the classes that enforce this style of place oriented programming. Not to mention that you lose the notion of time while doing OO.. enough said. I just think OO is a historical mistake.
Valentín because it is inherent -- objects encapsulate state AND behavior. Functional programming languages do not couple the two. You have data and then you have functions (behavior) that transform that data
I come back to this video often because it helps me think of things in new ways
Definitely the best 40 minutes I spent leaning in the last year
Never in my life has it ever been such a pleasure to watch someone work with code. I'm a believer in people teaching you things about coding now.
The more tech presentations I see, the more I think this might be the best one so far. Sandi Metz is my engineering hero.
She is the most convincing advocate for object oriented programming I have encountered and I learned a ton from her videos and book. I have really doubted OO programming but she shows that it can start a long relationship with your code that you can maintain and flourish and love. Most of the doubts I have had have boiled down to my experience not finishing and following through with refactors, and opting for faux-o, which as she argues is worse than procedural. She also argues that well written functional code and well written object oriented code are similar, which I find fascinating if true.
Every talk Sandi has even given is worth listening to - this is no exception.
I’ve watched this numerous times over the past few years and I love it each time even more then before. Sandi is a great teacher and really explains the concepts in a way that makes sense.
I have watched this talk multiple times, and I learn something new every time I do it!
I came here thinking there's no way I'm watching the whole 40 mins yet they went by real quick. Awesome talk. Only minus, she didn't know about WoW!
I keep coming back to this.
I actually came to this talk after watch Brian Will's video on how OOP is embarrassing, and I felt like in order to get the full value of that video I needed to watch this. To be honest, Sandi is great fun to watch and she killed it.
Sandi, that was an awesome talk. I kept thinking about what you'd do next and every time you did something smarter than I would have thought. Like everytime, genius!
Wonderful talk - Perfectly compressed and delivers.
This talk is just phenomenal. Thank you Sandi and Confreaks for your great effort...
Stumbled upon this from a reddit post. Super-awesome that I have learnt the OOPS objective refactoring. This helped me to move the complex logics, calculations and assignments from the case when block with objects.
Really liked the way the examples are laid out in this talk. One of the most compelling arguments I've seen in favour of delaying abstraction in favour of duplication.
One of the better talks I've ever seen.
Think about how much time she spent on preparing all those slide.. this is a superb talk!
This lady and Brian Will discussing programming would be the single most epic battle of the century.
I'm not gonna lie, I have 3 years of OO experience with Ruby, that's the third time I'm watching this video and I'm still learning.
This is so well done. Thank you for sharing these wonderful lessons!
I have just watched the first 3 minutes and that got me motivated to refactor everything
Such an amazing presentation with many important concepts effectively communicated, but...
She does actually introduce multiple bugs into the code because she trusted poor quality existing tests. It's possible for quality to go infinitely negative for normal items and for it to get to 51 for Aged Brie. So take that as a lesson in placing too much trust in existing unit tests for spaghetti code and on rewriting a block of code instead of refactoring it.
She said she purposely did not Google what it was, and treated it like a prod issue where all she had was the code and the tests. Tests are the executable specification. If the tests are wrong, that's an entirely different matter.
Man, I have exact same problem, now I am confident enough to refactor it, Thank you very much Sandi.
So coding thought process awakening.....Sandi is like programming 'Tomb-Raider' killing bugs :)!
Still a masterpiece today
15:01 "duplication is far cheaper than the wrong abstraction"
Even for someone like me who doesn’t do a lot of OOP, this was a fantastic presentation and I got a lot out of it.
Nice oop of course, however still there is this condition in the requirements:
`Feel free to make any changes to the UpdateQuality method and add any new code as long as everything
still works correctly. However, do not alter the Item class or Items property ..`
Sandi Metz rocks. I'm reading POODR (2nd ed) and trying to convince everyone in my organization to read it too (~50 devs working in Cobol-oriented java)
Looks like there's a bug that doesn't get caught by the tests after the first refactoring of normal_tick. If @days_remaining is 1 @quality should only be decremented by 1, but it gets decremented by 2.
There's another bug in the first implementation of brie_tick that looks like it should have been caught by the test_brie_on_sell_date_near_max_qality but the test run that is shown doesn't show any failures. With the implementation shown, if @quality is 49 and @days_remaining is
But then you could also argue that if no test was testing for a particular behaviour, then that behaviour should not be expected of the code.
Yes, that's what I was thinking. Sure it's nice to be able to refactor some messy code. But you can't refactor messy requirements. She should at least have added a few tests for those corner cases. If you refactor production code with the attitude "well the tests pass, I don't care for anything else", you're gonna have a bad time. Users will sometimes rely on "buggy" behavior. Often while refactoring code you'll find some bugs, but this way you'll create new bugs. If you look at the actual specs (github.com/NotMyself/GildedRose) there's a lot of corner cases that she missed or intentionally left out to get pretty code.
yes, the `backstage_tick` method has the same issue too i think - if `@quality` is 49 and `@days_remaining` is, say, 3, then `@quality` will get incremented to 52. the instructions for the kata say that quality should never increase beyond 50. still a great video tho
There is a problem. The point of the talk is to demonstrate how to reduce complexity with small and interchangeable objects, but she demonstrates something else.
She successfully reduces the complexity for the following reason: she throws away the code, re-writes it from the tests and has fewer branches.
That does not show how the small and interchangeable objects make it less complex.
When she adds the factory class and the abstract class, the code becomes slightly more complex by introducing two new concepts and indirections.
I'm not saying small and interchangeable objects are bad, I'm saying this talk did not really showed me why it's good.
Nice presentation anyhow.
Depends on how you look at it. The original if-statement is a horror for anyone to understand. Like she mentioned, better-than-average programmers get stuck on the problem quite easily.
In the new design, the only hard things are the relationships between the objects, and the role that each object plays. By explicitely telling that GildedRose is a factory class, most OO programmers already know what it does. (though it is hard if you're not used to OO). Then the only thing left is to find out the relationships between the objects, which is easy enough because there are very few methods, which consist of 4-5 lines at the max.
Understanding the new design is not easy per se, but it is definitely easier for most programmers. The ones that are more experienced in OO have an even easier time because they know what a factory is and as such, know that the different Item subclasses are just there for the Open/Closed principle.
Compare that with trying to understand the original if-statement and hopefully you can see that the "time to understanding" has been lowered from possibly hours to 2 or 3 minutes at most. She is not saying it is easy. The problem is hard and ugly, which that means the code will be hard. But she is saying it does not have to be ugly. That is what she's trying to show.
Wow what a coincidence! I was just reading the first chapters of her book and was thinking "Oh wow this really resembles what I'm reading about...". A happy accident! :)
Great talk. I really liked it, and I really benefited from it.
The original Gilded Rose kata prohibits changing the Items property and the Item class. I'm not convinced that Sandi avoided changing those two things... She creates a new class and called it Item, and the Items property does not appear in her final code.
Wow, that was an AMAZING talk!
good video, but damn, that 20p60 30i60 at the top left that blocks EVERY SINGLE SLIDE
@12:53 she compares @days_remaining
I noticed this too, it is an error on her part.
I believe there's a bug: take a normal item with quality=1 and a negative sell-by value. The quality is not zero, so it will be decremented. _Twice._ This makes the quality field negative, in violation of the requirements.
The way to easily catch this: keep the old code around while refactoring. Have a test which runs both the old code and the new code for every interesting value-that is, with name equal to each string in the old code, with sell-by ranging from -2 to 12 or so (making sure to dodge off-by-one errors) and with quality ranging from -2 to 52 (dodging off-by-one errors around the 0-to-50 quality interval). The assertion is that the old code and new code produce the same answer.
I feel enlightened after this talk
Amazing.
Thank you.
Holy crap, what a lesson!
she is the best everrr!!!
Nice talk, but the first refactoring was erroneous. Obviously, a test-case with remaining days '1' was missing (as her refactored code would have reduced the quality by 2 in this case).
I love the points in this talk. It's also worth reading Sandi's blog post on the topic.
www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction
I'm under the impression it all went to shit when she went from small methods to small objects. I fail to see how she didn't contradict her own principle of the wrong abstraction is way more expensive than some duplication. I wonder if she'd still defend this today.
how could a subclass NOT be in the leaf nodes? isn't it a given?
An intermediary class (one that both inherits from a superclass and has subclasses) is a subclass that could be functionally used in a program.
Having such a class structure is dangerous since it has a tendency to break things in subclasses when code is changed in this intermediary class.
This is why it's recommended to have an inheritance structure where the only classes that are functionally used in the program are the final - leaf - nodes.
A tool you can use to enforce this behavior is to use abstract base classes that are only used to inherit from. To further enforce this you can also seal your final nodes to prevent subclassing ("sealed" keyword in C#, "final" in C++/Java/probably more).
Amazing talk!
Amazing lecture!!!
Thank you Sandi! This was very helpful! Bill Sourour of devmastery.com sent me here :)
A list of bugs in her refactored code:
1) items without a special case are treated as Sulfuras, not Normal. Easy fix.
2) normal items can be reduced below 0 quality if odd after expiry date. Once reduced below 0, will continue deeper into the negatives
3) aged brie can increase above 50 quality if even after expiry date. Once increased above 50, will continue higher
4) backstage pass can increase above 50 when days remaining is less than 10. This corrects after the concert occurs
5) Conjured items can be reduced below 0 if it begins with an odd number quality. Once reduced below 0, will continue deeper into the negatives
6) the goblin in the corner may get mad (he has the item class and she has basically remade her function into an item repackager).
1 is easy to fix, the no-match case should return a Normal object
2-5 is the same basic error just duplicated. It just needs a set to 0/50 end to the functions.
6 is a diplomacy check against your fellow programmer.
sandi metz u are a goddess of ruby
very beautiful
That was so good!
incredible
Any chance someone has a reference to the github repository used for this gilded rose? I found various, but would like one with a working test suite
the point is to learn how to write tests around it, that's part of the kata. She didn't mention that she wrote those tests, they didn't just appear out of the blue.
does someone knows about some kata i can use to practice this?
+William Ocoró (Demondzeta) github.com/emilybache/GildedRose-Refactoring-Kata pick the language :)
Love this talk and the solution is pretty solid, but I still feel like it's overly complicated and you could accomplish the same thing using simple immutable functions instead of creating all these classes. Like, what's the point of even storing state in these classes for a simple `tick` method? Just have a look up table that grabs the appropriate function. You could even put in a default function for the Item class. It would look something like `lookup_tick_module(name).tick(quality, days_remaining)`
What are immutable functions? I know immutable data, do you mean functions that return immutable data? Not sure how that would be relevant here.
this is super awesome...
Why in 10:14 she use writes 'if name == 'normal' and not 'if @name == normal'?
Good talk.
Great video, like most of Sandi's work.
One nitpick though - at 23:53, shouldn't "item.tick" be "@item.tick"?
No "item.tick" is running a procedure on the object "item". "@x" is used for variable assignment
@@TheUnorthodoxGears which 'item'?
A 5000 line Active Record object? I think once you get past a couple hundred with AR, you're lost. Put the keyboard down and rethink what you're doing.
spitting fax fr fr
brilliant!
Sandi is GOD!
is GildedRose a real class written by some programmer ? wtf ?
Once again, we ignore the possibility that our tests don't test every input and output. And why should they? If they did, we'd replace the tested function's implementation _with our tests_. Do not delete code you don't understand just because x tests pass.
Aside from that, thanks for the perspective, Sandi.
Very true. You should not test that way either
she lost me when she said "Inheritance is not Evil"; we have to stay away from inheritance because we can't predict the future. We always make changes, therefore it's better to write smaller interfaces where you know what the requirements are and compose multiple interfaces together to get desired output you want.
Agreed, her implementation was honestly a lot better around 15 minutes than it was at the end of the talk.
Kent Beck's statement: twitter.com/KentBeck/status/250733358307500032
I recently did this in C#, there's a few things I disagree with in Metz's approach, having separate classes for brie and concert tickets, how would that scale in a real world example? If you had thousands of types of food and cheeses, your going to add a new class for each one?
Also one of the requirements is to not change or mutate the item class, but she has changed the design drastically, which violates the non functional requirements.
I still believe that business logic should be in your domain services/presenters, not in the business entities. Putting business logic in your domain models directly gets out of control and ends up leading to longer term bad designs where the entities become god objects, and monolithic themselves. I prefer a clean abstraction that handles one thing (updating quality) maybe another abstraction that handles something like (updating expiration dates)
All fair points. I think one of the difficulties, though, is when you're brought in for tactical coding (like in her case). Ultimately, there's a deeper issue related to the entire architecture of this project, but that can't always be solved in the outset. It can be tempting to throw the baby out with the bathwater, but it's not always practical or possible to do so. After all, we live in the real world. Contracts have limits and budgets, and many times there are for more issues than you can address in one go.
Often times, at the end of a tactical coding contract, you're not only providing the code as a result of your refactorings, but giving the client a clear path and plan forward on how to remedy the issues from a long-term perspective; educating the client is imperative. Not all clients will heed this, and you'll very often get a call back a while later to fix something that has gotten out of control again. But, ultimately, it's important to educate in addition to fixing the issue in the short-term.
Это нечто В)
Poor camera person. Hard to track the speaker when they are walking around so much.
who else came from Object-Oriented Programming is Embarrassing ?
This talk could also be titled "OO programming" sucks.
No it's not. For me it is ironic how OO people are trying to make their code loosely coupled but they are missing the biggest problem which is right in front of them, it's the objects that promote strongly coupling data and operations that you can perform on that data. And there is no way you can escape this, it's the classes that enforce this style of place oriented programming. Not to mention that you lose the notion of time while doing OO.. enough said. I just think OO is a historical mistake.
Here is a sample of the "functional is the silver bullet" bandwagon that started in the 2010's.
Juraci Vieira
lmao
Valentín because it is inherent -- objects encapsulate state AND behavior. Functional programming languages do not couple the two. You have data and then you have functions (behavior) that transform that data
amazing talk!