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

The Beauty of Bad Code

The Beauty of Bad Code

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

Raquel Vélez

March 09, 2017
Tweet

More Decks by Raquel Vélez

Other Decks in Programming

Transcript

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

    View full-size slide

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

    View full-size slide

  3. @rockbot #jsnz
    npm is a
    package manager

    View full-size slide

  4. @rockbot #nzjscon
    we’ve written a lot of code

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  7. @rockbot #nzjscon
    we know bad code

    View full-size slide

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

    View full-size slide

  9. @rockbot #nzjscon
    functionality

    View full-size slide

  10. @rockbot #nzjscon
    functionality
    maintainability

    View full-size slide

  11. @rockbot #nzjscon
    functionality
    maintainability
    readability

    View full-size slide

  12. @rockbot #nzjscon
    functionality

    View full-size slide

  13. @rockbot #nzjscon
    super-villain code

    View full-size slide

  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 */

    View full-size slide

  15. @rockbot #nzjscon
    doesn’t do the thing
    it’s supposed to do

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  20. @rockbot #nzjscon
    security risks

    View full-size slide

  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 }

    View full-size slide

  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 }

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  28. @rockbot #nzjscon
    happy path only

    View full-size slide

  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

    View full-size slide

  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

    View full-size slide

  31. @rockbot #nzjscon
    maintainability

    View full-size slide

  32. @rockbot #nzjscon
    “spaghetti” code

    View full-size slide

  33. @rockbot #nzjscon

    View full-size slide

  34. @rockbot #nzjscon

    View full-size slide

  35. @rockbot #nzjscon
    code with unknown
    side effects

    View full-size slide

  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 })

    View full-size slide

  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 })

    View full-size slide

  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 })

    View full-size slide

  39. @rockbot #nzjscon
    readability

    View full-size slide

  40. @rockbot #nzjscon
    “clever” code

    View full-size slide

  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 }

    View full-size slide

  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 }

    View full-size slide

  43. @rockbot #nzjscon
    too long for context

    View full-size slide

  44. @rockbot #nzjscon

    View full-size slide

  45. @rockbot #nzjscon

    View full-size slide

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

    View full-size slide

  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 }

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  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!

    View full-size slide

  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

    View full-size slide

  54. @rockbot #nzjscon
    there’s nothing wrong
    with this code

    View full-size slide

  55. @rockbot #nzjscon
    style is personal

    View full-size slide

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

    View full-size slide

  57. @rockbot #nzjscon
    how to minimize
    bad code

    View full-size slide

  58. @rockbot #nzjscon
    common code smells

    View full-size slide

  59. @rockbot #nzjscon
    many branching paths

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  65. @rockbot #nzjscon
    pair programming

    View full-size slide

  66. @rockbot #nzjscon
    commit early and often

    View full-size slide

  67. @rockbot #nzjscon
    write tests

    View full-size slide

  68. @rockbot #nzjscon
    use a linter

    View full-size slide

  69. @rockbot #nzjscon
    code reviews

    View full-size slide

  70. @rockbot #nzjscon
    holistic overview

    View full-size slide

  71. @rockbot #nzjscon
    commit by commit
    holistic overview

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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

    View full-size slide

  75. @rockbot #nzjscon
    don’t know it’s bad at the time

    View full-size slide

  76. @rockbot #nzjscon
    learning opportunity

    View full-size slide

  77. @rockbot #nzjscon
    “good enough”

    View full-size slide

  78. @rockbot #nzjscon
    signal to refactor

    View full-size slide

  79. @rockbot #nzjscon
    more than just code

    View full-size slide

  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);

    View full-size slide

  81. @rockbot #nzjscon

    View full-size slide

  82. @rockbot #nzjscon

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  88. @rockbot #nzjscon
    we mock bad code

    View full-size slide

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

    View full-size slide

  90. @rockbot #nzjscon
    unhealthy attachment to code

    View full-size slide

  91. @rockbot #nzjscon
    source of shame

    View full-size slide

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

    View full-size slide

  93. @rockbot #nzjscon
    code is an act of
    communication to your
    colleagues and your future self
    FYI:

    View full-size slide

  94. @rockbot #nzjscon
    writing good code is hard
    FYI:

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  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.

    View full-size slide

  98. @rockbot #nzjscon
    Your code is, however, a reflection of
    your thinking process at the time that
    you wrote it.

    View full-size slide

  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?

    View full-size slide

  100. @rockbot #nzjscon
    Your code can change.
    Your code will change.
    Your code must change, if it’s ever
    going to get better.

    View full-size slide

  101. @rockbot #nzjscon
    Stop marrying your code.

    View full-size slide

  102. @rockbot #nzjscon
    The sooner you accept this, the
    happier you will be and the better
    programmer you will become.

    View full-size slide

  103. Raquel Vélez
    @rockbot
    http://rckbt.me
    raquel@ js.com

    View full-size slide