Ruby / Rails Code Smell Basics 01

emner

  • Hodet oppe
  • Motstand
  • Stor klasse / Gud klasse
  • Utdrag klasse
  • Lang metode
  • Long Parameter List

Heads Up

Følgende korte serie av artikler er ment for litt erfarne Ruby-utviklere og forretter. Jeg hadde inntrykk av at kode lukter og deres refactorings kan være veldig skremmende og skremmende for nybegynnere, spesielt hvis de ikke er i den heldige posisjonen å ha mentorer som kan slå mystiske programmeringskoncept til skinnende lyspærer.

Å ha åpenbart gikk i disse skoene selv, husket jeg at det føltes unødvendig tøft å komme seg inn i kode lukter og refactorings.

På den ene siden forventer forfattere et visst nivå av ferdighet og kan derfor ikke føle seg superforpliktet til å gi leseren samme mengde kontekst som en nybegynner kan trenge å dykke seg komfortabelt inn i denne verden før.

Som en konsekvens danner nybegynnere på den annen side inntrykk av at de skal vente litt lenger til de er mer avanserte for å lære om lukter og refactorings. Jeg er ikke enig med denne tilnærmingen og tror at det å gjøre dette emnet mer tilnærmet, vil hjelpe dem med å utforme bedre programvare tidligere i karrieren. I hvert fall håper jeg det bidrar til å gi junior peeps med en solid head start.

Så hva snakker vi om nøyaktig når folk nevner kode lukter? Er det alltid et problem i koden din? Ikke nødvendigvis! Kan du unngå dem helt? Jeg tror ikke det! Mener du at kode lukter fører til ødelagt kode? Vel, noen ganger og noen ganger ikke. Skal det være min prioritet å fikse dem med en gang? Samme svar, jeg frykter: noen ganger ja og noen ganger skal du sikkert steke større fisk først. Er du gal? Fair spørsmål på dette punktet!

Før du fortsetter å dykke inn i denne stinkende virksomheten, husk å ta bort en ting fra alt dette: Ikke prøv å fikse hver lukt du møter - dette er absolutt sløsing med tiden din!

Det virker for meg at kode lukter er litt vanskelig å pakke opp i en pent merket boks. Det er alle slags lukter med ulike alternativer for å ta dem. Også forskjellige programmeringsspråk og rammer er utsatt for forskjellige typer lukter, men det er definitivt mange vanlige "genetiske" stammer blant dem. Mitt forsøk på å beskrive kode lukter er å sammenligne dem med medisinske symptomer som forteller deg at du kan ha et problem. De kan peke på alle slags latente problemer og har et bredt utvalg av løsninger hvis de diagnostiseres.

Heldigvis er de generelt ikke så kompliserte som å håndtere menneskekroppen og psyken selvfølgelig. Det er imidlertid en god sammenligning, fordi noen av disse symptomene må behandles med en gang, og noen andre gir deg god tid til å komme opp med en løsning som passer best for "pasientens" generelle trivsel. Hvis du har arbeidskode og du kommer inn i noe stinkende, må du ta den harde avgjørelsen om det er verdt tiden å finne en løsning, og hvis den refactoring forbedrer stabiliteten til appen din.

Når det blir sagt, hvis du snubler over kode som du kan forbedre med en gang, er det godt råd å forlate koden bak litt bedre enn før, selv om en liten bit bedre legges opp vesentlig over tid.

Motstand

Kvaliteten på koden din blir tvilsom dersom innlemmelsen av ny kode blir vanskeligere å bestemme hvor du skal sette ny kode er en smerte eller kommer med mange rippleeffekter gjennom din kodebase, for eksempel. Dette kalles motstand.

Som retningslinje for kodekvalitet kan du alltid måle det med hvor enkelt det er å innføre endringer. Hvis det blir vanskeligere og vanskeligere, er det definitivt tid til å refactor og å ta den siste delen av rød-grønn-refactor mer seriøst i fremtiden.

Stor klasse / Gud klasse

La oss begynne med noe fancy lyding - "Gudsklasser" - fordi jeg tror de er spesielt enkle å forstå for nybegynnere. Gudsklasser er et spesielt tilfelle av en kode lukt kalt Stor klasse. I denne delen skal jeg adressere dem begge. Hvis du har brukt litt tid i Rails land, har du sannsynligvis sett dem så ofte at de ser normalt ut til deg.

Du husker sikkert "mantraen" fete modeller, mager kontrolleren "? Vel faktisk, tynn er bra for alle disse klassene, men som en veiledning er det gode råd for nybegynnere jeg antar.

Gudsklasser er objekter som tiltrekker seg all slags kunnskap og oppførsel som et svart hull. Vanlige mistenkte bruker oftest brukermodellen og hva som helst problem (forhåpentligvis!) Appen din prøver å løse først og fremst i det minste. En todo app kan masse opp på todos modell, en handlekurv på Produkter, et bildeapp på Bilder-du får driften.

Folk kaller dem gudklasser fordi de vet for mye. De har for mange sammenhenger med andre klasser - for det meste fordi noen modellerte dem lat. Det er imidlertid hardt å holde gudklasser i sjakk. De gjør det veldig enkelt å dumpe mer ansvar på dem, og som mange greske helter ville attestere, tar det litt ferdighet å dele og erobre «guder».

Problemet med dem er at de blir vanskeligere og vanskeligere å forstå, spesielt for nye gruppemedlemmer, vanskeligere å endre, og gjenbruk av dem blir mindre og mindre av et alternativ, jo mer tyngdekraften de har samlet. Åh ja, du har rett, dine tester er unødvendig vanskeligere å skrive også. Kort sagt, det er egentlig ikke en oppside å ha store klasser, og spesielt gudklasser.

Det er et par vanlige symptomer / tegn på at klassen din trenger litt heroisme / kirurgi:

  • Du må bla!
  • Massevis av private metoder?
  • Har klassen din syv eller flere metoder på den?
  • Vanskelig å fortelle hva klassen virkelig gjør - konsistent!
  • Har klassen din mange grunner til å endre når koden din utvikler seg?

Også, hvis du knuser på klassen din og tenker "Eh? Ew! "Du kan også være med på noe også. Hvis alt som høres kjent, er sjansene gode at du fant deg et fint eksemplar.

"ruby class CastingInviter EMAIL_REGEX = /\A([^@\s]+)@((?:[-a-z0-9]+.)+[a-z]2,)\z/

attr_reader: melding,: inviterte,: casting

def initialisere (attributes = ) @message = attributter [: melding] || "@invitees = attributter [: invitees] ||" @sender = attributter [: sender] @casting = attributter [: casting] end

def gyldig? valid_message? && valid_invitees? slutten def levere hvis gyldig? invitee_list.each do | email | invitasjon = create_invitation (email) Mailer.invitation_notification (invitasjon, @message) end else fail_message = "Din # @casting melding kunne ikke sendes. Inviterer e-post eller melding er ugyldig" invitation = create_invitation (@sender) Mailer.invitation_notification (invitasjon, feilmelding) slutten privat def invalid_invitees @invalid_invitees || = invitee_list.map do | item | med mindre element item.match (EMAIL_REGEX) slutter.compact end def invitee_list @invitee_list || = @ invitees.gsub (/ \ s + /, "). split (/ [\ n,;] + /) slutten def valid_message? @ message.present? endte def valid_invitees? invalid_invitees.empty? ende 

def create_invitation (email) Invitation.create (casting: @casting, avsender: @sender, invitee_email: email, status: 'waiting') slutten "

Ugly fella, va? Kan du se hvor mye nastiness er buntet her inne? Selvfølgelig legger jeg litt kirsebær på toppen, men du vil komme inn i kode som dette før eller senere. La oss tenke på hva ansvar dette CastingInviter klassen må sjonglere.

  • Leverer e-post
  • Sjekker for gyldige meldinger og e-postadresser
  • Bli kvitt hvit plass
  • Splitting e-postadresser på komma og semikolon

Skal alt dette bli dumpet på en klasse som bare ønsker å levere et avstøpningsanrop via levere? Absolutt ikke! Hvis din invitasjonsmetode endres, kan du forvente å kjøre inn i noen hagleoperasjoner. CastingInviter trenger ikke å vite mesteparten av disse detaljene. Det er mer ansvar for noen klasser som er spesialisert på å håndtere e-postrelaterte ting. I fremtiden vil du finne mange grunner til å endre koden din også her.

Utdrag klasse

Så hvordan skal vi håndtere dette? Ofte er det å trekke ut en klasse et praktisk refaktoremønster som vil presentere seg som en rimelig løsning på slike problemer som store, innviklede klasser - spesielt når den aktuelle klassen omhandler flere ansvarsområder.

Private metoder er ofte gode kandidater til å begynne med og enkle karakterer også. Noen ganger må du trekke ut mer enn en klasse fra en så dårlig gutt, bare gjør ikke alt i ett steg. Når du finner nok sammenhengende kjøtt som ser ut til å tilhøre et eget spesialobjekt, kan du trekke ut den funksjonaliteten i en ny klasse.

Du oppretter en ny klasse og flyttes funksjonaliteten gradvis etter en etter en. Flytt hver metode separat, og endre navn på dem hvis du ser en grunn til det. Deretter refererer den nye klassen til den opprinnelige og delegerer den nødvendige funksjonaliteten. God ting du har testdekning (forhåpentligvis!) Som lar deg sjekke om ting fortsatt fungerer riktig hvert trinn på veien. Sikt på å kunne gjenbruke dine utvente klasser også. Det er lettere å se hvordan det er gjort i aksjon, så la oss lese noen kode:

"Ruby Class CastingInviter

attr_reader: melding,: inviterte,: casting

def initialisere (attributes = ) @message = attributter [: message] || "@invitees = attributter [: invitees] ||" @casting = attributter [: casting] @sender = attributter [: sender] slutten

def gyldig? casting_email_handler.valid? slutt

def levere casting_email_handler.deliver end

privat

def casting_email_handler @casting_email_handler || = CastingEmailHandler.new (melding: melding, invitasjoner: invitasjoner, casting: casting, sender: @sender) end-end "

"ruby class CastingEmailHandler EMAIL_REGEX = /\A([^@\s]+)@((?:[-a-z0-9]+.)+[a-z]2,)\z/

def initialiser (attr = ) @message = attr [: melding] || "@invitees = attr [: invitees] ||" @casting = attr [: casting] @ sender = attr [: sender] slutten

def gyldig? valid_message? && valid_invitees? slutt

def levere hvis gyldig? invitee_list.each do | email | invitasjon = create_invitation (email) Mailer.invitation_notification (invitasjon, @message) end else fail_message = "Din # @casting melding kunne ikke sendes. Inviterer e-post eller melding er ugyldig "invitation = create_invitation (@sender) Mailer.invitation_notification (invitasjon, failure_message) slutten

privat

def invalid_invitees @invalid_invitees || = invitee_list.map do | item | med mindre elementet item.match (EMAIL_REGEX) slutter end.compact-slutt

def invitee_list @invitee_list || = @ invitees.gsub (/ \ s + /, "). delt (/ [\ n,; + +) ende

def valid_invitees? invalid_invitees.empty? slutt

def valid_message? @ Message.present? slutt

def create_invitation (email) Invitation.create (casting: @casting, avsender: @sender, invitee_email: email, status: 'waiting') slutten "

I denne løsningen ser du ikke bare hvordan denne adskillelsen av bekymringer påvirker kodekvaliteten din, den leser også mye bedre og blir lettere å fordøye.

Her delegerer vi metoder til en ny klasse som er spesialisert på å håndtere leveransen av disse invitasjonene via e-post. Du har ett dedikert sted som sjekker om meldingene og inviterte er gyldige og hvordan de skal leveres. CastingInviter trenger ikke å vite noe om disse detaljene, så vi overfører disse ansvarene til en ny klasse CastingEmailHandler.

Kunnskapen om hvordan man leverer og kontrollerer for gyldigheten av disse kasteinvitasjonsemailene, er nå alle inneholdt i vår nye utvunnet klasse. Har vi mer kode nå? Det kan du vedde på! Var det verdt å skille bekymringer? Ganske sikker! Kan vi gå utover det og refactor CastingEmailHandler litt til? Absolutt! Slå deg løs!

I tilfelle du lurer på om gyldig? metode på CastingEmailHandler og CastingInviter, dette er for RSpec å opprette en tilpasset matcher. Dette lar meg skrive noe som:

Ruby forvente (casting_inviter) .til være ugyldig

Ganske nyttig, tror jeg.

Det er flere teknikker for å håndtere store klasser / gudobjekter, og i løpet av denne serien lærer du et par måter å refactor slike gjenstander.

Det er ikke noe fast resept for å håndtere disse tilfellene, det avhenger alltid, og det er en sak for sak dommen om du trenger å ta med de store kanonene, eller hvis mindre, inkrementelle refactoring teknikker forplikter seg best. Jeg vet, litt frustrerende til tider. Etter å ha fulgt ansvaret for enkelt ansvar (SRP), vil det gå en lang vei, og det er en god nese å følge.

Lang metode

Å ha metoder som er litt store, er en av de vanligste tingene du møter som utvikler. Generelt vil du vite på et øyeblikk hva en metode skal gjøre. Det bør også ha bare ett nivå av hekke eller et nivå av abstraksjon. Kort sagt, unngå å skrive kompliserte metoder.

Jeg vet at dette høres hardt ut, og det er ofte. En løsning som kommer opp ofte, utvinner deler av metoden til en eller flere nye funksjoner. Denne refactoring teknikken kalles ekstraktmetode-Det er en av de enkleste, men likevel svært effektive. Som en fin bivirkning blir koden mer lesbar hvis du navngi dine metoder på riktig måte.

La oss ta en titt på funksjonsspesifikasjoner hvor du trenger denne teknikken mye. Jeg husker å bli introdusert til ekstraktmetode mens du skriver slike spesifikasjoner og hvor fantastisk det føltes da lyspæren fortsatte. Fordi funksjonsspesifikasjoner som dette er enkle å forstå, er de en god kandidat til demonstrasjon. I tillegg vil du komme inn i lignende scenarier igjen og igjen når du skriver dine spesifikasjoner.

spec / funksjoner / some_feature_spec.rb

"rubin krever" rails_helper '

funksjonen 'M merker oppdrag som fullført' gjør scenario 'vellykket' gjør visit_root_path fill_in 'Email' med: '[email protected]' click_button Send inn besøk missions_path click_on 'Opprett misjon' fill_in 'Oppdragsnavn' med: 'Prosjekt Moonraker 'klikk' Send '

i "li: contains ('Project Moonraker')" gjør klikk på 'Oppdrag fullført' sluttforvente (side) .for å ha_css 'ul.missions li.mission-name.completed', tekst: 'Project Moonraker' slutten "

Som du lett kan se, skjer det mye i dette scenariet. Du går til indekssiden, logger på og oppretter et oppdrag for oppsettet, og trener deretter ved å markere oppdraget som komplett, og til slutt verifiserer du oppførselen. Ingen rakettvitenskap, men også ikke ren og definitivt ikke komponert for gjenbruk. Vi kan gjøre det bedre enn det:

spec / funksjoner / some_feature_spec.rb

"rubin krever" rails_helper '

funksjon 'M merker oppdrag som komplett' gjør scenario 'vellykket' gjør sign_in_as '[email protected]' create_classified_mission_named 'Project Moonraker'

mark_mission_as_complete 'Project Moonraker' agent_sees_completed_mission 'Project Moonraker' slutten 

def create_classified_mission_named (oppdragsnavn) besøk missions_path click_on 'Create Mission' fill_in 'Oppdragsnavn', med: oppdragsnavn click_button 'Send' slutt

def mark_mission_as_complete (oppdragsnavn) innenfor "li: contains ('# mission_name')" do click_on 'Mission completed' end-end

def agent_sees_completed_mission (mission_name) forvente (side) .to have_css 'ul.missions li.mission-name.completed', tekst: oppdragsnavn slutten

def sign_in_as (email) besøk root_path fill_in 'Email', med: email click_button 'Send' end "

Her hentet vi ut fire metoder som lett kan brukes på nytt i andre tester nå. Jeg håper det er klart at vi treffer tre fugler med en stein. Funksjonen er mye mer konsistent, den leser bedre, og den består av ekstraherte komponenter uten duplisering.

La oss forestille deg at du hadde skrevet alle slags lignende scenarier uten å utvinne disse metodene, og du ønsket å endre noen implementering. Nå ønsker du at du hadde tatt deg tid til å refactor dine tester og hadde ett sentralt sted å bruke endringene dine.

Jo, det er en enda bedre måte å håndtere funksjonsspesifikasjoner som dette-Page-objektene, for eksempel - men det er ikke vårt rom for i dag. Jeg antar det er alt du trenger å vite om utvinning metoder. Du kan bruke dette refactoringmønsteret overalt i koden din - ikke bare i spesifikasjoner, selvfølgelig. Med hensyn til bruksfrekvensen er det min gjetning at det vil være din første teknikk for å forbedre kvaliteten på koden din. Ha det gøy!

Long Parameter List

La oss lukke denne artikkelen med et eksempel på hvordan du kan slanke parametrene dine. Det blir kjedelig ganske fort når du må mate dine metoder mer enn ett eller to argumenter. Ville det ikke vært fint å slippe i ett objekt i stedet? Det er akkurat det du kan gjøre hvis du introduserer en parameterobjekt.

Alle disse parametrene er ikke bare en smerte for å skrive og holde orden, men kan også føre til kod duplisering - og vi vil sikkert unngå det der det er mulig. Det jeg liker spesielt om denne refactoring teknikken er hvordan dette påvirker andre metoder også. Du er ofte i stand til å kvitte seg med en masse parameter søppel ned i næringskjeden.

La oss gå over dette enkle eksempelet. M kan tildele et nytt oppdrag og trenger et oppdragsnavn, en agent og et mål. M er også i stand til å bytte agenter 'dobbel 0-status - som betyr at deres lisens skal drepe.

"ruby class M def assign_new_mission (oppdragsnavn, agentnavn, objektiv, lisens_to_kill: null) print" Mission # mission_name har blitt tildelt til # agent_name med målet om # objective. "hvis lisensen_to_kill print" Lisensen for å drepe har blitt tildelt. "annet utskrift" Lisens til å drepe har ikke blitt gitt. "slutten slutten

m = Meldingen m.assign_new_mission ('Octopussy', 'James Bond', 'finn den nukleare enheten', lisens_to_kill: true) # => Mission Octopussy har blitt tildelt James Bond med sikte på å finne den nukleare enheten. Tillatelsen til å drepe er gitt. "

Når du ser på dette og spør hva som skjer når oppdraget "parametere" vokser i kompleksitet, er du allerede på noe. Det er et smertepunkt som du bare kan løse hvis du passerer i et enkelt objekt som har all den informasjonen du trenger. Oftere enn ikke, dette hjelper deg også til å holde seg borte fra å endre metoden hvis parameterobjektet endres av en eller annen grunn.

"ruby class oppdrag attr_reader: mission_name,: agent_name,: objective,: license_to_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 "Tillatelsen til å drepe har blitt gitt." ellers skriv ut "Lisensen for å drepe har ikke blitt gitt." sluttendens ende 

klasse M def assign_new_mission (oppdrag) mission.assign slutten

m = Meldingsoppgave = Mission.new (oppdragsnavn: 'Octopussy', agentnavn: 'James Bond', mål: 'finn kjernekraften', lisens_til_kill: sant) m.assign_new_mission (oppdrag) tildelt James Bond med sikte på å finne den nukleare enheten. Tillatelsen til å drepe er gitt. "

Så vi opprettet et nytt objekt, Oppdrag, som er utelukkende fokusert på å tilby M med den informasjonen som trengs for å tildele et nytt oppdrag og gi #assign_new_mission med et enkeltparameterobjekt. Du trenger ikke å passere i disse irriterende parametrene selv. I stedet fortelle du objektet om å avsløre den informasjonen du trenger inne i selve metoden. I tillegg tok vi også ut litt oppførsel - informasjonen om hvordan du skriver ut i det nye Oppdrag gjenstand.

Hvorfor burde M trenger å vite om hvordan du skriver ut oppdragsoppgaver? Den nye #tildele også dra nytte av utvinning ved å miste vekt fordi vi ikke behøvde å passere i parameterobjektet, så det var ikke nødvendig å skrive ting som mission.mission_name, mission.agent_name og så videre. Nå bruker vi bare vår attr_reader(e), som er mye renere enn uten utvinning. Du graver?

Det er også praktisk om dette Oppdrag kan samle alle slags ekstra metoder eller stater som er pent innkapslet på ett sted og klar for deg å få tilgang til.

Med denne teknikken vil du ende opp med metoder som er mer konsise, pleier å lese bedre, og unngå å gjenta samme parametergruppe overalt. Ganske god avtale! Å bli kvitt identiske parametergrupper er også en viktig strategi for DRY-koden.

Prøv å se opp for å trekke ut mer enn bare dine data. Hvis du også kan plassere atferd i den nye klassen, har du objekter som er mer nyttige - ellers begynner de raskt å lukte også.

Visst, det meste vil du komme inn i mer kompliserte versjoner av det - og testene dine må sikkert også tilpasses samtidig under slike refactorings-men hvis du har det enkle eksempelet under beltet, vil du være klar for handling.

Jeg skal se den nye Bond nå. Hørt det er ikke så bra, skjønt ...

Oppdater: Saw Specter. Min dom: Sammenlignet med Skyfall-som var MEH imho-Specter var wawawiwa!