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.

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