The Beauty of Bad Code

The Beauty of Bad Code

Given at Web Rebels in Oslo, Norway, 1-2 June 2017

583a0cfd3e0ec851166c5c6fa5e506a5?s=128

Raquel Vélez

June 02, 2017
Tweet

Transcript

  1. The Beauty of Bad Code brought to you by @rockbot

  2. @rockbot #jsnz npm is a package manager for JavaScript

  3. @rockbot #jsnz npm is a package manager

  4. @rockbot #webrebels our #1 priority is to reduce friction

  5. @rockbot #webrebels npm i npm@latest -g npm @5

  6. @rockbot #webrebels we’ve written a lot of code

  7. @rockbot #webrebels we’ve rewritten our code

  8. @rockbot #webrebels we’ve rewritten our code several times

  9. @rockbot #webrebels we know bad code

  10. @rockbot #webrebels courtesy of npm wombats what is bad code?

  11. @rockbot #webrebels functionality

  12. @rockbot #webrebels functionality maintainability

  13. @rockbot #webrebels functionality maintainability readability

  14. @rockbot #webrebels functionality

  15. @rockbot #webrebels super-villain code

  16. @rockbot #webrebels 110 /* here's the potential situation: let's say

    111 I'm a hacker and I make a 112 package that does Something Evil™ then I 113 add you as a collaborator `npm 114 adduser isaacs evil-pkg` and then I publish 115 the package and then I remove 116 myself from the package so it looks like YOU 117 are the one who made the 118 package well, that's nasty so we blocked that 119 from showing because 120 hypothetically your friends would be like, 121 hey! this evil-pkg from isaacs 122 looks awesome, let me use it! and then I get 123 all their bank account numbers 124 and get super duper rich and become a VC and 125 create LinkedIn for Cats */
  17. @rockbot #webrebels doesn’t do the thing it’s supposed to do

  18. @rockbot #webrebels 1 const tap = require('tap') 2 const urlOf

    = require('./lib/url') 3 4 require('./lib/sharedNemo').then(nemo => { 5 tap.test('User with feature flag gets to the 6 new org creation page', t => { 7 return nemo.driver.get(urlOf('/express')) 8 .then(() => nemo.view.page.h1WaitVisible() 9 ) 10 .then(() => nemo.view.page.h1TextEquals( 11 'express')) 12 }) 13 14 tap.test('quit', () => nemo.driver.quit()) 15 }) npm-website/integration/package-redirect.js
  19. @rockbot #webrebels 1 const tap = require('tap') 2 const urlOf

    = require('./lib/url') 3 4 require('./lib/sharedNemo').then(nemo => { 5 tap.test('User with feature flag gets to the 6 new org creation page', t => { 7 return nemo.driver.get(urlOf('/express')) 8 .then(() => nemo.view.page.h1WaitVisible() 9 ) 10 .then(() => nemo.view.page.h1TextEquals( 11 'express')) 12 }) 13 14 tap.test('quit', () => nemo.driver.quit()) 15 }) npm-website/integration/package-redirect.js
  20. @rockbot #webrebels 1 const tap = require('tap') 2 const urlOf

    = require('./lib/url') 3 4 require('./lib/sharedNemo').then(nemo => { 5 tap.test('User with feature flag gets to the 6 new org creation page', t => { 7 return nemo.driver.get(urlOf('/express')) 8 .then(() => nemo.view.page.h1WaitVisible() 9 ) 10 .then(() => nemo.view.page.h1TextEquals( 11 'express')) 12 }) 13 14 tap.test('quit', () => nemo.driver.quit()) 15 }) npm-website/integration/package-redirect.js
  21. @rockbot #webrebels 1 const tap = require('tap') 2 const urlOf

    = require('./lib/url') 3 4 require('./lib/sharedNemo').then(nemo => { 5 tap.test('User with feature flag gets to the 6 new org creation page', t => { 7 return nemo.driver.get(urlOf('/express')) 8 .then(() => nemo.view.page.h1WaitVisible() 9 ) 10 .then(() => nemo.view.page.h1TextEquals( 11 'express')) 12 }) 13 14 tap.test('quit', () => nemo.driver.quit()) 15 }) npm-website/integration/package-redirect.js
  22. @rockbot #webrebels security risks

  23. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 18 request(url, function (err, resp, content) { 19 ... 25 return next(err, marked.parse(content)); 26 }); 27 } 28 }
  24. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 18 request(url, function (err, resp, content) { 19 ... 25 return next(err, marked.parse(content)); 26 }); 27 } 28 }
  25. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17
  26. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 system-set
  27. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 user-set system-set
  28. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 https://raw.githubusercontent.com/npm/newww/master/policies.md user-set system-set
  29. @rockbot #webrebels 10 function getPage (type) { 11 12 return

    function (name, next) { 13 14 var url = fmt('https://raw. 15 githubusercontent.com/npm/' + 16 type + '/master/%s.md', name); 17 https://raw.githubusercontent.com/npm/newww/master/policies.md https://raw.githubusercontent.com/npm/newww/master/../../ corporate_secrets/all_the_passwords.md user-set system-set
  30. @rockbot #webrebels happy path only

  31. @rockbot #webrebels 9 browse.packagesByKeyword = function(request, 10 reply) { ...

    20 21 Package(request.loggedInUser) 22 .list(options) 23 .then(function(result) { 24 context.items = chunk(result.results, 3); 25 paginate(request, options, result, 26 context); 27 return reply.view('browse/keyword', 28 context); 29 }); 30 }; 31
  32. @rockbot #webrebels 9 browse.packagesByKeyword = function(request, 10 reply) { ...

    20 21 Package(request.loggedInUser) 22 .list(options) 23 .then(function(result) { 24 context.items = chunk(result.results, 3); 25 paginate(request, options, result, 26 context); 27 return reply.view('browse/keyword', 28 context); 29 }).catch(err => reply(err)); 30 }; 31
  33. @rockbot #webrebels maintainability

  34. @rockbot #webrebels “spaghetti” code

  35. @rockbot #webrebels

  36. @rockbot #webrebels

  37. @rockbot #webrebels code with unknown side effects

  38. @rockbot #webrebels 65 const checkCache = getAccess( 66 this.authCacheUrl, 67

    name, 68 pkg, 69 creds.method 70 ).then(reply => { 71 if (reply.status !== 'not-found' || creds.method !== 'PUT') { 72 return reply 73 } 74 return createPackage(this.userAclUrl, this.billingApiUrl, name, 75 pkg, { 76 access: creds.body ? creds.body.access : null, 77 team: creds.team ? creds.body.team : null 78 }).then(() => { 79 // retry the cache! 80 return getAccess( 81 this.authCacheUrl, 82 name, 83 pkg, 84 creds.method 85 ) 86 })
  39. @rockbot #webrebels 65 const checkCache = getAccess( 66 this.authCacheUrl, 67

    name, 68 pkg, 69 creds.method 70 ).then(reply => { 71 if (reply.status !== 'not-found' || creds.method !== 'PUT') { 72 return reply 73 } 74 return createPackage(this.userAclUrl, this.billingApiUrl, name, 75 pkg, { 76 access: creds.body ? creds.body.access : null, 77 team: creds.team ? creds.body.team : null 78 }).then(() => { 79 // retry the cache! 80 return getAccess( 81 this.authCacheUrl, 82 name, 83 pkg, 84 creds.method 85 ) 86 })
  40. @rockbot #webrebels 65 const checkCache = getAccess( 66 this.authCacheUrl, 67

    name, 68 pkg, 69 creds.method 70 ).then(reply => { 71 if (reply.status !== 'not-found' || creds.method !== 'PUT') { 72 return reply 73 } 74 return createPackage(this.userAclUrl, this.billingApiUrl, name, 75 pkg, { 76 access: creds.body ? creds.body.access : null, 77 team: creds.team ? creds.body.team : null 78 }).then(() => { 79 // retry the cache! 80 return getAccess( 81 this.authCacheUrl, 82 name, 83 pkg, 84 creds.method 85 ) 86 })
  41. @rockbot #webrebels readability

  42. @rockbot #webrebels “clever” code

  43. @rockbot #webrebels 173 function transform (type, arg, data, skip, 174

    limit) { ... 187 // normally has an arg. sort, and then 188 manually paginate. 189 if (!arg && transformKeyArg[type]) { 190 data = data.sort(function (a, b) { 191 return a.value === b.value ? ( 192 a.name === b.name ? 0 : a.name < b.name 193 ? -1 : 1 194 ) : a.value > b.value ? -1 : 1 195 }).slice(skip, skip + limit) 196 } 197 198 return data 199 }
  44. @rockbot #webrebels 173 function transform (type, arg, data, skip, 174

    limit) { ... 187 // normally has an arg. sort, and then 188 manually paginate. 189 if (!arg && transformKeyArg[type]) { 190 data = data.sort(function (a, b) { 191 return a.value === b.value ? ( 192 a.name === b.name ? 0 : a.name < b.name 193 ? -1 : 1 194 ) : a.value > b.value ? -1 : 1 195 }).slice(skip, skip + limit) 196 } 197 198 return data 199 }
  45. @rockbot #webrebels too long for context

  46. @rockbot #webrebels

  47. @rockbot #webrebels

  48. @rockbot #webrebels doesn’t match style standards

  49. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 }
  50. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs!
  51. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs! 4 spaces!
  52. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs! 4 spaces! weird new-lined curly braces!
  53. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs! 4 spaces! semicolons! weird new-lined curly braces!
  54. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs! 4 spaces! hanging semicolons! semicolons! weird new-lined curly braces!
  55. @rockbot #webrebels 1 var 2 BORING_EMOJI = require('emoji-list'), 3 FS

    = require('fs'), 4 MARKOV = require('markoff'), 5 MARK = new MARKOV(), 6 SLACK = require('@slack/client'), 7 SLACK_EVENTS = SLACK.CLIENT_EVENTS.RTM, 8 RTM_EVENTS = SLACK.RTM_EVENTS, 9 LOG = console.log 10 ; 11 12 function ISLOUD(MSG) 13 { 14 return MSG !== MSG.toLowerCase() && MSG === 15 MSG.toUpperCase(); 16 } tabs! 4 spaces! hanging semicolons! semicolons! weird new-lined curly braces! STOP
  56. @rockbot #webrebels there’s nothing wrong with this code

  57. @rockbot #webrebels style is personal

  58. @rockbot #webrebels use a linter. get over it.

  59. @rockbot #webrebels how to minimize bad code

  60. @rockbot #webrebels common code smells

  61. @rockbot #webrebels many branching paths

  62. @rockbot #webrebels context-dependent understanding many branching paths

  63. @rockbot #webrebels context-dependent understanding weird/long variable names many branching paths

  64. @rockbot #webrebels “works on my machine”

  65. @rockbot #webrebels “not invented here” (NIH) syndrome “works on my

    machine”
  66. @rockbot #webrebels “not invented here” (NIH) syndrome know it will

    break “works on my machine”
  67. @rockbot #webrebels useful code cleansers

  68. @rockbot #webrebels pair programming

  69. @rockbot #webrebels commit early and often

  70. @rockbot #webrebels write tests

  71. @rockbot #webrebels use a linter

  72. @rockbot #webrebels code reviews

  73. @rockbot #webrebels holistic overview

  74. @rockbot #webrebels commit by commit holistic overview

  75. @rockbot #webrebels commit by commit holistic overview line by line

  76. @rockbot #webrebels when is bad code good ?

  77. @rockbot #webrebels the beauty and horror of code is you

    get to see how other people think — CJ Silverio (@ceejbot) CTO of npm, Inc
  78. @rockbot #webrebels don’t know it’s bad at the time

  79. @rockbot #webrebels learning opportunity

  80. @rockbot #webrebels “good enough”

  81. @rockbot #webrebels signal to refactor

  82. @rockbot #webrebels more than just code

  83. @rockbot #webrebels 999 // ----------------------------------------------------------- 1000 // apply a given

    rotation to all the fingers in the hand 1001 // ----------------------------------------------------------- 1002 function applyRotToAllFingers(fingers, origin_palm, rotAngles) 1003 { 1005 for (var f = 0; f < fingers.length; ++f) 1006 { 1007 var thisFinger = fingers[f]; 1008 1010 for (var p = 0; p < thisFinger.points3D.length; ++p) 1011 { 1013 var vip = RotVectorAboutOrigin(thisFinger.points3D[p], 1014 origin_palm, rotAngles, true); 1015 thisFinger.visPoints[p] = new Vector(vip[0], vip[1], vip[2]); 1016 } 1017 1020 var pt0 = thisFinger.visPoints[1].v; 1021 var pt1 = thisFinger.visPoints[2].v; 1022 vip = [pt1[0] - pt0[0], pt1[1] - pt0[1], pt1[2] - pt0[2]]; 1023 thisFinger.visPoints[1] = new Vector(pt0[0] + vip[0] / 3, pt0[1] 1024 + vip[1] / 3, pt0[2] + vip[2] / 3);
  84. @rockbot #webrebels 999 // ----------------------------------------------------------- 1000 // apply a given

    rotation to all the fingers in the hand 1001 // ----------------------------------------------------------- 1002 function applyRotToAllFingers(fingers, origin_palm, rotAngles) 1003 { 1005 for (var f = 0; f < fingers.length; ++f) 1006 { 1007 var thisFinger = fingers[f]; 1008 1010 for (var p = 0; p < thisFinger.points3D.length; ++p) 1011 { 1013 var vip = RotVectorAboutOrigin(thisFinger.points3D[p], 1014 origin_palm, rotAngles, true); 1015 thisFinger.visPoints[p] = new Vector(vip[0], vip[1], vip[2]); 1016 } 1017 1020 var pt0 = thisFinger.visPoints[1].v; 1021 var pt1 = thisFinger.visPoints[2].v; 1022 vip = [pt1[0] - pt0[0], pt1[1] - pt0[1], pt1[2] - pt0[2]]; 1023 thisFinger.visPoints[1] = new Vector(pt0[0] + vip[0] / 3, pt0[1] 1024 + vip[1] / 3, pt0[2] + vip[2] / 3); lol python-y
  85. @rockbot #webrebels 999 // ----------------------------------------------------------- 1000 // apply a given

    rotation to all the fingers in the hand 1001 // ----------------------------------------------------------- 1002 function applyRotToAllFingers(fingers, origin_palm, rotAngles) 1003 { 1005 for (var f = 0; f < fingers.length; ++f) 1006 { 1007 var thisFinger = fingers[f]; 1008 1010 for (var p = 0; p < thisFinger.points3D.length; ++p) 1011 { 1013 var vip = RotVectorAboutOrigin(thisFinger.points3D[p], 1014 origin_palm, rotAngles, true); 1015 thisFinger.visPoints[p] = new Vector(vip[0], vip[1], vip[2]); 1016 } 1017 1020 var pt0 = thisFinger.visPoints[1].v; 1021 var pt1 = thisFinger.visPoints[2].v; 1022 vip = [pt1[0] - pt0[0], pt1[1] - pt0[1], pt1[2] - pt0[2]]; 1023 thisFinger.visPoints[1] = new Vector(pt0[0] + vip[0] / 3, pt0[1] 1024 + vip[1] / 3, pt0[2] + vip[2] / 3); so C++ lol python-y
  86. @rockbot #webrebels 999 // ----------------------------------------------------------- 1000 // apply a given

    rotation to all the fingers in the hand 1001 // ----------------------------------------------------------- 1002 function applyRotToAllFingers(fingers, origin_palm, rotAngles) 1003 { 1005 for (var f = 0; f < fingers.length; ++f) 1006 { 1007 var thisFinger = fingers[f]; 1008 1010 for (var p = 0; p < thisFinger.points3D.length; ++p) 1011 { 1013 var vip = RotVectorAboutOrigin(thisFinger.points3D[p], 1014 origin_palm, rotAngles, true); 1015 thisFinger.visPoints[p] = new Vector(vip[0], vip[1], vip[2]); 1016 } 1017 1020 var pt0 = thisFinger.visPoints[1].v; 1021 var pt1 = thisFinger.visPoints[2].v; 1022 vip = [pt1[0] - pt0[0], pt1[1] - pt0[1], pt1[2] - pt0[2]]; 1023 thisFinger.visPoints[1] = new Vector(pt0[0] + vip[0] / 3, pt0[1] 1024 + vip[1] / 3, pt0[2] + vip[2] / 3); so C++ too-clever variable name lol python-y
  87. @rockbot #webrebels 999 // ----------------------------------------------------------- 1000 // apply a given

    rotation to all the fingers in the hand 1001 // ----------------------------------------------------------- 1002 function applyRotToAllFingers(fingers, origin_palm, rotAngles) 1003 { 1005 for (var f = 0; f < fingers.length; ++f) 1006 { 1007 var thisFinger = fingers[f]; 1008 1010 for (var p = 0; p < thisFinger.points3D.length; ++p) 1011 { 1013 var vip = RotVectorAboutOrigin(thisFinger.points3D[p], 1014 origin_palm, rotAngles, true); 1015 thisFinger.visPoints[p] = new Vector(vip[0], vip[1], vip[2]); 1016 } 1017 1020 var pt0 = thisFinger.visPoints[1].v; 1021 var pt1 = thisFinger.visPoints[2].v; 1022 vip = [pt1[0] - pt0[0], pt1[1] - pt0[1], pt1[2] - pt0[2]]; 1023 thisFinger.visPoints[1] = new Vector(pt0[0] + vip[0] / 3, pt0[1] 1024 + vip[1] / 3, pt0[2] + vip[2] / 3); context-specific so C++ too-clever variable name lol python-y
  88. @rockbot #webrebels

  89. @rockbot #webrebels

  90. @rockbot #webrebels bad code is part of our origin stories

  91. @rockbot #webrebels we’ve all done it

  92. @rockbot #webrebels change languages === start over* PSA:

  93. @rockbot #webrebels change languages === start over* PSA: * varies

    with experience
  94. @rockbot #webrebels what’s wrong with bad code?

  95. @rockbot #webrebels we mock bad code

  96. @rockbot #webrebels beginners are afraid to publish for fear of

    mocking
  97. @rockbot #webrebels unhealthy attachment to code

  98. @rockbot #webrebels source of shame

  99. @rockbot #webrebels code is ultimately zeros and ones FYI:

  100. @rockbot #webrebels code is ultimately zeros and ones FYI: BUT

  101. @rockbot #webrebels code is an act of communication to your

    colleagues and your future self FYI:
  102. @rockbot #webrebels writing good code is hard FYI:

  103. @rockbot #webrebels accept the bad code to become great

  104. @rockbot #webrebels Code http://rckbt.me/2014/10/code/

  105. @rockbot #webrebels Your code is not a reflection of you.

    It isn’t a reflection of your beliefs, your upbringing, or your ability to be a good person.
  106. @rockbot #webrebels Your code is, however, a reflection of your

    thinking process at the time that you wrote it.
  107. @rockbot #webrebels Given our innate ability to change our minds,

    consider other viewpoints, and play with new ideas, why do we hold our code so dear?
  108. @rockbot #webrebels Your code can change. Your code will change.

    Your code must change, if it’s ever going to get better.
  109. @rockbot #webrebels Stop marrying your code.

  110. @rockbot #webrebels The sooner you accept this, the happier you

    will be and the better programmer you will become.
  111. Raquel Vélez @rockbot http://rckbt.me raquel@ js.com