diff --git a/collects/scribblings/style/correct-maintain-speed.scrbl b/collects/scribblings/style/correct-maintain-speed.scrbl index 0ac439852d..ffaa5ad626 100644 --- a/collects/scribblings/style/correct-maintain-speed.scrbl +++ b/collects/scribblings/style/correct-maintain-speed.scrbl @@ -2,6 +2,9 @@ @title[#:tag "correct-maintain-speed"]{Basic Facts of Life} +@nested[#:style 'inset]{ Favor readers over writers. + --- Yaron Minsky, JaneStreet, 2010 at Northeastern} + Write code that is correct; maintainable; and fast. The ordering of this adjectives is critical: correct is more important than maintainable; maintainable is more important than fast; and fast is important to include, @@ -13,10 +16,119 @@ Code is maintainable. Code is fast. -This guide is about creating maintainable code and it touches on -correctness and speed on occasion. For the former, formulate test suites; -for the latter, start with stress test suites. +The purpose of this guide is to spell out suggestions that help with these +three points. Specifically, it spells out suggestions @emph{that you can +check} and that everyone else can check. -If the above is too many words, remember this slogan: -@nested[#:style 'inset]{ Favor readers over writers. - --- Yaron Minsky, JaneStreet, 2010 at Northeastern} +@; ----------------------------------------------------------------------------- +@section{Correctness} + +@nested[#:style 'inset]{I have bug reports, therefore I exist. -- Matthias, +watching Matthew, Robby, Shriram and others create the original code base} + +@nested[#:style 'inset]{It is the way we choose to fight our bugs that + determines our character, not their presence or absence. -- Robby, in response} + +Correctness is a perfectionist goal beyond the reach of PLT. All software + has mistakes. If they are unknown, the software isn't being used. The goal + is, however, not to ensure some basic level of correctness before a + feature is released and to ensure that the same mistake isn't introduced + again. + +Formulate test suites. Use unit testing. Use random testing. Use fuzz + testing. Test! + +Run the test suites before you commit. Read drdr's emails; don't ignore + them. + +Add tests to test suites during debugging. That is, first, write an + automated test that exposes the mistake in the existing + implementation. Put this in the software's test suite so it will never be + accidentally introduced again. Second, modify the code to fix the + mistake. Do this second to be sure you didn't introduce a mistake in your + tests; it is all too easy to think you've fixed a mistake when in reality + your new test just doesn't properly reveal the old mistake. Third, re-run + the test suite to ensure that the mistake is fixed and no existing tests + fail. + +Create test suites. Editing code without an existing test suite is like + flying blind. If there is no existing test suite, you have no idea + whether changes are introducing any new mistakes or breaking intended + functionality. Make a reasonable effort to put in a test case or two for + the specific functionality that you're adding or modifying. If there is + no test suite and you aren't sure how to build one, then ask, see what + responses you get, and go from there. In the special case that you found + a mistake and are fixing it, there should be a way to observe that mistake + and you should be able to turn that into a test case. If you cannot, you + have a few options: + + @itemlist[#:style 'ordered + + @item{Add an end-to-end test that may have to be verified by a human. For + example, it might be hard to test Slideshow, so you could create a slide + set and describe what it should look like so future maintainers to + verify when @emph{they} make changes.} + + @item{Add functionality to the library to expose properties that reveal the + bug. For example, you may be able to add a few accessors to Slideshow + slides to enable an automated test suite.} +] + As we slowly increase the number of tests across the system, this problem + will go away for future generations. + +@; ----------------------------------------------------------------------------- +@section{Maintenance} + +Comprehensible code is maintainable. + +Code is comprehensible when you can understand its external purpose. To + this end, code must come with external documentation. Released code must + have documentation. A change to the external behavior of code must induce + an immediate change to its documentation. + +In order to document code, refer to the + @hyperlink["http://docs.racket-lang.org/scribble/how-to-doc.html#%28part._reference-style%29"]{style + guide} in the Scribble manual. Ideally documentation comes in two parts: + a "Guide" section, which explains the purpose and suggests use cases, and + a traditional "Reference" section, which presents the minutae. Also + consider adding examples to each function in your "Reference" section. + Finally, ensure you have all the correct @tt{for-label} @tt{require}s + and make use of other useful cross-references. + +Code comprehension also requires adherence to basic elements of style and + some internal documentation. The rest of this document is about these + elements of style, including some suggestions on internal documentation. + +@; ----------------------------------------------------------------------------- +@section{Speed} + +Making code fast is an endless task. + +Making code @emph{reasonably} fast is the goal. + +It is especially the goal for all pieces of the code base that are reused +elsewhere. + +Just as for correctness, strive for basic tests, that is, tests that + exercise your code on reasonably large inputs. While a regular test suite + for a universe display deal with a 50 x 50 display window, the stress test + suite should check whether universe event handlers and drawing routines + can cope with laptop size displays or even a 30in display. Or, if you were + to write a library for a queue data structure, a regular test suite + ensures that it deals correctly with enqueue and dequeue for small + queues, including empty ones; a stress test suite for the same library + would run the queue operations on a variety of queue sizes, including very + large queues of say 10,000 elements. + +Stress tests don't normally have an expected output, so they never + "pass". The practice of writing stress tests exposes implementation flaws + or provides comparative data to be used when choosing between two + APIs. Just writing them and keeping them around reminds us that things can + go bad and we can detect when performance degrades through some other + door. Most importantly, a stress test suite may reveal that your code + isn't implementing an algorithm with the expected O(.) running + time. Finding out that much alone is useful. If you can't think of an + improvement, just document the weakness in the external library and move + on. + +We are not perfectionist. We produce reasonable software. diff --git a/collects/scribblings/style/shared.rkt b/collects/scribblings/style/shared.rkt index 242910cff8..c1aa66485f 100644 --- a/collects/scribblings/style/shared.rkt +++ b/collects/scribblings/style/shared.rkt @@ -7,3 +7,6 @@ (provide (for-label (all-from-out racket)) (all-from-out scribble/manual)) + + +; (provide good) diff --git a/collects/scribblings/style/textual.scrbl b/collects/scribblings/style/textual.scrbl index d3dbf41a23..8d8900f451 100644 --- a/collects/scribblings/style/textual.scrbl +++ b/collects/scribblings/style/textual.scrbl @@ -2,54 +2,106 @@ @(require "shared.rkt") +@(define-syntax-rule + (good form code ...) + (racketmod #:file (tt "good") racket form code ...)) + @title{Textual Matters} +Simple textual conventions help eyes find pieces of code quickly. Here are +some of those that are easy to check---some automatically and some +manually. If you find yourself editing a file that violates some of the +constraints below, edit it into the proper shape. + @; ----------------------------------------------------------------------------- @section{Indentation} -DrRacket indents code. Use it. If you wish to be a real friend of PLT, use -DrRacket all the time, and do request features and changes for things you -don't like. +DrRacket indents code and it is the only tool that everyone in PLT agrees +on. So use DrRacket's indentation style. Here is what this means. +@nested[#:style 'inset]{ + For every file in the repository, DrRacket's "indent all" functions leaves + the file alone.} +That's all there is to it. -Real friends of PLT use DrRacket to write their programs, and they don't -override DrRacket's indentation style. They accept it and leave it alone. -The resulting uniformity helps guide eyes. +If you prefer to use some other editor (emacs, vi/m, etc), program it so +that it follows DrRacket's indentation style. -@margin-note{Okay, okay. We are using emacs to write this guide.} Minor -friends of PLT use Emacs and/or vi(m). Nevertheless, these minor friends -adjust their chosen editor so that it follows DrRacket's way of indenting -code. +Examples: -One of the most important disagreements may concern @scheme[if]. So once -and for all: - -@(racketmod +@racketmod[#:file +@tt{good} racket -#:file good + (if (positive? x) (send rocket-object launch) (redirect (- x))) -) +] -@(racketmod +@racketmod[#:file +@tt{bad} racket -#:file bad + (if (positive? x) (send rocket-object launch) (redirect (- x))) -) +] -Also note that the then- and else-branches are separate entities, and each -entity deserves at least one line. +@margin-note{we need more of these rules} + +@; ----------------------------------------------------------------------------- +@section{Line Breaks} + +Next to indentation, proper line breaks are critical. + +For an @scheme[if] expression, put each alternative on a separate line. + +@racketmod[#:file +@tt{good} +racket + +(if (positive? x) + (send rocket-object launch) + (redirect (- x))) +] + +@racketmod[#:file +@tt{bad} +racket + +(if (positive? x) (send rocket-object launch) + (redirect (- x))) +] + +Each definition and each local definition deserves at least one line. + +@racketmod[#:file +@tt{good} +racket + +(define (start-reactor x) + (define width (* 10 x)) + (define height (* 3 x)) + ...) +] + +@racketmod[#:file +@tt{bad} +racket + +(define (start-reactor x) + (define width (* 10 x)) (define height (* 3 x)) + ...) +] +@margin-note{we need more of these rules} @; ----------------------------------------------------------------------------- @section{Line Width} -A line in Racket is at most 100 characters wide. +A line in a Racket file is at most 102 characters wide. -If you use Emacs to edit, create a line with ";; " followed by ctrl-U 77 -and "-". Okay, use DrRacket to create such lines by holding down the dash -key. +When you create a file, add a line with ";; " followed by ctrl-U 99 and "-". +When you separate "sections" of code in a file, insert the same line. This +provides some line-width orientation in the middle of a file, too. @; ----------------------------------------------------------------------------- @section{Where to Put Parentheses}