Share
Dit is deel 2 van een kleine serie over code ruikt en mogelijke refactorings. De doelgroep die ik in gedachten had, zijn nieuwkomers die over dit onderwerp hebben gehoord en die misschien een beetje wilden wachten voordat ze dit geavanceerde water betraden. Het volgende artikel behandelt "Feature Envy", "Shotgun Surgery" en "Divergent Change".
Wat je snel zult beseffen met code ruikt, is dat sommigen van hen zeer hechte neven zijn. Zelfs hun refactorings zijn soms gerelateerd, bijvoorbeeld, Inline klasse en Klasse extraheren zijn niet zo verschillend.
Door bijvoorbeeld een klasse in te voegen, extraheer je de hele klas terwijl je de originele verwijdert. Zo een extractklas met een kleine draai. Het punt dat ik probeer te maken is dat je je niet overweldigd voelt door het aantal geuren en refactorings, en zeker niet ontmoedigd mag raken door hun slimme namen. Dingen zoals Shotgun Surgery, Feature Envy en Divergent Change klinken misschien chique en intimiderend voor mensen die net zijn begonnen. Misschien heb ik het mis, natuurlijk.
Als je een beetje duikt in dit hele onderwerp en met een paar refactorings speelt om zelf codes te ruiken, zie je snel dat ze vaak in dezelfde marge terechtkomen. Veel refactorings zijn gewoon verschillende strategieën om een punt te bereiken waarop je lessen hebt die beknopt zijn, goed georganiseerd en gericht op een kleine hoeveelheid verantwoordelijkheden. Ik denk dat het eerlijk is om te zeggen dat als je dat kunt bereiken, je het grootste deel van de tijd voor bent - niet dat het voor anderen erg belangrijk is, maar een dergelijk klassenontwerp ontbreekt vaak in code van mensen voordat ze Worden beschouwd als "experts".
Dus waarom zou u niet vroeg in het spel stappen en een concreet fundament bouwen voor het ontwerpen van uw code. Geloof niet wat je eigen verhaal is dat dit een geavanceerd onderwerp is dat je een tijdje moet uitstellen tot je zover bent. Zelfs als je een newbie bent, als je kleine stapjes zet, kun je veel eerder je hoofd rond de geur en hun refactorings wikkelen dan je zou denken.
Voordat we ingaan op de mechanica, wil ik een belangrijk punt uit het eerste artikel herhalen. Niet elke geur is inherent slecht, en niet elke refactoring is altijd de moeite waard. U moet ter plaatse beslissen wanneer u over alle relevante informatie beschikt, of uw code stabieler is na een refactoring en of het de moeite waard is om de geur te repareren.
Laten we een voorbeeld uit het vorige artikel opnieuw bekijken. We hebben een lange lijst met parameters uitgepakt voor #assign_new_mission in een parameter object via de Missie klasse. Tot zover zo cool.
M met functie-afgunst
"ruby class M def assign_new_mission (mission) print" Mission # mission.mission_name is toegewezen aan # mission.agent_name met als doelstelling # mission.objective. "als mission.licence_to_kill print" De licentie om te doden is verleend. "else print" De licentie om te doden is niet toegekend. "end-end-end
class Mission attr_reader: mission_name,: agent_name,: objective,: licence_to_kill
def initialize (mission_name: mission_name, agent_name: agent_name, doelstelling: objective, licence_to_kill: licence_to_kill) @mission_name = mission_name @agent_name = agent_name @objective = objective @licence_to_kill = licence_to_kill end end
m = M.new
mission = Mission.new (missienaam: 'Octopussy', agentnaam: 'James Bond', doelstelling: 'vind het nucleaire apparaat', licence_to_kill: true)
m.assign_new_mission (missie)
"
Ik heb kort gezegd hoe we het kunnen vereenvoudigen M klasse zelfs meer door de methode te verplaatsen #assign_new_mission naar de nieuwe klasse voor het parameterobject. Waar ik het niet over had, was het feit dat M had een gemakkelijk te genezen vorm van eigenschap afgunst ook. M was veel te nieuwsgierig naar attributen van Missie. Anders gezegd, ze stelde veel te veel "vragen" over het zendingsobject. Het is niet alleen een slecht geval van micromanagement, maar ook een heel gewone codegeur.
Ik zal je laten zien wat ik bedoel. In M # assign_new_mission, M is "jaloers" op de gegevens in het nieuwe parameterobject en wil er overal toegang toe hebben.
Daarnaast hebt u ook een parameterobject Missie dat is op dit moment alleen verantwoordelijk voor gegevens, wat een andere geur is, een Dataklasse.
Deze hele situatie vertelt je dat in feite #assign_new_mission wil ergens anders zijn en M hoeft geen details te kennen over hoe missies worden toegewezen. Waarom zou het tenslotte niet de verantwoordelijkheid van een missie zijn om nieuwe missies toe te wijzen? Vergeet niet om altijd dingen samen te voegen die ook samen veranderen.
M zonder afgunst op de functies
"ruby class M def assign_new_mission (missie) mission.assign end end
class Mission attr_reader: mission_name,: agent_name,: objective,: licence_to_kill
def initialize (mission_name: mission_name, agent_name: agent_name, doelstelling: objective, licence_to_kill: licence_to_kill) @mission_name = mission_name @agent_name = agent_name @objective = objective @licence_to_kill = licence_to_kill end
def assign print "Mission # mission_name is toegewezen aan # agent_name met als doelstelling # objective." als licence_to_kill print "De licentie om te doden is verleend." anders print "De licentie om te doden is niet geweest verleend. "end end end
m = M.new mission = Mission.new (missienaam: 'Octopussy', agentnaam: 'James Bond', doelstelling: 'vind het nucleaire apparaat', licence_to_kill: true) m.assign_new_mission (missie) "
Zoals je kunt zien, vereenvoudigden we de dingen nogal. De methode is aanzienlijk afgeslankt en delegeert het gedrag aan het verantwoordelijke object. M vraagt niet meer specifiek om missiegegevens en blijft zeker niet betrokken bij hoe opdrachten worden afgedrukt. Nu kan ze zich concentreren op haar echte werk en hoeft ze niet gestoord te worden als er details over zendingstaken veranderen. Meer tijd voor mindgames en het opsporen van malafide agenten. Win-win!
Feature jaloersheid verstrengelt verstrikking - hiermee bedoel ik niet de goede soort, degene die informatie sneller dan licht spookachtig laat reizen - ik heb het over degene die in de loop van de tijd je ontwikkelingsmomentum zou kunnen laten afnemen tot een naderende stop. Niet goed! Waarom? Rimpeleffecten door uw hele code zullen weerstand oproepen! Een verandering op één plek vlinders door allerlei dingen en je eindigt als een vlieger in een orkaan. (Ok, een beetje overdreven dramatisch, maar ik geef mezelf daar een B + voor de Bond-referentie.)
Als algemeen tegengif voor feature envy, wil je lessen ontwerpen die voornamelijk betrekking hebben op hun eigen dingen en die, indien mogelijk, een enkele verantwoordelijkheid hebben. Kortom, klassen zouden iets als vriendelijke otakus moeten zijn. Sociaal gezien is dat misschien niet het gezondste gedrag, maar voor het ontwerpen van klassen is het vaak een redelijke richtlijn om je momentum te houden waar het zou moeten zijn - vooruitgaan!
De naam is een beetje gek, niet? Maar tegelijkertijd is het een vrij nauwkeurige beschrijving. Klinkt als een serieuze zaak, en dat is het ook! Gelukkig is het niet zo moeilijk te begrijpen, maar het is niettemin een van de gemenere codes. Waarom? Omdat het duplicatie kweekt als geen ander, en het is gemakkelijk om alle veranderingen uit het oog te verliezen die je zou moeten aanbrengen om dingen te repareren. Wat er tijdens een shotgun-operatie gebeurt, is dat je een wijziging in één klasse / bestand aanbrengt en dat je ook veel andere klassen / bestanden aanraakt die moeten worden bijgewerkt. Ik hoop dat het niet klinkt als een goed moment dat je er bent.
Je zou bijvoorbeeld kunnen denken dat je met een kleine verandering op één plek weg kunt komen, en dan beseffen dat je een hele reeks bestanden moet doorlopen om ofwel dezelfde verandering aan te brengen of iets anders te repareren dat daardoor kapot is gegaan. Niet goed, helemaal niet! Dat klinkt meer als een goede reden waarom mensen de code waarmee ze te maken hebben gaan haten.
Als je een spectrum hebt met DRY-code aan de ene kant, dan is code die vaak een shotgun-operatie nodig heeft, vrijwel aan de andere kant. Wees niet lui en laat jezelf dat gebied betreden. Ik weet zeker dat je liever één bestand opent en je wijzigingen daar aanbrengt en er klaar voor bent. Dat is het soort lui waar je naar moet streven!
Om deze geur te voorkomen, hier is een korte lijst met symptomen waar u op kunt letten:
Wat bedoelen we als we het hebben over gekoppelde code? Laten we zeggen dat we objecten hebben EEN en B. Als ze niet gekoppeld zijn, kun je een van hen veranderen zonder de andere te beïnvloeden. Anders zul je vaker wel dan niet ook met het andere object te maken krijgen.
Dit is een probleem, en een shotgun-operatie is ook een symptoom voor een strakke koppeling. Dus let altijd op hoe gemakkelijk u uw code kunt wijzigen. Als het relatief eenvoudig is, betekent dit dat je koppelingsniveau acceptabel laag is. Dat gezegd hebbende, realiseer ik me dat uw verwachtingen onrealistisch zouden zijn als u verwacht dat u de hele tijd tegen elke prijs aan elkaar kunt koppelen. Dat gaat niet gebeuren! U zult goede redenen vinden om te besluiten om die drangachtige vervanging van conditionaliteiten niet te nemen polymorfisme. In een dergelijk geval is een beetje koppeling, shotgun-chirurgie en het synchroon houden van de API van objecten de moeite waard om een heleboel case-statements kwijt te raken via een Null-object (meer hierover in een later stuk).
Meestal kun je een van de volgende refactorings toepassen om de wonden te genezen:
Laten we naar een code kijken. Dit voorbeeld is een deel van hoe een Specter-app betalingen tussen hun aannemers en kwaadwillende klanten afhandelt. Ik heb de betalingen een beetje vereenvoudigd door standaardtarieven te hanteren voor zowel aannemers als klanten. Het maakt dus niet uit of Specter de taak heeft een kat te ontvoeren of een heel land af te persen: de vergoeding blijft hetzelfde. Hetzelfde geldt voor wat zij hun contractanten betalen. In het zeldzame geval gaat een operatie zuidwaarts en een andere Nr. 2 moet de haai letterlijk springen, Spectre biedt een volledige terugbetaling om kwaadwillende klanten gelukkig te houden. Specter maakt gebruik van een eigen betaald betaal-juweel dat in feite een tijdelijke aanduiding is voor elke betalingsprocessor.
In het eerste voorbeeld hieronder zou het vervelend zijn als Specter besloot om een andere bibliotheek te gebruiken om betalingen af te handelen. Er zouden meer bewegende delen bij betrokken zijn, maar voor het demonstreren van shotgun-chirurgie zal deze hoeveelheid complexiteit denk ik:
Voorbeeld met geurtje van een shotgun-operatie:
"ruby class EvilClient # ...
STANDARD_CHARGE = 10000000 BONUS_CHARGE = 10000000
def accept_new_client PaymentGem.create_client (email) einde
def charge_for_initializing_operation evil_client_id = PaymentGem.find_client (email) .payments_id PaymentGem.charge (evil_client_id, STANDARD_CHARGE) einde
def charge_for_successful_operation evil_client_id = PaymentGem.find_client (email) .payments_id PaymentGem.charge (evil_client_id, BONUS_CHARGE) end end
klasse Operatie # ...
REFUND_AMOUNT = 10000000
def refund transaction_id = PaymentGem.find_transaction (payments_id) PaymentGem.refund (transaction_id, REFUND_AMOUNT) end end
klasse aannemer # ...
STANDARD_PAYOUT = 200000 BONUS_PAYOUT = 1000000
def process_payout spectre_agent_id = PaymentGem.find_contractor (email) .payments_id if operation.enemy_agent == 'James Bond' && operation.enemy_agent_status == 'Killed in action' PaymentGem.transfer_funds (spectre_agent_id, BONUS_PAYOUT) else PaymentGem.transfer_funds (spectre_agent_id, STANDARD_PAYOUT) end-end end "
Als je naar deze code kijkt, moet je jezelf afvragen: "Zou het moeten? EvilClients klasse zich echt zorgen maken over hoe de betalingsprocessor nieuwe kwaadwillende klanten accepteert en hoe ze worden belast voor operaties? "Natuurlijk niet! Is het een goed idee om de verschillende bedragen te spreiden om overal te betalen? Moeten de implementatiegegevens van de betalingsverwerker worden weergegeven in een van deze klassen? Zeker niet!
Bekijk het vanaf die manier. Als u iets wilt veranderen aan de manier waarop u met betalingen omgaat, waarom zou u het moeten openen? EvilClient klasse? In andere gevallen kan het een gebruiker of klant zijn. Als je erover nadenkt, heeft het geen zin om hen vertrouwd te maken met dit proces.
In dit voorbeeld zou het gemakkelijk moeten zijn om te zien dat wijzigingen in de manier waarop u betalingen accepteert en overdraagt, rimpeleffecten in uw gehele code creëren. Als u het bedrag dat u aanlevert of overboekt naar uw contractanten wilt wijzigen, moet u bovendien overal extra wijzigingen aanbrengen. Eerste voorbeelden van shotgun-chirurgie. En in dit geval hebben we alleen maar te maken met drie klassen. Stel je je pijn voor als het een beetje meer realistische complexiteit betreft. Ja, dat zijn de dingen waar nachtmerries van gemaakt zijn. Laten we een voorbeeld bekijken dat een beetje gezonder is:
Voorbeeld zonder shotgun chirurgische geur en geëxtraheerde klasse:
"robijnklasse PaymentHandler STANDARD_CHARGE = 10000000 BONUS_CHARGE = 10000000 REFUND_AMOUNT = 10000000 STANDARD_CONTRACTOR_PAYOUT = 200000 BONUS_CONTRACTOR_PAYOUT = 1000000
def initialize (payment_handler = PaymentGem) @payment_handler = payment_handler end
def accept_new_client (evil_client) @ payment_handler.create_client (evil_client.email) einde
def charge_for_initializing_operation (evil_client) evil_client_id = @ payment_handler.find_client (evil_client.email) .payments_id @ payment_handler.charge (evil_client_id, STANDARD_CHARGE) einde
def charge_for_successful_operation (evil_client) evil_client_id = @ payment_handler.find_client (evil_client.email) .payments_id @ payment_handler.charge (evil_client_id, BONUS_CHARGE) einde
def refund (bewerking) transaction_id = @ payment_handler.find_transaction (operation.payments_id) @ payment_handler.refund (transaction_id, REFUND_AMOUNT) einde
def contractor_payout (contractor) spectre_agent_id = @ payment_handler.find_contractor (contractor.email) .payments_id if operation.enemy_agent == 'James Bond' && operation.enemy_agent_status == 'Killed in action' @ payment_handler.transfer_funds (spectre_agent_id, BONUS_CONTRACTOR_PAYOUT) else @ payment_handler.transfer_funds (spectre_agent_id, STANDARD_CONTRACTOR_PAYOUT) end end end
class EvilClient # ...
def accept_new_client PaymentHandler.new.accept_new_client (zelf) einde
def charge_for_initializing_operation PaymentHandler.new.charge_for_initializing_operation (self) end
def charge_for_successful_operation (operation) PaymentHandler.new.charge_for_successful_operation (self) end end
klasse Operatie # ...
Def refund PaymentHandler.new.refund (self) end end
klasse aannemer # ...
def process_payout PaymentHandler.new.contractor_payout (self) end end "
Wat we hier hebben gedaan, is de PaymentGem API in onze eigen klas. Nu hebben we één centrale plaats waar we onze wijzigingen toepassen als we besluiten dat, bijvoorbeeld, een SpectrePaymentGem zou beter voor ons werken. Niet meer aanraken van interne niet-gerelateerde bestanden van meerdere naar betalingen als we ons aan veranderingen moeten aanpassen. In de lessen die met betalingen te maken hebben, hebben we eenvoudigweg het PaymentHandler en delegeer de benodigde functionaliteit. Eenvoudig, stabiel en geen reden om te veranderen.
En we hebben niet alleen alles in één bestand opgeslagen. Binnen de PaymentsHandler klasse, er is maar één plaats die we moeten uitruilen en verwijzen naar een mogelijke nieuwe betalingsprocessor-in initialiseren. Dat is rad in mijn boek. Natuurlijk, als de nieuwe betalingsdienst een compleet andere API heeft, moet je de instanties van een aantal methoden aanpassen PaymentHandler. Het is een kleine prijs om te betalen in vergelijking met een full-on shotgun-operatie - dat is meer een operatie voor een kleine splinter in je vinger. Goede deal!
Als je niet oppast wanneer je tests schrijft voor een dergelijke betalingsverwerker - of voor een externe service waarop je kunt vertrouwen - loop je mogelijk ernstige kopzorgen wanneer ze hun API wijzigen. Ze "lijden ook aan verandering", natuurlijk. En de vraag is niet of ze hun API zullen veranderen, alleen wanneer.
Door onze inkapseling bevinden we ons in een veel betere positie om onze methoden voor de betalingsprocessor te ontcijferen. Waarom? Omdat de methoden die we stompen de onze zijn en ze alleen veranderen wanneer we dat willen. Dat is een grote overwinning. Als u nog nooit bent getest en dit is niet helemaal duidelijk voor u, maakt u zich daar geen zorgen over. Neem je tijd; dit onderwerp kan in het begin lastig zijn. Omdat het zo'n voordeel is, wilde ik het alleen maar voor de volledigheid noemen.
Zoals u kunt zien, vereenvoudigde ik de verwerking van betalingen behoorlijk in dit dwaze voorbeeld. Ik had het eindresultaat ook wat meer kunnen opruimen, maar het punt was om de geur duidelijk te demonstreren en hoe je er door abstractie van af kunt komen..
Als je niet helemaal tevreden bent met deze klas en mogelijkheden ziet voor refactoring, dan groet ik je - en ik wil graag de eer opeisen. Ik raad je aan jezelf knock-out te gaan! Een goed begin kan te maken hebben met de manier waarop u vindt payments_ids. De klas zelf werd ook al een beetje druk ...
Uiteenlopende verandering is in zekere zin het tegenovergestelde van shotgun-chirurgie, waar je één ding wilt veranderen en die verandering door een aantal verschillende bestanden moet schieten. Hier wordt een enkele klasse vaak om verschillende redenen en op verschillende manieren gewijzigd. Mijn aanbeveling is om delen die samen veranderen te identificeren en uit te pakken in een aparte klasse die zich kan concentreren op die ene verantwoordelijkheid. Deze klassen kunnen beurtelings ook niet meer dan één reden hebben om te veranderen - zo niet, dan wacht een andere afwijkende verandergeur je waarschijnlijk te bijten.
Klassen die last hebben van uiteenlopende veranderingen zijn er die veel veranderen. Met tools zoals Churn kunt u meten hoe vaak bepaalde delen van uw code in het verleden moesten worden gewijzigd. Hoe meer punten je in een klas aantreft, hoe groter de kans dat verschillende wijzigingen op het werk optreden. Ik zou ook niet verbaasd zijn als juist deze klassen degenen zijn die de meeste bugs veroorzaken.
Begrijp me niet verkeerd: vaak veranderen is niet direct de geur. Het is echter een nuttig symptoom. Een ander veel voorkomend en meer expliciet symptoom is dat dit object met meer dan één verantwoordelijkheid moet jongleren. De principe van enkele verantwoordelijkheid SRP is een uitstekende richtlijn om te voorkomen dat deze code ruikt en in het algemeen stabielere code te schrijven. Het kan lastig zijn om te volgen, maar toch de moeite waard.
Laten we eens kijken naar dit vervelende voorbeeld hieronder. Ik heb het shotgun-operatie-voorbeeld een beetje aangepast. Blofeld, hoofd van Spectre, staat misschien bekend om micromanage, maar ik betwijfel of hij geïnteresseerd zou zijn in de helft van de dingen waar deze klasse mee te maken heeft.
"ruby class Spectre
STANDARD_CHARGE = 10000000 STANDARD_PAYOUT = 200000
def charge_for_initializing_operation (client) evil_client_id = PaymentGem.find_client (client.email) .payments_id PaymentGem.charge (evil_client_id, STANDARD_CHARGE) einde
def contractor_payout (contractor) spectre_agent_id = PaymentGem.find_contractor (contractor.email) .payments_id PaymentGem.transfer_funds (spectre_agent_id, STANDARD_PAYOUT) einde
def assign_new_operation (operation) operation.contractor = 'Some evil fat' operation.objective = 'Steel een lading waardevolle spullen' operation.deadline = 'Midnight, 18 november' end
def print_operation_assignment (operatie) print "# operation.contractor is toegewezen aan # operation.objective. De missie-deadline eindigt om # operation.deadline. "Einde
def dispose_of_agent (spectre_agent) stelt: "Je stelde deze organisatie teleur. Je weet hoe Spectre met mislukking omgaat. Tot ziens # spectre_agent.code_name! "End-end"
De Spook klas heeft veel te veel verschillende dingen waar het zich zorgen over maakt:
Zeven verschillende verantwoordelijkheden voor één klas. Niet goed! U moet wijzigen hoe agenten worden verwijderd? Een vector voor het wijzigen van de Spook klasse. U wilt betalingen anders afhandelen? Een andere vector. Jij krijgt de drift.
Hoewel dit voorbeeld verre van realistisch is, vertelt het nog steeds hoe gemakkelijk het is om onnodig gedrag te vergaren dat vaak op één plek moet veranderen. We kunnen het beter doen!
"ruby class Spectre # ...
def dispose_of_agent (spectre_agent) stelt: "Je stelde deze organisatie teleur. Je weet hoe Spectre met mislukking omgaat. Tot ziens # spectre_agent.code_name! "End-end
klasse PaymentHandler STANDARD_CHARGE = 10000000 STANDARD_CONTRACTOR_PAYOUT = 200000
# ...
def initialize (payment_handler = PaymentGem) @payment_handler = payment_handler end
def charge_for_initializing_operation (evil_client) evil_client_id = @ payment_handler.find_client (evil_client.email) .payments_id @ payment_handler.charge (evil_client_id, STANDARD_CHARGE) einde
def contractor_payout (contractor) spectre_agent_id = @ payment_handler.find_contractor (contractor.email) .payments_id @ payment_handler.transfer_funds (spectre_agent_id, STANDARD_CONTRACTOR_PAYOUT) end end end
class EvilClient # ...
def charge_for_initializing_operation PaymentHandler.new.charge_for_initializing_operation (self) end end
klasse aannemer # ...
def process_payout PaymentHandler.new.contractor_payout (self) end end
klasse Operatie attr_accessor: contractant,: doelstelling,: deadline
def initialize (attrs = ) @contractor = attrs [: contractor] @objective = attrs [: objective] @deadline = attrs [: deadline] einde
def print_operation_assignment print "# contractor is toegewezen aan # objective. De deadline voor de missie eindigt om # deadline. "End end"
Hier haalden we een aantal klassen uit en gaven ze hun eigen unieke verantwoordelijkheden - en daarom hun eigen ingesloten reden om te veranderen.
U wilt betalingen anders afhandelen? Nu hoeft u de. Niet aan te raken Spook klasse. Moet u anders in rekening brengen of betalen? Nogmaals, het bestand hoeft niet te worden geopend Spook. Opdrachten voor afdrukbewerkingen zijn nu de bedrijfsactiviteiten, waar deze thuishoren. Dat is het. Niet te gecompliceerd, denk ik, maar absoluut een van de meest voorkomende geuren die je vroeg moet leren hanteren.
Ik hoop dat je op het punt bent gekomen dat je klaar bent om deze aanpassingen in je eigen code uit te proberen en dat het eenvoudiger is om code te vinden die om je heen ruikt. Pas op dat we net zijn begonnen, maar dat je al een paar grote hebt aangepakt. Ik wed dat het niet zo lastig was als je ooit had kunnen denken!
Natuurlijk zullen praktijkvoorbeelden een stuk uitdagender zijn, maar als je de mechanica en patronen hebt begrepen om geuren te herkennen, kun je je zeker snel aanpassen aan realistische complexiteiten.
Accentsconagua