Coding Standards That Improve Quality

Coding Standards That Improve Quality

You’ve sat down to do a code review for a pull/merge request. How do you decide whether or not to accept the request? How do you agree amongst yourselves on that? And how to do train others to achieve a consistent level of quality for when they’re doing code reviews too?

In this talk, Stuart will show you a breakdown of PSR-2 - the PHP Community’s main official coding standard - and look at the parts of PSR-2 that directly affect code quality. From there, he’ll present a definition of what quality means, and how to go about writing your own in-house coding standards to achieve consistent quality across all your pull/merge requests.

Presented at Leeds PHP User Group, 20th March 2019.

2c1dc90ff7bf69097a151677624777d2?s=128

Stuart Herbert

March 20, 2019
Tweet

Transcript

  1. A presentation by @stuherbert
 for @GanbaroDigital Coding Standards That Improve

    Quality
  2. Industry veteran: architect, engineer, leader, manager, mentor F/OSS contributor since

    1994 Talking and writing about PHP since 2004 Chief Software Archaeologist Building Quality @GanbaroDigital About Stuart
  3. Follow me I do tweet a lot about non-tech stuff

    though :) @stuherbert
  4. @GanbaroDigital ?? ?? Do you follow any written coding standards?

  5. @GanbaroDigital ?? ?? What do you follow?

  6. @GanbaroDigital ?? ?? Have you written your own?

  7. @GanbaroDigital Let's Slay Some Dragons!

  8. @GanbaroDigital QA Teams

  9. @GanbaroDigital ?? ?? Do you have a QA team?

  10. @GanbaroDigital ?? ?? What do they do?

  11. @GanbaroDigital “ You don't pass code to a QA team

    to add quality.
  12. @GanbaroDigital A QA team is an audit function. It assesses

    the quality you've already delivered.
  13. @GanbaroDigital “ QA teams don't assess code quality. They assess

    outcome quality.
  14. @GanbaroDigital “ You can't outsource quality to a team that

    doesn't do construction.
  15. @GanbaroDigital Construction is done by developers.

  16. @GanbaroDigital Coding standards are virtual building regulations.

  17. @GanbaroDigital “Requirements define what is built. Coding standards (help) define

    how it is built.
  18. @GanbaroDigital Coding standards are part of your engineering manual.

  19. @GanbaroDigital But they don't always improve quality ...

  20. @GanbaroDigital PSR-2

  21. @GanbaroDigital https://www.php-fig.org/psr/psr-2/

  22. @GanbaroDigital A Quick Breakdown of PSR-2

  23. @GanbaroDigital ?? ?? How many requirements are in PSR-2?

  24. @GanbaroDigital 96 Requirements in PSR-2

  25. @GanbaroDigital • MUST • MUST NOT • REQUIRED • SHALL

    • SHALL NOT • SHOULD • SHOULD NOT • RECOMMENDED • MAY • OPTIONAL PSR-2 Keywords
  26. @GanbaroDigital PSR-2 Keywords Count 0 17.5 35 52.5 70 MUST

    MUST NOT SHOULD SHOULD NOT MAY
  27. @GanbaroDigital Some important caveats ...

  28. @GanbaroDigital PSR-2 Analysis Caveats • Stronger keywords (MUST) can appear

    wrapped inside weaker keywords (MAY) • PSR-2 contains duplicate requirements • Some PSR-2 requirements are enforced by the PHP runtime
  29. @GanbaroDigital PSR-2 Analysis Caveats • Stronger keywords (MUST) can appear

    wrapped inside weaker keywords (MAY) • PSR-2 contains duplicate requirements • Some PSR-2 requirements are enforced by the PHP runtime
  30. @GanbaroDigital PSR-2 Analysis Caveats • Stronger keywords (MUST) can appear

    wrapped inside weaker keywords (MAY) • PSR-2 contains duplicate requirements • Some PSR-2 requirements are enforced by the PHP runtime
  31. @GanbaroDigital PSR-2 And Addressing Quality

  32. @GanbaroDigital ?? ?? How much of PSR-2 directly prevents defects

    in your code?
  33. @GanbaroDigital PSR-2 Keywords Count 0 17.5 35 52.5 70 MUST

    MUST NOT SHOULD SHOULD NOT MAY
  34. @GanbaroDigital PSR-2 Requirements That Prevent Defects 0 17.5 35 52.5

    70 MUST MUST NOT SHOULD SHOULD NOT MAY
  35. @GanbaroDigital Defect-Preventing PSR-2 Bits 1. Visibility MUST be declared on

    all properties and methods (x3) 2. The closing ?> tag MUST be omitted from files containing only PHP. 3. There MUST be a comment such as // no break when fall- through is intentional in a non-empty case body
  36. @GanbaroDigital “ Only 5.2% of PSR-2 directly addresses quality.

  37. @GanbaroDigital What Is PSR-2?

  38. @GanbaroDigital ?? ?? What are the other 91 requirements in

    PSR-2 all about?
  39. @GanbaroDigital “ PSR-2 is a code formatting standard.

  40. @GanbaroDigital “ PSR-2 is a social standard.

  41. @GanbaroDigital Can consistent formatting improve quality? Yes and no.

  42. @GanbaroDigital It reduces the required reading age (so to speak).

    That can make code more accessible as average industry experience declines.
  43. @GanbaroDigital It's an example of the industry learning from the

    mistakes of Perl.
  44. @GanbaroDigital It also leads to reduced comprehension because of how

    we learn to read prose.
  45. @GanbaroDigital “ Keep using PSR-2 as your code layout standard.

    Stop using PSR-2 as your code quality standard.
  46. @GanbaroDigital PSR-2 Replacement

  47. @GanbaroDigital https://github.com/php-fig/fig-standards/blob/master/ proposed/extended-coding-style-guide.md

  48. @GanbaroDigital Is the upcoming PSR-2 replacement any different?

  49. @GanbaroDigital PSR-2 Keywords Count 0 22.5 45 67.5 90 MUST

    MUST NOT SHOULD SHOULD NOT MAY
  50. @GanbaroDigital Draft PSR-2 Replacement Keywords Count 0 22.5 45 67.5

    90 MUST MUST NOT SHOULD SHOULD NOT MAY
  51. @GanbaroDigital 138 Requirements in the current draft

  52. @GanbaroDigital PSR-2 Requirements That Prevent Defects 0 22.5 45 67.5

    90 MUST MUST NOT SHOULD SHOULD NOT MAY
  53. @GanbaroDigital Draft PSR-2 Requirements That Prevent Defects 0 22.5 45

    67.5 90 MUST MUST NOT SHOULD SHOULD NOT MAY
  54. @GanbaroDigital Defect-Preventing Draft Bits • The closing ?> tag MUST

    be omitted from files containing only PHP. • Visibility MUST be declared on all properties. • Visibility MUST be declared on all constants if your project PHP minimum version supports constant visibilities (PHP 7.1 or later). • Visibility MUST be declared on all methods. • The body of each structure MUST be enclosed by braces. • There MUST be a comment such as // no break when fall-through is intentional in a non-empty case body.
  55. @GanbaroDigital “ Only 4.3% of the current draft directly addresses

    quality.
  56. @GanbaroDigital The decline is down to better drafting.

  57. @GanbaroDigital As it stands today, the PSR-2 replacement is a

    better drafted PSR-2. It does not change the scope of the standard.
  58. @GanbaroDigital “ The PSR-2 replacement is not a quality standard.

  59. @GanbaroDigital ?? ?? Have I slain the twin dragons of

    QA teams and PSR-2?
  60. @GanbaroDigital Let's look at writing coding standards that do improve

    quality.
  61. @GanbaroDigital Writing A Coding Standard For Quality

  62. @GanbaroDigital There's a limit to what you can achieve through

    coding standards.
  63. @GanbaroDigital A Worked Example

  64. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  65. @GanbaroDigital Let's break this down.

  66. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  67. @GanbaroDigital “ If everything is top priority, nothing is top

    priority.
  68. @GanbaroDigital Establish a set of priorities

  69. @GanbaroDigital I Use Three Priority Levels • Key - essential

    items • Major - significant immediate benefit • Minor - consistency / stylistic / philosophical
  70. @GanbaroDigital Priorities do *not* define what is mandatory and what

    is not.
  71. @GanbaroDigital Everything is mandatory.

  72. @GanbaroDigital Priorities define where your time goes.

  73. @GanbaroDigital Where Time Goes ... • During development • In

    code reviews • On (avoiding!) rework
  74. @GanbaroDigital “ Zero-defect code is perfectly possible. You just can't

    afford it.
  75. @GanbaroDigital

  76. @GanbaroDigital In code reviews, everyone checks all Key criteria.

  77. @GanbaroDigital In code reviews, check Major criteria between you.

  78. @GanbaroDigital Minor priorities will need picking up at first. Over

    time, Minor should become a given.
  79. @GanbaroDigital If they don't, change your criteria or change your

    people.
  80. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  81. @GanbaroDigital Give every coding standard a unique ID

  82. @GanbaroDigital Unique IDs allow people to communicate more clearly.

  83. @GanbaroDigital IDs are immutable for all time

  84. @GanbaroDigital “ Coding standards are not immutable!

  85. @GanbaroDigital Coding Standards Evolve • Adding new standards • Retiring

    obsolete standards • Improving established standards
  86. @GanbaroDigital Immutable IDs prevent confusion as standards evolve.

  87. @GanbaroDigital If the meaning of the standard changes, treat it

    as a new standard, allocate a new ID.
  88. @GanbaroDigital Use a structured ID system. e.g. A.B.C A =

    priority B = category C = count within category
  89. @GanbaroDigital Structured IDs help you group standards together as they

    evolve.
  90. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  91. @GanbaroDigital Categories help us see relevant standards together

  92. @GanbaroDigital • API Design • Architecture • Audit Log •

    Authentication & Authorisation • Code Health • Common Attacks • Correctness • Documentation • Logical Layering • Persistent Data • Package Management • Naming of Things • Robustness • Session Management • Third Party Packages • Traceability • Unit Testing Example Categories
  93. @GanbaroDigital *not* an exhaustive list!

  94. @GanbaroDigital Categories are concepts / themes that run throughout the

    code.
  95. @GanbaroDigital • API Design • Architecture • Audit Log •

    Authentication & Authorisation • Code Health • Common Attacks • Correctness • Documentation • Logical Layering • Persistent Data • Package Management • Naming of Things • Robustness • Session Management • Third Party Packages • Traceability • Unit Testing Example Categories
  96. @GanbaroDigital We can review all the standards for a category

    and spot obvious gaps.
  97. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  98. @GanbaroDigital Criteria tells us what success looks like

  99. @GanbaroDigital Some Example Criteria • Capture all end-user activity in

    an audit log. • Always return a JSON object when an error occurs. • Never support a request body in a HTTP GET request.
  100. @GanbaroDigital I use a single sentence.

  101. @GanbaroDigital Some Example Criteria • Capture all end-user activity in

    an audit log. • Always return a JSON object when an error occurs. • Never support a request body in a HTTP GET request.
  102. @GanbaroDigital I use active voice.

  103. @GanbaroDigital Some Example Criteria • Capture all end-user activity in

    an audit log. • Always return a JSON object when an error occurs. • Never support a request body in a HTTP GET request.
  104. @GanbaroDigital I avoid ambiguity.

  105. @GanbaroDigital Some Example Criteria • Capture all end-user activity in

    an audit log. • Always return a JSON object when an error occurs. • Never support a request body in a HTTP GET request.
  106. @GanbaroDigital https://www.gov.uk/guidance/style-guide/a-to-z-of-gov-uk- style

  107. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  108. @GanbaroDigital Impacts remind us who/what we are doing this for

  109. @GanbaroDigital (but they're not really 'why' ...)

  110. @GanbaroDigital Impacted functions are a mixture of people and capabilities.

  111. @GanbaroDigital • End-User • The Business • Engineering • Governance

    • Regulation • Maintenance • Operations • Platform • Security • Testability Example Impacted Functions
  112. @GanbaroDigital *not* an exhaustive list!

  113. @GanbaroDigital Identify your impacted functions

  114. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  115. @GanbaroDigital Description tells you what to do or what not

    to do
  116. @GanbaroDigital I use as many sentences as required.

  117. @GanbaroDigital I use active voice.

  118. @GanbaroDigital I avoid ambiguity.

  119. @GanbaroDigital Priority Key ID 1.1.1 Category 3-Tier Architecture Criteria Do

    not trust higher levels of the architecture. Impacts Security Description Do not assume that any input data is safe. Validate all input data before using it.
  120. @GanbaroDigital You can go beyond this minimum if time /

    budget allows.
  121. @GanbaroDigital Extra Points For ... • Examples • Documenting the

    'why'
  122. @GanbaroDigital ?? ?? Is the 'why' optional or mandatory in

    a coding standard?
  123. @GanbaroDigital General Guidance

  124. @GanbaroDigital When you look back at your coding standard, is

    it SMART?
  125. @GanbaroDigital SMART (in this case!) is ... • Specific •

    Measurable • Actionable • Relevant • Timely
  126. @GanbaroDigital When you look back at your coding standard, is

    it clear?
  127. @GanbaroDigital Be Clear! • What the standard is • Where

    the standard needs applying • Who it impacts • What problem it prevents (the Why) • How important it is to prevent the problem
  128. @GanbaroDigital When you look back at your coding standard, is

    it necessary?
  129. @GanbaroDigital Is It Necessary? • Does it prevent real or

    imaginary problems? • Is it built on (bitter) experience? • Or is it just there to control people?
  130. @GanbaroDigital Do you have buy-in?

  131. @GanbaroDigital Do You Have Buy-In? • Are developers actively maintaining

    the coding standard? • Are they actively following it? • Or is it simply imposed by the boss?
  132. @GanbaroDigital “ You cannot fix people problems with technology or

    standards.
  133. @GanbaroDigital

  134. Thank You How Can We Help You? A presentation by

    @stuherbert
 for @GanbaroDigital