The Beauty of Bad Code

The Beauty of Bad Code

Opening keynote at nz.js(con); in Wellington, NZ on 9 March 2017

583a0cfd3e0ec851166c5c6fa5e506a5?s=128

Raquel Vélez

March 09, 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 #nzjscon we’ve written a lot of code

  5. @rockbot #nzjscon we’ve rewritten our code

  6. @rockbot #nzjscon we’ve rewritten our code several times

  7. @rockbot #nzjscon we know bad code

  8. @rockbot #nzjscon courtesy of npm wombats what is bad code?

  9. @rockbot #nzjscon functionality

  10. @rockbot #nzjscon functionality maintainability

  11. @rockbot #nzjscon functionality maintainability readability

  12. @rockbot #nzjscon functionality

  13. @rockbot #nzjscon super-villain code

  14. @rockbot #nzjscon 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 */
  15. @rockbot #nzjscon doesn’t do the thing it’s supposed to do

  16. @rockbot #nzjscon 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
  17. @rockbot #nzjscon 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
  18. @rockbot #nzjscon 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 #nzjscon 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 #nzjscon security risks

  21. @rockbot #nzjscon 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 }
  22. @rockbot #nzjscon 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 }
  23. @rockbot #nzjscon 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
  24. @rockbot #nzjscon 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
  25. @rockbot #nzjscon 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
  26. @rockbot #nzjscon 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
  27. @rockbot #nzjscon 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
  28. @rockbot #nzjscon happy path only

  29. @rockbot #nzjscon 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
  30. @rockbot #nzjscon 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
  31. @rockbot #nzjscon maintainability

  32. @rockbot #nzjscon “spaghetti” code

  33. @rockbot #nzjscon

  34. @rockbot #nzjscon

  35. @rockbot #nzjscon code with unknown side effects

  36. @rockbot #nzjscon 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 })
  37. @rockbot #nzjscon 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 })
  38. @rockbot #nzjscon 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 #nzjscon readability

  40. @rockbot #nzjscon “clever” code

  41. @rockbot #nzjscon 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 }
  42. @rockbot #nzjscon 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 }
  43. @rockbot #nzjscon too long for context

  44. @rockbot #nzjscon

  45. @rockbot #nzjscon

  46. @rockbot #nzjscon doesn’t match style standards

  47. @rockbot #nzjscon 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 }
  48. @rockbot #nzjscon 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!
  49. @rockbot #nzjscon 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!
  50. @rockbot #nzjscon 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!
  51. @rockbot #nzjscon 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!
  52. @rockbot #nzjscon 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!
  53. @rockbot #nzjscon 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
  54. @rockbot #nzjscon there’s nothing wrong with this code

  55. @rockbot #nzjscon style is personal

  56. @rockbot #nzjscon use a linter. get over it.

  57. @rockbot #nzjscon how to minimize bad code

  58. @rockbot #nzjscon common code smells

  59. @rockbot #nzjscon many branching paths

  60. @rockbot #nzjscon context-dependent understanding many branching paths

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

  62. @rockbot #nzjscon “works on my machine”

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

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

    break “works on my machine”
  65. @rockbot #nzjscon pair programming

  66. @rockbot #nzjscon commit early and often

  67. @rockbot #nzjscon write tests

  68. @rockbot #nzjscon use a linter

  69. @rockbot #nzjscon code reviews

  70. @rockbot #nzjscon holistic overview

  71. @rockbot #nzjscon commit by commit holistic overview

  72. @rockbot #nzjscon commit by commit holistic overview line by line

  73. @rockbot #nzjscon when is bad code good ?

  74. @rockbot #nzjscon the beauty and horror of code is you

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

  76. @rockbot #nzjscon learning opportunity

  77. @rockbot #nzjscon “good enough”

  78. @rockbot #nzjscon signal to refactor

  79. @rockbot #nzjscon more than just code

  80. @rockbot #nzjscon 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);
  81. @rockbot #nzjscon

  82. @rockbot #nzjscon

  83. @rockbot #nzjscon bad code is part of our origin stories

  84. @rockbot #nzjscon we’ve all done it

  85. @rockbot #nzjscon change languages === start over* PSA:

  86. @rockbot #nzjscon change languages === start over* PSA: * varies

    with experience
  87. @rockbot #nzjscon what’s wrong with bad code?

  88. @rockbot #nzjscon we mock bad code

  89. @rockbot #nzjscon beginners are afraid to publish for fear of

    mocking
  90. @rockbot #nzjscon unhealthy attachment to code

  91. @rockbot #nzjscon source of shame

  92. @rockbot #nzjscon code is ultimately zeros and ones FYI:

  93. @rockbot #nzjscon code is an act of communication to your

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

  95. @rockbot #nzjscon accept the bad code to become great

  96. @rockbot #nzjscon Code http://rckbt.me/2014/10/code/

  97. @rockbot #nzjscon 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.
  98. @rockbot #nzjscon Your code is, however, a reflection of your

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

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

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

  102. @rockbot #nzjscon The sooner you accept this, the happier you

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