Commit cd65fcb
authored
Record decision about optional fields as ADR. (googleapis#1358)
This is just rewriting googleapis#934 as a ADR.1 parent 25b778a commit cd65fcb
1 file changed
Lines changed: 69 additions & 0 deletions
Lines changed: 69 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,69 @@ | |||
| 1 | + ## The GCS Library uses optional fields only for sub-objects. | ||
| 2 | + | ||
| 3 | + **Status**: accepted | ||
| 4 | + | ||
| 5 | + **Context**: because the underlying protocol for GCS is JSON, any field can be | ||
| 6 | + absent from the responses received from the server. Furthermore, the application | ||
| 7 | + can use the `fields` query parameter to select a subset of the fields in a | ||
| 8 | + response. A natural question is whether the fields should be represented as | ||
| 9 | + `optional<T>`. That is, whether the class to represent object attributes should | ||
| 10 | + look like this: | ||
| 11 | + | ||
| 12 | + ```C++ | ||
| 13 | + class ObjectMetadata { public: | ||
| 14 | + std::string const& name() const; | ||
| 15 | + std::chrono::system_clock::time_point time_created() const; | ||
| 16 | + CustomerEncryption const& customer_encryption() const; | ||
| 17 | + }; | ||
| 18 | + ``` | ||
| 19 | + | ||
| 20 | + or it should look like this: | ||
| 21 | + | ||
| 22 | + ```C++ | ||
| 23 | + using google::cloud::optional; | ||
| 24 | + class ObjectMetadata { public: | ||
| 25 | + optional<std::string> const& name() const; | ||
| 26 | + optional<std::chrono::system_clock::time_point> time_created() const; | ||
| 27 | + optional<CustomerEncryption> const& customer_encryption() const; | ||
| 28 | + }; | ||
| 29 | + ``` | ||
| 30 | + | ||
| 31 | + **Decision**: | ||
| 32 | + | ||
| 33 | + * For string fields where there is no semantic difference between an empty | ||
| 34 | + string and the field not present we just use `std::string<>`. | ||
| 35 | + * For array fields where there is no semantic difference between field not | ||
| 36 | + present and an empty array we just use `std::vector<>`. | ||
| 37 | + * For integer and boolean fields we default to `0` (and `false`) if the field | ||
| 38 | + is not present. | ||
| 39 | + * For object fields we default to wrapping the field in `optional<>`. | ||
| 40 | + | ||
| 41 | + For fields wrapped in `optional<>` we offer convenience functions to make it | ||
| 42 | + easier to operate on these fields. For a field called `foo` these are: | ||
| 43 | + | ||
| 44 | + * `has_foo()` returns true if the field is set. | ||
| 45 | + * `foo()` returns the field value if set, the behavior is undefined if the value | ||
| 46 | + is not set. | ||
| 47 | + * `foo_as_optional()` returns the optional field. | ||
| 48 | + * `reset_foo()` resets the field (for writable fields). | ||
| 49 | + * `set_foo()` sets the field (for writable fields). | ||
| 50 | + | ||
| 51 | + **Consequences**: The advantage of this approach is that most fields are easy | ||
| 52 | + to use most of the time. The disadvantage of this approach include: | ||
| 53 | + | ||
| 54 | + * The ambiguity when the application filters the returned fields, the value | ||
| 55 | + may be the default value because the client did not get the field. | ||
| 56 | + * As applications change over time and they start filtering different fields | ||
| 57 | + the code may assume that the value of a field is valid, but it has a default | ||
| 58 | + value. With optionals the application should crash during testing, or may | ||
| 59 | + be programmed defensively since the start. | ||
| 60 | + * It also requires more thought designing the classes a field has different | ||
| 61 | + semantics for "not there" vs. "the default value". | ||
| 62 | + | ||
| 63 | + **Reference** | ||
| 64 | + | ||
| 65 | + This was originally discussed in | ||
| 66 | + [#934](https://github.com/GoogleCloudPlatform/google-cloud-cpp/issues/934). | ||
| 67 | + | ||
| 68 | + The original [PR](https://github.com/GoogleCloudPlatform/google-cloud-cpp/pull/1358) | ||
| 69 | + also has some interesting discussions. | ||
0 commit comments