Slide 1

Slide 1 text

Rails anti patterns Florent Guilleux, Ruby Peru meetup, April 10th 2014 1 / 13

Slide 2

Slide 2 text

Disclaimer Not exhaustive list, not always Rails specific... 2 / 13

Slide 3

Slide 3 text

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

Slide 4

Slide 4 text

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

Slide 5

Slide 5 text

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

Slide 6

Slide 6 text

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

Slide 7

Slide 7 text

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

Slide 8

Slide 8 text

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

Slide 9

Slide 9 text

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

Slide 10

Slide 10 text

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

Slide 11

Slide 11 text

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

Slide 12

Slide 12 text

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

Slide 13

Slide 13 text

Last anti pattern: Using Rails 13 / 13