From bceca4b22f8973e46a8aee7fc299714accf5a319 Mon Sep 17 00:00:00 2001 From: Matthias Felleisen Date: Sun, 20 Feb 2011 14:32:31 -0500 Subject: [PATCH] [Style] @compare plus some rules on use of constructs --- collects/scribblings/style/constructs.scrbl | 73 ++++++++++++++++++ .../style/correct-maintain-speed.scrbl | 75 ++++++++++--------- collects/scribblings/style/shared.rkt | 19 ++++- .../scribblings/style/some-performance.scrbl | 28 +++++-- collects/scribblings/style/style.scrbl | 57 +++++++++----- collects/scribblings/style/textual.scrbl | 48 ++++++------ 6 files changed, 216 insertions(+), 84 deletions(-) create mode 100644 collects/scribblings/style/constructs.scrbl diff --git a/collects/scribblings/style/constructs.scrbl b/collects/scribblings/style/constructs.scrbl new file mode 100644 index 0000000000..bf149cc2d7 --- /dev/null +++ b/collects/scribblings/style/constructs.scrbl @@ -0,0 +1,73 @@ +#lang scribble/base + +@(require "shared.rkt") + +@title{Choosing the Right Construct} + +Racket provides a range of constructs for the same or similar purposes. +Although the Racket designers don't think that there is one right way for +everything, we prefer certain constructs in certain situations for consistency +and readability. + +@; ----------------------------------------------------------------------------- +@section{Definitions} + +Here are a few of Racket's definitional constructs: @scheme[let], @scheme[let*], +@scheme[letrec], and @scheme[define]. Except for the last one, all others force +an increase to the indentation level. We therefore request that you favor +@scheme[define] over all other features when feasible. + +@compare[ +@racketmod[#:file +@tt{good} +racket + +(define (swap x y) + (define t (unbox x)) + (set-box! x (unbox y)) + (set-box! y t)) +] +@; ----------------------------------------------------------------------------- +@racketmod[#:file +@tt{bad} +racket + +(define (swap x y) + (let ([t (unbox x)]) + (set-box! x (unbox y)) + (set-box! y t))) +] +] + +@; ----------------------------------------------------------------------------- +@section{Conditionals} + +Like definitional constructs, conditionals come in many flavors, too. Because +@scheme[cond] and its relatives now allow local uses of @scheme[define], you +should prefer them over @scheme[if]. + +@compare[ +@racketmod[#:file +@tt{good} +racket + +(cond + [(empty? l) true] + [else + (define fst (first l)) + (define rst (rest l)) + (and (flat-rate fst) + (curved fst (chk rst)))]) +] +@racketmod[#:file +@tt{bad} +racket + +(if (empty? l) + true + (let ([fst (first l)] + [rst (rest l)]) + (and (flat-rate fst) + (curved fst (chk rst))))) +] +] diff --git a/collects/scribblings/style/correct-maintain-speed.scrbl b/collects/scribblings/style/correct-maintain-speed.scrbl index 1cdbb351b7..9fa43d19d0 100644 --- a/collects/scribblings/style/correct-maintain-speed.scrbl +++ b/collects/scribblings/style/correct-maintain-speed.scrbl @@ -1,5 +1,7 @@ #lang scribble/base +@(require "shared.rkt") + @title[#:tag "correct-maintain-speed"]{Basic Facts of Life} @nested[#:style 'inset]{ Favor readers over writers. @@ -29,11 +31,11 @@ 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, to ensure some basic level of correctness before a - feature is released and to ensure that the same mistake isn't introduced - again. +Complete 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, 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! @@ -41,15 +43,13 @@ Formulate test suites. Use unit testing. Use random testing. Use fuzz 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. +When you debug, formulate a test case first. Put it into the test suite for + the component so the mistake will never be accidentally re-introduced. + 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 @@ -62,7 +62,7 @@ Create test suites. Editing code without an existing test suite is like and you should be able to turn that into a test case. If you cannot, you have a few options: - @itemlist[#:style 'ordered +@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 @@ -84,16 +84,18 @@ 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. + a simultaneous change to its documentation---"simultaneous" means that the + two changes are in the same commit to the code base. 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. + consider adding examples for each function and construct 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 mostly about @@ -108,29 +110,34 @@ 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. + elsewhere. Write them using @racketmod[racket] so that they don't affect + the load-time for scripts. See the next section. -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 deals with a 50 x 50 display window, the stress test - suite should check whether Universe event handlers and drawing routines +As with correctness, performance demands some "testing". At a minimum, + exercise your code on some reasonably large inputs. Add a file to the test + suite that runs large inputs regularly. For example, a regular test suite + for a Universe display deals with a 50 x 50 display window; one of its + stress tests checks 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. + 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. + door. Most importantly, a stress test 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 perfectionists. We produce reasonable software. +And as you read on, keep in mind that we are not perfectionists. We produce + reasonable software. +@nested[#:style 'inset]{When you fix a bug, make sure to commit (1) the + code delta, (2) the new test case, and (3) the revised docs (if + applicable) in one batch.} diff --git a/collects/scribblings/style/shared.rkt b/collects/scribblings/style/shared.rkt index c1aa66485f..29a2ba8eae 100644 --- a/collects/scribblings/style/shared.rkt +++ b/collects/scribblings/style/shared.rkt @@ -3,10 +3,25 @@ ; things to be shared among all sections of the style guide (require (for-label racket) - scribble/manual) + scribble/manual + scribble/struct + (only-in scribble/core table-columns style) + scribble/html-properties + racket/list) (provide (for-label (all-from-out racket)) (all-from-out scribble/manual)) +(provide compare) -; (provide good) +;; compare: two elements, + +(define (compare stuff1 stuff2) + (define stuff (list (list stuff1) (list stuff2))) + (define space (style #f (list (attributes '((width . "500") (valign . "top")))))) + (table + (style #f + (list + (attributes '((border . "1") (cellpadding . "10"))) + (table-columns (make-list (length stuff) space)))) + (apply map (compose make-flow list) stuff))) diff --git a/collects/scribblings/style/some-performance.scrbl b/collects/scribblings/style/some-performance.scrbl index dd259b6063..2f3b791e7f 100644 --- a/collects/scribblings/style/some-performance.scrbl +++ b/collects/scribblings/style/some-performance.scrbl @@ -3,15 +3,27 @@ @(require "shared.rkt") @; ----------------------------------------------------------------------------- -@(define rurl "http://docs.racket-lang.org/reference/index.html?q=racket/base") -@(define (rkt) @hyperlink[rurl]{racket}) -@(define (rkt/base) @hyperlink[rurl]{racket/base}) +@(define (rurl x) (format "http://docs.racket-lang.org/reference/index.html?q=racket/~a" x)) +@(define (rkt) @hyperlink[(rurl "")]{racket}) +@(define (rkt/base) @hyperlink[(rurl "base")]{racket/base}) +@(define (rkt/gui) @hyperlink[(rurl "gui")]{racket/gui}) @title{Some Performance Hints} -Use @rkt/base[] instead of @rkt[] for any library that others may use - eventually. For all other modules, use @rkt[]. +When you write a module, you first pick a language. In Racket you can + choose a lot of languages. The most important choice concerns @rkt/base[] + vs @rkt[]. + +If you are writing a script, try to use @rkt/base[]. The @rkt/base[] + language loads significantly faster than the @rkt[] language because it is + significantly smaller than the @rkt[]. + +If your module is intended as a library, stick to @rkt/base[]. That way + script writers can use it without incurring the overhead of loading all of + @rkt[] unknowingly. + +Conversely, you should use @rkt[] (or even @rkt/gui[]) when you just want a + convenient language to write some program. The @rkt[] language comes with + almost all the batteries, and @rkt/gui[] adds the rest of the GUI base. + -The @rkt/base[] language loads significantly faster than the @rkt[] -language and is also significnatly smaller. Conversely, it is much more -convenient to program with @rkt[] than @rkt/base[]. diff --git a/collects/scribblings/style/style.scrbl b/collects/scribblings/style/style.scrbl index e82c3754d9..11a83f2b2e 100644 --- a/collects/scribblings/style/style.scrbl +++ b/collects/scribblings/style/style.scrbl @@ -8,31 +8,50 @@ @; ----------------------------------------------------------------------------- Since 1995 PLT has grown from a handful of people who worked on/with the - repository to three dozen and more. In addition, Racket is an open source - project, meaning other people study the code in the repository and use it - as an implicit guide to Racket programming. To manage the growth of the PLT - developer basis and to showcase good Racket coding, every contribution - should satisfy certain basic criteria. + repository to three dozen and more. This growth naturally implies a lot + of learning on our side and the introduction of inconsistencies. It is + time to leverage the former and to start eliminating the latter. Doing + so will help us, the developers, and our users, who use the open source + code in our repository as an implicit guide to Racket programming. -This document spells out these criteria, and it is also a call for - improvements and suggestions for additional criteria. Like code, this - document lives and benefits from the "many pairs of eyes" effect of open - source. Code should be viewed by more than one person; a second person is - likely to catch mistakes and problems and hidden bugs. This document is - meta-code, and the same ideas apply. If you have suggestions, contact the - authors via email. +To manage the growth of PLT and to showcase good Racket coding, we need + rules that govern the contributions to the code base. This document + spells out some basic criteria. They cover a range of topics, from basic + work (commit) habits to small syntactic ideas like indentations and + naming. The major goal is to achieve some level of consistency across + the different portions of the code base so that everyone who opens files + should easily find his way around. -@bold{Note} We understand that some of the files in the code base do not - live up to these standards. Help us improve these files. If you need to - edit and understand a file that fails in some ways, take the time to - reorganize it properly as soon as the comprehension step takes longer than - a few minutes. Because if understanding takes a lot of time, it is likely - that the file isn't maintainable. Whoever touches the file next will be - grateful. +Many pieces of the code base don't live up to our suggestions yet. Here + is how we get started. We encourage everyone to look over the commit + messages. If you see problems with the code deltas, let the committer + know. If you see a bug fix without docs and tests, let the committer + know. Code should be viewed by more than one person because a second + person is likely to catch logical mistakes, performance problems, and + unintended effects. In the past Eli has done a great job catching + problems; now everyone is asked to do so. + +Also, help us improve the existing files. If you need to edit and + understand an imperfect file, take the time to fix some of it as soon as + comprehending the file takes longer than a few minutes. After all, if + the inconsistencies throw you off for that much time, others are likely + to have the same problems. If you help fixing it, we reduce future + maintenance time. In other words, whoever touches the file next will be + grateful to you. + +@bold{Request} This document isn't complete and it isn't perfect. In other + words, it is also a call for improvements and suggestions. If you have + ideas, contact the first author via email. + +@bold{Note} If the style guide doesn't suit your personal style because +you grew up on something different, fine. Use your @emph{personal style} +for your @emph{personal programs}, but do use this style guide when you +create code for the PLT code base. @; ----------------------------------------------------------------------------- @include-section["correct-maintain-speed.scrbl"] @include-section["some-performance.scrbl"] @include-section["size.scrbl"] +@include-section["constructs.scrbl"] @include-section["textual.scrbl"] diff --git a/collects/scribblings/style/textual.scrbl b/collects/scribblings/style/textual.scrbl index 859aa54459..5948624765 100644 --- a/collects/scribblings/style/textual.scrbl +++ b/collects/scribblings/style/textual.scrbl @@ -33,22 +33,24 @@ that it follows DrRacket's indentation style. Examples: -@racketmod[#:file -@tt{good} -racket +@compare[ + @racketmod[#:file + @tt{good} + racket -(if (positive? x) - (send rocket-object launch) - (redirect (- x))) -] + (if (positive? (rocket-x r)) + (launch r) + (redirect (- x))) + ] -@racketmod[#:file -@tt{bad} -racket + @racketmod[#:file + @tt{bad} + racket -(if (positive? x) - (send rocket-object launch) - (redirect (- x))) + (if (positive? (rocket-x r)) + (launch r) + (redirect (- x))) + ] ] @margin-note{We need more of these rules} @@ -60,32 +62,35 @@ Next to indentation, proper line breaks are critical. For an @scheme[if] expression, put each alternative on a separate line. +@compare[ @racketmod[#:file @tt{good} racket (if (positive? x) - (send rocket-object launch) + (launch r) (redirect (- x))) -] + ] @racketmod[#:file @tt{bad} racket -(if (positive? x) (send rocket-object launch) +(if (positive? x) (launch r) (redirect (- x))) ] +] Each definition and each local definition deserves at least one line. +@compare[ @racketmod[#:file @tt{good} racket -(define (start-reactor x) - (define width (* 10 x)) - (define height (* 3 x)) +(define (launch x) + (define wdt (* 10 x)) + (define hgt (* 3 x)) ...) ] @@ -93,10 +98,11 @@ racket @tt{bad} racket -(define (start-reactor x) - (define width (* 10 x)) (define height (* 3 x)) +(define (launch x) + (define wdt (* 10 x)) (define hgt (* 3 x)) ...) ] +] @margin-note{We need more of these rules} @; -----------------------------------------------------------------------------