Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Rails anti patterns

Rails anti patterns

A loose collection of common bad practices in Rails projects

Florent Guilleux

April 10, 2014
Tweet

More Decks by Florent Guilleux

Other Decks in Programming

Transcript

  1. Storing credentials in versioned files If your source code is

    accessed by an attacker, people may use your credentials: for their own benefits (S3) send emails on your behalf (SendGrid) harm your reputation (Twitter) impersonate users (s e c r e t _ k e y _ b a s e ) erase data (S3) ... So store your credentials in ENV variables. 3 / 13
  2. Using different database engines in development and production DBs engines

    have differences, what works with your SQLite in development may not with the PostgreSQL in production. For instance PostgreSQL follows more closely the SQL standard for the G R O U P B Y clause. 4 / 13
  3. Anti patterns in views write DB calls misuse of h

    t m l _ s a f e < % = " < h 1 > # { t e x t } < / h 1 > " . h t m l _ s a f e % > # t e x t i s n o t e s c a p e d ! # s h o u l d b e d i r e c t l y < % = c o n t e n t _ t a g : h 1 , t e x t % > See Everything you know about html_safe is wrong overuse of view helpers < % = l i n k _ t o " A n o t h e r w e b p a g e " , " h t t p : / / a n o t h e r p a g e . c o m " % > # s h o u l d b e d i r e c t l y < a h r e f = " h t t p : / / a n o t h e r p a g e . c o m " > A n o t h e r w e b p a g e < / a > 5 / 13
  4. Muti anti patterns combo c l a s s P

    o s t < A c t i v e R e c o r d : : B a s e a f t e r _ c r e a t e : p o s t _ t o _ t w i t t e r p r i v a t e d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " e n d e n d 6 / 13
  5. Muti anti patterns combo c l a s s P

    o s t < A c t i v e R e c o r d : : B a s e a f t e r _ c r e a t e : p o s t _ t o _ t w i t t e r p r i v a t e d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " e n d e n d What's wrong here: no handling of the possible failures of the external API call if the call to the Twitter API fails (for instance Twitter is unavailable), the P o s t record is not created tweeting is probably not a core responsibility of the P o s t class P o s t instance creation is unnecessarily slow 7 / 13
  6. Not dealing correctly with possible failures of external APIs Calls

    to external APIs may fail to a variety of reasons (incorrect params, API change, call timeout, network issue, etc.). You need to handle gracefully possible errors (it's not easy to find out all the exception classes you need to catch...) d e f p o s t _ t o _ t w i t t e r t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { c o n t e n t } " r e s c u e T w i t t e r : : E r r o r , T i m e o u t : : E r r o r , E r r n o : : E C O N N R E S E T # , e t c . # h a n d l e t h e e r r o r e n d 8 / 13
  7. Putting in a callback a non core responsibility of a

    class creates a new dependency in the class harder to grasp the full behavior of the class make instance creation/update/destroy slower can't easily create a record without side effects 9 / 13
  8. Some alternatives to callbacks Put the call to Twitter in

    the controller (either directly or through a service object): c l a s s P o s t s C o n t r o l l e r < A p p l i c a t i o n C o n t r o l l e r d e f c r e a t e . . . i f @ p o s t . s a v e t w i t t e r _ c l i e n t . u p d a t e " N e w p o s t : # { @ p o s t . c o n t e n t } " r e d i r e c t _ t o @ p o s t e n d e n d Or, if posting to Twitter is a core responsibility of the class, decorate the native s a v e method: d e f s a v e _ a n d _ b r o a d c a s t t o _ t w i t t e r = f a l s e i f s a v e t w i t t e r _ c l i e n t . u p d a t e ( " N e w p o s t : # { @ p o s t . c o n t e n t } " ) i f t o _ t w i t t e r e n d e n d 10 / 13
  9. Good practice: use a callback only when the logic refers

    to state internal to the object source: "The only acceptable use for callbacks in Rails ever" 11 / 13
  10. Calling external API "inline" We don't know how much time

    an external API call can take. If we don't need the result of the call, or don't need it immediately, just do the call in a background job. c l a s s P o s t s C o n t r o l l e r < A p p l i c a t i o n C o n t r o l l e r d e f c r e a t e . . . i f @ p o s t . s a v e T w i t t e r P o s t W o r k e r . p e r f o r m _ a s y n c ( @ p o s t . i d ) r e d i r e c t _ t o @ p o s t e n d e n d 12 / 13