Foundations of API Design chapter #2994
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| minutes: 2 | ||
| --- | ||
|
Collaborator
There was a problem hiding this comment. Do you intend to add something here? At the very least, a title and |
||
|
|
||
| # Foundations of API Design | ||
|
|
||
| {{%segment outline}} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| --- | ||
| minutes: 5 | ||
| --- | ||
|
|
||
| # Meaningful Doc Comments | ||
|
|
||
| ```rust,compile_fail | ||
| /// API for the client // ❌ Lacks detail | ||
| pub mod client {} | ||
|
|
||
| /// Function from A to B // ❌ Redundant | ||
| fn a_to_b(a: A) -> B {...} | ||
|
|
||
| /// Connects to the database. // ❌ Lacks detail │ | ||
| fn connect() -> Result<(), Error> {...} | ||
| ``` | ||
|
|
||
| Doc comments are the most common form of documentation developers | ||
| engage with. | ||
|
|
||
| Good doc comments provide information that the code, names, and types | ||
| cannot, without restating the obvious information. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| --- | ||
| minutes: 5 | ||
| --- | ||
|
|
||
| # The Anatomy of a Doc Comment | ||
|
|
||
| 1. A brief, one-sentence summary. | ||
| 2. A more detailed explanation. | ||
| 3. Special sections: code examples, panics, errors, safety preconditions. | ||
|
|
||
| ````rust,no_compile | ||
| /// Parses a key-value pair from a string. | ||
| /// | ||
| /// The input string must be in the format `key=value`. Everything before the | ||
| /// first '=' is treated as the key, and everything after is the value. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use my_crate::parse_key_value; | ||
| /// let (key, value) = parse_key_value("lang=rust").unwrap(); | ||
| /// assert_eq!(key, "lang"); | ||
| /// assert_eq!(value, "rust"); | ||
| /// ``` | ||
| /// | ||
| /// # Panics | ||
| /// | ||
| /// Panics if the input is empty. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns a `ParseError::Malformed` if the string does not contain `=`. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// Triggers undefined behavior if... | ||
| unsafe fn parse_key_value(s: &str) -> Result<(String, String), ParseError> | ||
|
|
||
| enum ParseError { | ||
| Empty, | ||
| Malformed, | ||
| } | ||
| ```` | ||
|
|
||
| <details> | ||
|
|
||
| - Idiomatic Rust doc comments follow a conventional structure that makes them | ||
| easier for developers to read. | ||
|
|
||
| - The first line of a doc comment is a single-sentence summary of the function. | ||
| Keep it concise. `rustdoc` and other tools have a strong expectation about | ||
| that: it is used as a short summary in module-level documentation and search | ||
| results. | ||
|
|
||
| - Next, you can provide a long, multi-paragraph description of the "why" and | ||
| "what" of the function. Use Markdown. | ||
|
|
||
| - Finally, you can use top-level section headers to organize your content. Doc | ||
| comments commonly use `# Examples`, `# Panics`, `# Errors`, and `# Safety` as | ||
| section titles. The Rust community expects to see relevant aspects of your API | ||
| documented in these sections. | ||
|
|
||
| - Rust heavily focuses on safety and correctness. Documenting behavior of your | ||
| code in case of errors is critical for writing reliable software. | ||
|
|
||
| - `# Panics`: If your function may panic, you must document the specific | ||
| conditions when that might happen. Callers need to know what to avoid. | ||
|
|
||
| - `# Errors`: For functions returning a `Result`, this section explains what | ||
| kind of errors can occur and under what circumstances. Callers need this | ||
| information to write robust error handling logic. | ||
|
|
||
| - **Question:** Ask the class why documenting panics is so important in a | ||
| language that prefers returning `Result`. | ||
|
|
||
| - **Answer:** Panics are for unrecoverable, programming errors. A library | ||
| should not panic unless a contract is violated by the caller. Documenting | ||
| these contracts is essential. | ||
|
|
||
| - `# Safety` comments document safety preconditions on unsafe functions that | ||
| must be satisfied, or else undefined behavior might result. They are discussed | ||
| in detail in the Unsafe Rust deep dive. | ||
|
|
||
| </details> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| --- | ||
| minutes: 15 | ||
| --- | ||
|
|
||
| # Avoiding Redundancy | ||
|
|
||
| Names and type signatures communicate a lot of information, don't repeat it in | ||
| comments! | ||
|
|
||
| ```rust,compile_fail | ||
| // Repeats name/type information. Can omit! | ||
| /// Parses an ipv4 from a str. Returns an option for failure modes. | ||
| fn parse_ip_addr_v4(input: &str) -> Option<IpAddrV4> { ... } | ||
| // Repeats information obvious from the field name. Can omit! | ||
| struct BusinessAsset { | ||
| /// The customer id. | ||
| let customer_id: u64, | ||
| } | ||
| // Mentions the type name first thing, don't do this! | ||
| /// `ServerSynchronizer` is an orchestrator that sends local edits [...] | ||
| struct ServerSynchronizer { ... } | ||
| // Better! Focuses on purpose. | ||
| /// Sends local edits [...] | ||
| struct ServerSynchronizer { ... } | ||
| // Mentions the function name first thing, don't do this! | ||
| /// `sync_to_server` sends local edits [...] | ||
| fn sync_to_server(...) | ||
| // Better! Focuses on function. | ||
| /// Sends local edits [...] | ||
| fn sync_to_server(...) | ||
| ``` | ||
|
|
||
| <details> | ||
| - Motivation: Documentation that merely repeats name/signature information provides nothing new to the API user. | ||
|
|
||
| Additionally, signature information may change over time without the | ||
| documentation being updated accordingly! | ||
|
|
||
| - This is an understandable pattern to fall into! | ||
|
|
||
| Naive approach to "always document your code," follows this advice literally | ||
|
Collaborator
There was a problem hiding this comment. Consider adding the following nuance somewhere. In Google's style guide for Rust we talk about a distinction between library code and application code:
I think this distinction also applies here. Here's my quick attempt (could be a standalone slide):
Collaborator
There was a problem hiding this comment. WDYT about calling out the
Collaborator
Author
There was a problem hiding this comment. I think it'll need to be put in a more to explore section, a lot of material could be written on the value one can get out of lints. |
||
| but does not follow the intent. | ||
|
|
||
| Tests might enforce documentation coverage, this kind of documentation is an | ||
| easy fix. | ||
|
|
||
| - The name of an item is part of the documentation of that item. | ||
|
|
||
| Similarly, the signature of a function is part of the documentation of that | ||
| function. | ||
|
|
||
| Therefore: Some aspects of the item are already covered when you start writing | ||
| doc comments! | ||
|
Collaborator
There was a problem hiding this comment. Also: Don't be pressured to repeat information just to write a neat-looking bullet list which includes every parameter. |
||
|
|
||
| Do not feel you need to repeat information for the sake of a bullet point | ||
| list. | ||
|
|
||
| - Many areas of the standard library have minimal documentation because the name | ||
| and types do give enough information. | ||
|
|
||
| Rule of Thumb: What information is missing from a user's perspective? Other | ||
| than name, signature, and irrelevant details of the implementation. | ||
|
|
||
| - Don't explain the basics of Rust or the standard library. Assume the reader of | ||
| doc comments has an intermediate understanding of the language itself. Focus | ||
| on documenting your API. | ||
|
|
||
| For example, if your function returns `Result`, you don't need to explain how | ||
| `Result` or the question mark operators work. | ||
|
|
||
| - If there is a complex topic involved with the functions and types you're | ||
| documenting, signpost to a "source of truth" if one exists such as an internal | ||
| document, a paper, a blog post etc. | ||
|
|
||
| - Collaborate with Students: Go through the methods in the slide and discuss | ||
| what might be relevant to an API user. | ||
|
|
||
| ## More to Explore | ||
|
|
||
| - The `#![warn(missing_docs)]` lint can be helpful for enforcing the existence | ||
| of doc comments, but puts a large burden on developers that could lead to | ||
| leaning onto these patterns of writing low-quality comments. | ||
|
|
||
| This kind of lint should only be enabled if the people maintaining a project | ||
| can afford to keep up with its demands, and usually only for library-style | ||
| crates rather than application code. | ||
|
|
||
| </details> | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,55 @@ | ||||||||
| --- | ||||||||
| minutes: 15 | ||||||||
| --- | ||||||||
|
|
||||||||
| # Name-dropping keywords and signposting topics | ||||||||
|
|
||||||||
| ```rust | ||||||||
| /// This function covers <namedrop>, for further reading see: reference A, B, C. | ||||||||
| fn highly_specific_function(/* */) { /* ... */ | ||||||||
| } | ||||||||
|
Collaborator
There was a problem hiding this comment. Let's replace it with a concrete example. I was reading about the MARC format the other day. /// A parsed representation of a MARC 21 record [leader](//www.loc.gov/marc/bibliographic/bdleader.html).
///
/// A MARC leader contains metadata that dictates how to interpret the rest
/// of the record.
pub struct Leader {
/// Determines the schema and the set of valid subsequent data fields.
///
/// Encoded in byte 6 of the leader.
pub type_of_record: char,
/// Indicates whether to parse relationship fields, such as a "773 Host
/// Item Entry" for an article within a larger work.
///
/// Encoded in byte 7 of the leader.
pub bibliographic_level: char,
// ... other fields
}
/// Parses the [leader of a MARC 21 record](https://www.loc.gov/marc/bibliographic/bdleader.html).
///
/// The leader is encoded as a fixed-length 24-byte field, containing metadata
/// that determines the semantic interpretation of the rest of the record.
pub fn parse_leader(leader_bytes: &[u8; 24]) -> Result<Leader, MarcError> {
todo!()
}
#[derive(Debug)]
pub enum MarcError {}Likely most people in the audience are not familiar with library science, and the instructor could ask the audience to look for keywords in this documentation that the reader could search for to better understand what is going on ("MARC 21", "MARC record", "leader", "fields", "host item entry" etc.) |
||||||||
| ``` | ||||||||
|
|
||||||||
| <details> | ||||||||
| - Motivation: Readers of documentation will not be closely reading most of your doc comments like they would dialogue in a novel they love. | ||||||||
|
|
||||||||
| Users will most likely be skimming and scan-reading to find the part of the | ||||||||
| documentation that is relevant to whatever problem they're trying to solve in | ||||||||
| the moment. | ||||||||
|
|
||||||||
| Once a user has found a keyword or potential signpost that's relevant to them | ||||||||
| they will begin to search for context surrounding what is being documented. | ||||||||
|
|
||||||||
| - Ask the class: What do you look for in documentation? Focus on the | ||||||||
| moment-to-moment searching for information here, not general values in | ||||||||
| documentation | ||||||||
|
Collaborator
There was a problem hiding this comment.
Suggested change
|
||||||||
|
|
||||||||
| - Name-drop keywords close to the beginning of a paragraph. | ||||||||
|
|
||||||||
| This aids skimming and scanning, as the first few words of a paragraph stand | ||||||||
| out the most. | ||||||||
|
|
||||||||
| Skimming and scanning lets users quickly navigate a text, keeping keywords as | ||||||||
| close to the beginning of a paragraph as possible lets a user | ||||||||
|
|
||||||||
|
|
||||||||
| - Signpost, but don't over-explain. | ||||||||
|
|
||||||||
| Users will not necessarily have the same domain expertise as an API designer. | ||||||||
|
|
||||||||
| If a tangential, specialist term or acronym is mentioned try to bring in | ||||||||
| enough context such that a novice could quickly do more research. | ||||||||
|
|
||||||||
| - Signposting often happens organically, consider a networking library that | ||||||||
| mentions various protocols. But when it doesn't happen organically, it can be | ||||||||
| difficult to choose what to mention. | ||||||||
|
|
||||||||
| Rule of thumb: API developers should be asking themselves "if a novice ran | ||||||||
| into what they are documenting, what sources would they look up and are there | ||||||||
| any red herrings they might end up following"? | ||||||||
|
|
||||||||
| Users should be given enough information to look up subjects on their own. | ||||||||
|
|
||||||||
| - What we've already covered, predictability of an API including the naming | ||||||||
| conventions, is a form of signposting. | ||||||||
|
Comment on lines
+52
to
+53
Collaborator
There was a problem hiding this comment.
Suggested change
"What we've already covered" suggests that there is exactly one thing that we covered previously (or that we are referring to all of that material collectively). However we covered many things, and we are only pointing out one of them. |
||||||||
|
|
||||||||
| </details> | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| --- | ||
| minutes: 5 | ||
| --- | ||
|
|
||
| Names and Signatures are not full documentation | ||
|
|
||
| ```rust | ||
|
Collaborator
There was a problem hiding this comment. // bad
/// Returns a future that resolves when operation completes.
fn syncToServer() -> Future<Bool>
// good
/// Sends local edits to the server, overwriting concurrent edits
/// if any happened.
fn syncToServer() -> Future<Bool>// bad
/// Returns an error if sending the email fails.
fn send(&self, email: Email) -> Result<(), Error>
// good
/// Queues the email for background delivery and returns immediately.
/// Returns an error immediately if the email is malformed.
fn send(&self, email: Email) -> Result<(), Error> |
||
| ``` | ||
|
|
||
| <details> | ||
| - Motivation: API designers can over-commit to the idea that a function name and signature is enough documentation. | ||
|
|
||
| It's better than nothing, but it's worse than good documentation. | ||
|
|
||
| - Again, names and types are _part_ of the documentation. They are not always | ||
| the full story! | ||
|
|
||
| - TODO: give some rules of thumb for when to go into more detail, | ||
| cross-reference rust stdlib docs. This may live better in the name conventions | ||
| area. | ||
|
Collaborator
There was a problem hiding this comment. "Think about parts of the behavior that are not already covered by the function name, parameter names and types. In the example on the slide it is not obvious that
Collaborator
There was a problem hiding this comment. "Use comments to disambiguate. For example, consider a |
||
|
|
||
| </details> | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||
| minutes: 10 | ||||||||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Why and What, not How and Where | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Avoid documenting irrelevant details that may frequently change. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ```rust,no_compile | ||||||||||||||||||||||||||||||||
| /// Sorts a slice. Implemented using recursive quicksort. | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Oh this is an interesting case study you could turn around on its head as you go through it! "This comment says that it uses quicksort. This information is irrelevant to the caller, and could change in the future. Let's leave a suggested edit in the code review to remove this part of the comment." "Our code review came back and the author is pushing back, they are saying this is actually critical for the callers to know." "Did they say why? - no - huh? I don't understand, why would the caller need to know which algorithm is used here?" "We had a quick meeting with the PR author and it turns out they were trying to signal to the caller that they should not pass in untrusted data, because quicksort can go quadratic on carefully crafted collections! https://www.cs.dartmouth.edu/~doug/mdmspe.pdf" "Wow, I didn't know that! Let's write a comment that actually communicates that." The point here is that what is implementation detail vs not depends a lot on what the actual public contract is (e.g., can you supply untrusted data or not), and that is often something that only a human can do. Doing this on this slide would dilute the message; maybe do it on a separate slide as an interactive exercise? And then also pull this point "It could be that the implementation is necessary to explain, but this is likely due to whatever effects or invariants the user of that API needs to be aware of instead." to that slide, because this example would illustrate it well. |
||||||||||||||||||||||||||||||||
| fn sort_quickly<T: Ord>(to_sort: &mut [T]) { /* ... */ | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| /// Calls an org-internal service using reqwest. | ||||||||||||||||||||||||||||||||
| fn ask_service(url: &str) -> String { /* ... */ | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+16
Collaborator
There was a problem hiding this comment.
Suggested change
Could we do this, or would rustfmt not let us?
Comment on lines
+14
to
+16
Collaborator
There was a problem hiding this comment.
Suggested change
I'm not sure if you'd want to have a discussion about the "org-internal service" part, whether it is useful or not, so let's proactively omit that. |
||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. // bad
/// Fetches the resource by URL from the remote server.
///
/// This function creates a new connection, sets a timeout of 5000 ms,
/// and retries **3 times** if a 500 family error is returned, each time increasing the timeout 2x, therefore achieving exponential backoff, which is crucial for not overwhelming the remote server.
fn fetch_url(url: &str) -> Result<String, Error>
// good
/// Fetches the resource by URL.
///
/// Uses exponential backoff, configured by command line parameters X and Y.
fn fetch_url(url: &str) -> Result<String, Error>Most likely, there is some other place that is the source of truth for the exponential backoff
Collaborator
There was a problem hiding this comment. // bad
/// Saves a `User` record to the Postgres database.
///
/// This function opens a new connection and begins a transaction. It checks
/// if a user with the given ID exists with a `SELECT` query. If a user is
/// not found, performs an `INSERT`.
///
/// # Errors
///
/// Returns an error if any database operation fails.
pub fn save_user(user: &User) -> Result<(), db::Error> {
// ...
}
// good
/// Atomically saves a user record.
///
/// # Errors
///
/// Returns a `db::Error::DuplicateUsername` error if the user (keyed by
/// `user.username` field) already exists.
pub fn save_user(user: &User) -> Result<(), db::Error> {
// ...
} |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| <details> | ||||||||||||||||||||||||||||||||
| - Motivation: Using doc comments to explain how a function does something internally means if that internal implementation detail changes, the doc comment needs to change as well. | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Internal information is likely irrelevant to a user. Imagine explaining in a doc | ||||||||||||||||||||||||||||||||
| comment for a function that you're using for loops to solve a problem, what is | ||||||||||||||||||||||||||||||||
| the point of this information? | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - It could be that the implementation is necessary to explain, but this is | ||||||||||||||||||||||||||||||||
| likely due to whatever effects or invariants the user of that API needs to be | ||||||||||||||||||||||||||||||||
| aware of instead. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Focus on those effects and invariants instead of instead of the implementation | ||||||||||||||||||||||||||||||||
| details themselves. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Reiterate: Implementation details can and will change, so do not explain these | ||||||||||||||||||||||||||||||||
| details. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| TODO: Real-life example of something appropriate to a large system. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - Don't talk about where something is used for the sake of it. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| This is another instance where this information can become stale quickly. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| - Prefer instead to focus on what the function does (though again, not how it is | ||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||||||||||||||
| implemented) for a user trying to reach this practical information as quickly | ||||||||||||||||||||||||||||||||
| as possible. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| </details> | ||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.