Share
Dette er del 2 av en liten serie om kode lukter og mulige refactorings. Målgruppen jeg hadde i tankene er nybegynnere som hørte om dette emnet, og som kanskje ønsket å vente litt før de kom inn i disse avanserte farvannene. Følgende artikkel dekker "Feature Envy", "Shotgun Surgery" og "Divergent Change".
Det du raskt vil forstå med kode lukter er at noen av dem er veldig nær fettere. Selv deres refactorings er noen ganger relatert, for eksempel, Inline Class og Utdrag klasse er ikke så annerledes.
Ved å legge inn en klasse, for eksempel, trekker du ut hele klassen mens du blir kvitt den opprinnelige. Så god utdragsklasse med litt vri. Poenget jeg prøver å gjøre er at du ikke bør føle seg overveldet av antall lukter og refactorings, og absolutt ikke bør bli motløs av deres klare navn. Ting som Shotgun Surgery, Feature Envy og Divergent Change kan høres fint og skremmende ut for folk som nettopp har startet. Kanskje jeg tar feil, selvfølgelig.
Hvis du dykker litt inn i dette hele emnet og spiller med et par refactorings for kode lukter, ser du raskt at de ofte kommer opp i samme ballpark. Mange refactorings er ganske enkelt forskjellige strategier for å komme til et punkt der du har klasser som er konsise, godt organisert og fokusert på en liten mengde ansvarsområder. Jeg synes det er rettferdig å si at hvis du kan oppnå det, vil du være foran pakken mesteparten av tiden - ikke det å være foran andre er så viktig, men en slik klassedesign mangler ganske enkelt ofte i kode fra folk før de er betraktet som "eksperter".
Så hvorfor ikke komme inn i spillet tidlig og bygge et konkret grunnlag for å designe koden din. Ikke tro hva som kan være din egen fortelling om at dette er et avansert emne som du burde sette av en stund før du er klar. Selv om du er nybegynner, hvis du tar små skritt, kan du bryte hodet rundt luktene og deres refactorings mye tidligere enn du kanskje tror.
Før vi dykker inn i mekanikken, vil jeg gjenta et viktig punkt fra den første artikkelen. Ikke hver lukt er iboende dårlig, og ikke alle refactoring er alltid verdt det. Du må bestemme på stedet - når du har all relevant informasjon til din disposisjon - hvis koden din er stabilere etter en refactoring, og hvis det er verdt din tid å fikse lukten.
La oss se et eksempel fra forrige artikkel. Vi hentet en lang liste med parametere for #assign_new_mission inn i en parameterobjekt via Oppdrag klasse. Så langt så kult.
M med funksjon misunnelse
"ruby class M def assign_new_mission (oppdrag) utskrift" Mission # mission.mission_name har blitt tildelt til # mission.agent_name med målet om # mission.objective. "hvis oppdrag.licence_to_kill print" Lisensen for å drepe har blitt tildelt. "annet utskrift" Lisens til å drepe har ikke blitt gitt. "slutten slutten
klasse oppdrag attr_reader: oppdragnavn,: agent_navn,: objektiv,: lisens_til_kill
def initialisere (oppdragnavn: oppdragnavn, agentnavn: agent_navn, objektiv: objektiv, lisens_til_kill: lisens_til_kill) @mission_name = mission_name @agent_name = agent_name @objective = objektiv @licence_to_kill = lisence_to_kill slutten
m = M.New
oppdrag = Mission.new (oppdragsnavn: 'Octopussy', agentnavn: 'James Bond', mål: 'finn den nukleare enheten', lisens_to_kill: true)
m.assign_new_mission (oppdrag)
"
Jeg nevnte kort hvordan vi kan forenkle M klassen enda mer ved å flytte metoden #assign_new_mission til den nye klassen for parameterobjektet. Det jeg ikke adresserte var det faktum at M hadde en lett herdbar form av har misunnelse også. M var altfor nosy om attributter av Oppdrag. Sett annerledes, hun spurte altfor mange "spørsmål" om oppdragsobjektet. Det er ikke bare et dårlig tilfelle av mikromanagement, men også en veldig vanlig kode lukt.
La meg vise deg hva jeg mener. I M # assign_new_mission, M er "misunnelig" om dataene i det nye parameterobjektet og ønsker å få tilgang til det over alt.
I tillegg til det har du også et parameterobjekt Oppdrag Det er bare ansvar for data akkurat nå - som er en annen lukt, a Dataklasse.
Denne hele situasjonen forteller deg i utgangspunktet det #assign_new_mission ønsker å være et annet sted og M trenger ikke å vite detaljene for hvordan oppdrag blir tildelt. Tross alt, hvorfor ville det ikke være et oppdrags ansvar å tildele nye oppdrag? Husk å alltid sette sammen ting som også forandres sammen.
M uten funksjon misunnelse
"ruby klasse M def assign_new_mission (oppdrag) mission.assign slutten
klasse oppdrag attr_reader: oppdragnavn,: agent_navn,: objektiv,: lisens_til_kill
def initialisere (oppdragsnavn: oppdragsnavn, agentnavn: agentnavn, objektiv: objektiv, lisens_til_kill: lisens_til_kill) @mission_name = oppdragsnavn @agent_name = agent_name @objective = objektiv @ license_to_kill = lisence_to_kill end
def tilordne utskrift "Mission # mission_name har blitt tilordnet til # agent_name med målet til # objective." hvis lisensen_to_kill print "Lisensen for å drepe har blitt gitt." else print "Lisensen for å drepe har ikke vært gitt. "slutten slutten
m = Meldingsoppgave = Mission.new (oppdragsnavn: 'Octopussy', agentnavn: 'James Bond', mål: 'Finn kjernenheten', lisens_til_kill: sant) m.assign_new_mission (oppdrag)
Som du kan se, forenklet vi ting ganske mye. Metoden slanket betydelig og delegerer oppførselen til objektet som er ansvarlig. M ber ikke lenger oppdragsinformasjonsspecifikke lenger og holder seg ikke unna å bli involvert i hvordan oppdrag blir trykt. Nå kan hun fokusere på sin virkelige jobb og trenger ikke å bli forstyrret dersom noen detaljer om oppdragsoppgaver endrer seg. Mer tid for tankespill og jakt på rogue agenter. Vinn-vinn!
Kjenne misunnelse raser entanglement-ved at jeg ikke mener den gode typen, den som lar informasjon reise raskere enn lyset spøkelig-jeg snakker om den som over tid kan la utviklingsmomentet slipes til en hverandre nærmer seg slutt. Ikke bra! Hvorfor det? Rippleeffekter i hele koden din vil skape motstand! En endring på ett sted sommerfugler gjennom alle slags ting, og du ender opp som en drage i en orkan. (Ok, litt overdrevent dramatisk, men jeg gir meg en B + for Bond-referansen der inne.)
Som en generell motgift mot misunnelse, vil du sikte på å designe klasser som er bekymret for det meste om sine egne ting og har om mulig-enkelt ansvar. Kort sagt, klasser skal være noe som vennlig otakus. Sosialt som kanskje ikke er den sunneste oppførselen, men for å designe klasser er det ofte en rimelig retningslinje for å holde momentumet der det skal være fremover!
Navnet er litt dumt, er det ikke? Men samtidig er det en ganske nøyaktig beskrivelse. Høres ut som alvorlig virksomhet, og det er! Heldigvis er det ikke så vanskelig å forstå, men det er likevel en av nastierkoden luktene. Hvorfor? Fordi det raser duplisering som ingen andre, og det er lett å miste alle endringene du måtte gjøre for å fikse ting. Hva som skjer under haglgeværskirurgi, gjør du en endring i en klasse / fil, og du må også røre mange andre klasser / filer som må oppdateres. Håper det høres ikke ut som en god tid du er i for.
For eksempel kan du tenke at du kan komme unna med en liten forandring på ett sted, og innse at du må vade gjennom en hel masse filer for å gjøre det samme forandring eller fikse noe annet som er ødelagt på grunn av det. Ikke bra, ikke i det hele tatt! Det høres mer ut som en god grunn til at folk begynner å hate koden de håndterer.
Hvis du har et spekter med DRY-kode på den ene siden, så er kode som ofte trenger hagleoperasjon, ganske mye i motsatt ende. Ikke vær lat og la deg gå inn på dette territoriet. Jeg er sikker på at du helst vil åpne en fil og bruke endringene dine der og gjøres med den. Det er det slags lat du bør streve for!
For å unngå denne lukten, her er en kort liste over symptomer du kan se etter:
Hva mener vi når vi snakker om kode som er koblet? La oss si at vi har objekter EN og B. Hvis de ikke er koblet, kan du endre en av dem uten å påvirke den andre. Ellers vil du oftere enn ikke også måtte håndtere det andre objektet.
Dette er et problem, og haglgevær kirurgi er et symptom for tett kobling også. Så alltid pass på hvor enkelt du kan endre koden din. Hvis det er relativt enkelt, betyr det at koblingsnivået ditt er akseptabelt lavt. Når det er sagt, skjønner jeg at dine forventninger ville være urealistiske hvis du forventer å unngå å koble hele tiden til enhver pris. Det kommer ikke til å skje! Du vil finne gode grunner til å bestemme deg for at du ønsker å erstatte betingelsene med polymorfisme. I et slikt tilfelle, en liten bit av kobling, hagleoperasjon og å holde API-en av objekter i synkronisering, er vel verdt å kvitte seg med massevis av saksoppgaver via en Null Object (mer om det i et senere stykke).
Vanligvis kan du bruke en av følgende refactorings for å helbrede sårene:
La oss se på noen kode. Dette eksemplet er et stykke av hvordan en Specter-app håndterer betalinger mellom sine entreprenører og onde klienter. Jeg forenklet betalingene litt ved å ha standardavgifter for både entreprenører og kunder. Så det spiller ingen rolle om Specter har til oppgave å kidnappe en katt eller utpresse et helt land: gebyret forblir det samme. Det samme gjelder for hva de betaler sine entreprenører. I sjeldne tilfeller går en operasjon sør og en annen Nr. 2 må bokstavelig talt hoppe over haien, Specter tilbyr full refusjon for å holde onde klienter lykkelige. Specter bruker noen proprietære betalingsmaling som i utgangspunktet er en plassholder for noen form for betalingsprosessor.
I det første eksemplet under, ville det være vondt hvis Specter bestemte seg for å bruke et annet bibliotek for å håndtere utbetalinger. Det ville være flere bevegelige deler involvert, men for å demonstrere hagleoperasjon vil denne mengden kompleksitet gjøre jeg tenker:
Eksempel med haglgevær kirurgi lukt:
"Ruby Class EvilClient # ...
STANDARD_CHARGE = 10000000 BONUS_CHARGE = 10000000
def accept_new_client PaymentGem.create_client (email) slutten
def charge_for_initializing_operation evil_client_id = PaymentGem.find_client (email) .payments_id PaymentGem.charge (evil_client_id, STANDARD_CHARGE) slutter
def charge_for_successful_operation evil_client_id = PaymentGem.find_client (email) .payments_id PaymentGem.charge (evil_client_id, BONUS_CHARGE) slutten
klasse operasjon # ...
REFUND_AMOUNT = 10000000
def refusjon transaction_id = PaymentGem.find_transaction (payments_id) Endring av PaymentGem.refund (transaction_id, REFUND_AMOUNT)
klasse Entreprenør # ...
STANDARD_PAYOUT = 200000 BONUS_PAYOUT = 1000000
def process_payout spectre_agent_id = PaymentGem.find_contractor (email) .payments_id hvis operation.enemy_agent == 'James Bond' && operation.enemy_agent_status == 'Død i handling' PaymentGem.transfer_funds (spectre_agent_id, BONUS_PAYOUT) annet PaymentGem.transfer_funds (spectre_agent_id, STANDARD_PAYOUT) slutten slutten "
Når du ser på denne koden, bør du spørre deg selv: "Skal EvilClients klassen være veldig bekymret for hvordan betalingsprosessoren aksepterer nye onde klienter og hvordan de belastes for operasjoner? "Selvfølgelig ikke! Er det en god ide å spre de ulike beløpene til å betale over alt? Skal implementeringsdetaljer for betalingsprosessoren vises i noen av disse klassene? Mest definitivt ikke!
Se på det fra den måten. Hvis du vil endre noe om måten du håndterer betalinger på, hvorfor skulle du måtte åpne EvilClient klasse? I andre tilfeller kan det være en bruker eller kunde. Hvis du tenker på det, har det ingen mening å gjøre dem kjent med denne prosessen.
I dette eksemplet bør det være enkelt å se at endringer i måten du godtar og overfører betalinger, oppretter rippleeffekter i hele koden din. Også, hvis du ønsker å endre beløpet du belaster eller overfører til entreprenørene dine, trenger du ytterligere endringer over alt. Prime eksempler på hagleoperasjon. Og i dette tilfellet handler vi bare om tre klasser. Tenk deg din smerte hvis litt mer realistisk kompleksitet er involvert. Ja, det er de tingene som marerittene er laget av. La oss se på et eksempel som er litt mer sane:
Eksempel uten haglgevær kirurgi lukt og utvunnet klasse:
"Ruby-klassen PaymentHandler STANDARD_CHARGE = 10000000 BONUS_CHARGE = 10000000 REFUND_AMOUNT = 10000000 STANDARD_CONTRACTOR_PAYOUT = 200000 BONUS_CONTRACTOR_PAYOUT = 1000000
def initialisere (payment_handler = PaymentGem) @payment_handler = payment_handler slutten
def accept_new_client (evil_client) @ payment_handler.create_client (evil_client.email) slutte
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) slutter
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) slutter
def refusjon (drift) transaction_id = @ payment_handler.find_transaction (operation.payments_id) @ payment_handler.refund (transaction_id, REFUND_AMOUNT) slutter
def entreprenør_payout (entreprenør) spectre_agent_id = @ payment_handler.find_contractor (contractor.email) .payments_id hvis operation.enemy_agent == 'James Bond' && operation.enemy_agent_status == 'Død i handling' @ payment_handler.transfer_funds (spectre_agent_id, BONUS_CONTRACTOR_PAYOUT) ellers @ payment_handler.transfer_funds (spectre_agent_id, STANDARD_CONTRACTOR_PAYOUT) slutten slutten
klasse EvilClient # ...
def accept_new_client PaymentHandler.new.accept_new_client (selv) ende
def charge_for_initializing_operation PaymentHandler.new.charge_for_initializing_operation (selv) ende
def charge_for_successful_operation (drift) PaymentHandler.new.charge_for_successful_operation (self) end-end
klasse operasjon # ...
def refusjon PaymentHandler.new.refund (selv) ende ende
klasse Entreprenør # ...
def process_payout PaymentHandler.new.contractor_payout (self) end-end "
Det vi gjorde her er å pakke inn PaymentGem API i vår egen klasse. Nå har vi et sentralt sted hvor vi bruker våre endringer dersom vi bestemmer at for eksempel a SpectrePaymentGem ville fungere bedre for oss. Ikke mer rørende av flere-til-betalings internalsider uten tilknyttede filer hvis vi trenger å tilpasse seg endringer. I klassene som omhandler utbetalinger, fant vi rett og slett det PaymentHandler og delegere den nødvendige funksjonaliteten. Enkel, stabil og ingen grunn til å forandre seg.
Og ikke bare har vi alt i en enkelt fil. Innen PaymentsHandler klasse, er det bare ett sted vi trenger å bytte ut og referere til en mulig ny betalingsprosessor-in initial. Det er rad i boken min. Jo, hvis den nye betalingstjenesten har en helt annen API, må du tilpasse kroppene til et par metoder i PaymentHandler. Det er en liten pris å betale i forhold til full-on hagloperasjon-det er mer som kirurgi for en liten splinter i fingeren. God avtale!
Hvis du ikke er forsiktig når du skriver tester for en betalingsprosessor som denne eller en ekstern tjeneste du må stole på, er du muligens inne for alvorlige hodepine når de endrer API-en. De lider også av forandring, selvfølgelig. Og spørsmålet er ikke, vil de endre deres API, bare når.
Gjennom vår innkapsling er vi i en mye bedre posisjon for å stubbe våre metoder for betalingsprosessoren. Hvorfor? Fordi metodene vi stubber er våre egne, endres de bare når vi vil ha dem til. Det er en stor seier. Hvis du er ny til testing, og dette ikke er helt klart for deg, ikke bekymre deg for det. Ta den tiden du trenger; dette emnet kan være vanskelig først. Fordi det er en slik fordel, ville jeg bare nevne det for fullstendig skyld.
Som du kan se, forenklet jeg betalingsprosessen ganske mye i dette dumme eksemplet. Jeg kunne også ha renset sluttresultatet noe mer, men poenget var å tydelig demonstrere lukten og hvordan du kan bli kvitt den gjennom abstraksjon.
Hvis du ikke er helt fornøyd med denne klassen og ser muligheter for refactoring, hilser jeg deg - og er glad for å ta æren for det. Jeg anbefaler deg å slå deg ut! En god start kan ha å gjøre med måten du finner payments_ids. Klassen selv ble også litt overfylt allerede ...
Divergerende endring er på en måte det motsatte av hagleoperasjon - hvor du vil endre en ting og trenger å sprenge den forandringen gjennom en rekke forskjellige filer. Her endres en enkelt klasse ofte av forskjellige grunner og på forskjellige måter. Min anbefaling er å identifisere deler som forandrer seg sammen og trekke dem ut i en egen klasse som kan fokusere på det ene ansvaret. Disse klassene i sin tur burde heller ikke ha mer enn én grunn til å forandre. Hvis ikke, vil en annen divergerende endringsluft trolig vente på å bite deg.
Klasser som lider av divergerende endring er de som blir forandret mye. Med verktøy som Churn kan du måle hvor ofte bestemte deler av koden din måtte endre tidligere. Jo flere poeng du finner på en klasse, desto større er sannsynligheten for at divergerende endringer kan være på jobb. Jeg ville heller ikke bli overrasket om nøyaktig disse klassene er de som forårsaker de fleste feilene generelt.
Ikke misforstå meg: å bli forandret ofte er ikke direkte lukten. Det er imidlertid et nyttig symptom. Et annet veldig vanlig og mer eksplisitt symptom er at dette objektet trenger å jonglere mer enn ett ansvar. De enkeltansvarsprinsipp SRP er en utmerket retningslinje for å hindre at denne koden lukter og å skrive mer stabil kode generelt. Det kan være vanskelig å følge, men likevel verdt slakken.
La oss se på dette ekkel eksempelet nedenfor. Jeg endret shotgun kirurgi eksempelet litt. Blofeld, spekterets leder, kan være kjent for micromanage ting, men jeg tviler på at han ville være interessert i halvparten av denne klassen er involvert med.
"Ruby Class Specter
STANDARD_CHARGE = 10000000 STANDARD_PAYOUT = 200000
def charge_for_initializing_operation (klient) evil_client_id = PaymentGem.find_client (client.email) .payments_id PaymentGem.charge (evil_client_id, STANDARD_CHARGE) slutter
def contractor_payout (entreprenør) spectre_agent_id = PaymentGem.find_contractor (contractor.email) .payments_id PaymentGem.transfer_funds (spectre_agent_id, STANDARD_PAYOUT) slutter
def assign_new_operation (operasjon) operation.contractor = 'Noen onde dude' operation.objective = 'Stjel en båt av verdifulle ting' operation.deadline = 'Midnatt, 18. november' slutten
def print_operation_assignment (drift) print "# operation.contractor er tildelt til # operation.objective. Oppdragsfristen avsluttes på # operation.deadline. "Slutt
def dispose_of_agent (spectre_agent) setter "Du skuffet denne organisasjonen. Du vet hvordan Specter håndterer feil. Farvel # spectre_agent.code_name! "End-end"
De Spekter klassen har altfor mange forskjellige ting det er bekymret for:
Syv forskjellige ansvarsområder på en enkelt klasse. Ikke bra! Du må endre hvordan agenter bortskaffes? En vektor for å endre Spekter klasse. Du ønsker å håndtere utbetalinger annerledes? En annen vektor. Du får driften.
Selv om dette eksemplet er langt fra å være realistisk, forteller det fortsatt historien om hvor lett det er å unødig samle oppførsel som må endres ofte på et enkelt sted. Vi kan gjøre det bedre!
"Ruby Class Specter # ...
def dispose_of_agent (spectre_agent) setter "Du skuffet denne organisasjonen. Du vet hvordan Specter håndterer feil. Farvel # spectre_agent.code_name! "Slutten
klasse PaymentHandler STANDARD_CHARGE = 10000000 STANDARD_CONTRACTOR_PAYOUT = 200000
# ...
def initialisere (payment_handler = PaymentGem) @payment_handler = payment_handler slutten
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) slutter
def contractor_payout (entreprenør) spectre_agent_id = @ payment_handler.find_contractor (contractor.email) .payments_id @ payment_handler.transfer_funds (spectre_agent_id, STANDARD_CONTRACTOR_PAYOUT) slutten slutten
klasse EvilClient # ...
def charge_for_initializing_operation PaymentHandler.new.charge_for_initializing_operation (selv) ende ende
klasse Entreprenør # ...
def process_payout PaymentHandler.new.contractor_payout (selv) slutten
klasse Drift Attr_accessor: entreprenør,: objektiv,: frist
def initialisere (attrs = ) @contractor = attrs [: entreprenør] @objective = attrs [: objektiv] @deadline = attrs [: deadline] end
def print_operation_assignment print "# entreprenør er tilordnet # objective. Oppgavingsfristen avsluttes på # deadline. "End-end"
Her hentet vi ut en gruppe klasser og ga dem sitt eget unike ansvar - og derfor deres egen begrunnelse for å forandre seg.
Du vil håndtere utbetalinger annerledes? Nå trenger du ikke å røre på Spekter klasse. Du må lade opp eller betale ut annerledes? Igjen, det er ikke nødvendig å åpne filen for Spekter. Utskriftsoperasjonsoppgaver er nå operasjonsvirksomheten der den tilhører. Det er det. Ikke for komplisert, tror jeg, men definitivt en av de mer vanlige lukter du burde lære å håndtere tidlig.
Jeg håper du kommer til det punktet hvor du føler deg klar til å prøve disse refactorings i din egen kode og ha en enklere tid å identifisere kode lukter rundt deg. Pass på at vi nettopp har begynt, men at du allerede taklet et par store. Jeg vedder på at det ikke var så vanskelig som du en gang kanskje hadde trodd!
Visste eksempler på ekte verden vil være mye mer utfordrende, men hvis du har forstått mekanikken og mønstrene for å få øye på lukter, vil du sikkert kunne tilpasse deg raskt til realistiske kompleksiteter.
Accentsconagua