6 ms·
The original poster says that these functions can't ever have invariants broken: > Functions that only mutate instances of well-encapsulated types through thei
by lalaithion 5y ago
The original poster says that these functions can't ever have invariants broken:
> Functions that only mutate instances of well-encapsulated types through their public APIs. That includes nearly all functions that are not methods.
> Methods of well-encapsulated types that only mutate non-member instances of other well-encapsulated types through their public APIs.
This is incorrect! Consider the following function (in psuedocode):
func insert(id: Int, object: ThingInSpace) {
ids.update(id);
var location = get_location(object);
locations[id] = location;
var metadata = compute_metadata(object);
metadatas[id] = metadata;
}
If compute_metadata or get_location throw, then even though we're only modifying dictionaries or sets through their public APIs, we can still invalidate an invariant that affects our program correctness. This function is safe if and only if get_location and compute_metadata cannot throw. If they can throw, this should be rewritten as
func insert(id: Int, object: ThingInSpace) {
var location = get_location(object);
var metadata = compute_metadata(object);
ids.update(id);
locations[id] = location;
metadatas[id] = metadata;
}
Adding "try" to the original function makes it clear that this function can destroy the invariant of ids, locations, and metadatas, and it needs to be rearranged to be the second function in order for the code to be correct.
- Spivak 5y agoBut you broke encapsulation because locations and metadatas have an invariant that must be externally enforced. I don't think that meets the standard of well-encapsulated. If locations and metadatas must be updated together then you should only be able to do it via some method update() that takes location and metadata.
- lalaithion 5y agoConsider this to be the only function that modifies locations and metadatas. It is the function that is supposed to maintain that invariant. You can pass in location and metadata (or make this a method on an object with those values as private members) without changing my point. Under the rules laid down by the original poster in the link, functions which only modify well-encapsulated types via their public APIs cannot violate invariants. My code shows that there are "invariant[s] that must be externally enforced" even in code that only deals with well-encapsulated types via their public APIs. And if you don't have explicit error handling, it isn't obvious that the invariant can be broken.
- Spivak 5y ago> well-encapsulated types via their public APIs cannot violate invariants … of those well-encapsulated types. Throwing an exception like this cannot break an internal invariant of the list object which is what the author is saying. You can’t break the list object which sounds crazy by modern standards but in languages without this kind of encapsulation (like C) this can and does happen.
- lalaithion 5y agoThat's a very useful guarantee, I agree, but it's not a good enough reason to not have "try" as a syntax.
- cipherzero 5y agoYou’re maybe right, but does that mean the language should be hostile to those that don’t write well designed code?
- b3morales 5y agoYour premise assumes that `insert` will abort immediately if `get_location` or `compute_metadata` throw, but that's an arbitrary (poor) decision by the author of `insert`. The errors can (should) be caught and the relationship between `locations` and `metadatas` restored in the handler. And there's certainly no reason for `insert` to re-throw its helpers' internal errors. What would its caller do with them?
- lalaithion 5y agoI agree. A function that properly handles its helpers internal errors is better than a function that simply rethrows them. However, both are better than a function that simply rethrows them AND leaves the application in an inconsistent state. Additionally, adding the "try" syntax makes it clear that "insert" has been written poorly. For example, consider the code: func insert(id: Int, object: ThingInSpace) { var location = try get_location(object); ids.update(id); locations[id] = location; metadatas[id] = compute_metadata(object); } Now I can be sure that compute_metadata doesn't fail, so it's fine that I've inlined the call, whereas it's important that get_location isn't inlined. But this syntax also suggests to a code reviewer that maybe insert should wrap the error returned by get_location and caught. Of course you could always catch the error and restore the state, but without the `try` syntax, you can't tell by reading the function which lines can fail, and where you need to handle the error.
- nine_k 5y agoBTW thank you for reminding what a pain mutable data are, and doubly so, mutable data that can be put into invalid by merely calling legitimate methods.
- deleted 5y ago[deleted]
- nybble41 5y agoYour example doesn't fit into either of those two categories. In order to have an invariant, `ids`, `locations`, and `metadatas` would be need to be members of a well-encapsulated type, one which has `insert` as a method. So the first category is out since `insert` mutates the instance's members directly, without going through the instance's public API, and the second category doesn't apply since `insert` mutates its member instances, and not only non-member instances. I believe the author would agree that this function needs the "try" keyword to show that the invariants can be broken by an ill-placed throw. The proposal is to keep the "try" keyword but allow functions to explicitly opt out so that "try" is not needed on every function call which can throw. The author suggested a "throws_anywhere" keyword after the argument list. My proposal would be to add "try blocks" which have the effect of allowing an exception to be rethrown anywhere inside the block (i.e. within a "try block" you can omit the normal "try" on a function call). The pattern: try { do_something(); do_something_else(); } would be equivalent to: try do_something(); try do_something_else(); So if you wanted the effect of "throws_anywhere" you just wrap the whole body of the function in one of these blocks. But you could also wrap just part of a function, for example just the part where the invariants are maintained, and require explicit handling in the parts where invariants are temporarily broken.