6 ms·
Having used defer in golang, I'm firmly in the camp that scope-based cleanup like defer is bad and value-based cleanup like destructors is better. Say you have
by Arnavion 5y ago
Having used defer in golang, I'm firmly in the camp that scope-based cleanup like defer is bad and value-based cleanup like destructors is better.
Say you have this (pseudocode):
function foo() {
let bar = make_bar();
defer cleanup_bar(bar);
let baz = make_baz();
defer cleanup_baz(baz);
let quux = make_quux();
defer cleanup_quux(quux);
do_something_with(bar, baz, quux);
}
Now imagine you have multiple functions like this that all create bar, baz and quux's. Say these are unit tests and the bar-baz-quux are mocks or whatever.
So you decide to DRY by moving the creation to a common function.
function create_mocks() -> (Bar, Baz, Quux) {
let bar = make_bar();
defer cleanup_bar(bar);
let baz = make_baz();
defer cleanup_baz(baz);
let quux = make_quux();
defer cleanup_quux(quux);
(bar, baz, quux)
}
... except this doesn't work any more, because the cleanup happens within create_mocks and create_mocks ends up returning cleaned-up values.
You could fix this by switching to a callback approach:
function run_with_mocks(f: function(Bar, Baz, Quux)) {
let bar = make_bar();
defer cleanup_bar(bar);
let baz = make_baz();
defer cleanup_baz(baz);
let quux = make_quux();
defer cleanup_quux(quux);
f(bar, baz, quux)
}
... but now you have to make the caller use a callback that necessarily returns void and not any other type. (This is mostly a golang problem, since languages with generics can be generic on the return type of the callback.) It also means you can't easily do things like set variables in an outer scope easily, depending on how the language deals with closure captures.
Worse, if not all tests use the same mocks, you actually end up with multiple of these functions:
function with_bar(f: function(Bar)) {
let bar = make_bar();
defer cleanup_bar(bar);
f(bar)
}
...
function foo() {
with_bar(bar => {
with_baz(baz => {
with_quux(quux => {
do_something_with(bar, baz, quux);
});
});
});
}
All these problems are avoided by having destructors.
- AndyKelley 5y ago> except this doesn't work any more it does in zig: fn create_mocks() !struct {bar: Bar, baz: Baz, quux: Quux} { var bar = try make_bar(); errdefer cleanup_bar(bar); var baz = try make_baz(); errdefer cleanup_baz(baz); var quux = try make_quux(); errdefer cleanup_quux(quux); return .{ .bar = bar, .baz = quux, .quux = quux, }; }
- andrenth 5y agoI would put the mocks in a Mock struct with a Cleanup() method and have the caller call defer mocks.Cleanup()
- Arnavion 5y agoPlease see the third example I gave, which talks about the set of mocks not being the same for every test.
- axaxs 5y agoYou keep saying this, but it doesn't make a lot of sense. Your initial pretense was that you always make 3 objects, so move it into a function. When people show how to accomplish that, you state 'well what about my test that doesn't need all three?' Why are you testing things your code doesn't do?
- Arnavion 5y ago>When people show how to accomplish that, you state 'well what about my test that doesn't need all three?' You've phrased this as if I said this after I'd been given a solution for the initial problem. In reality, I mentioned this in the original post itself, but it seems many people didn't read it. It's almost like I have prior real-world experience of this problem, where I already went down the path of making common functions for all the creation and cleanup respectively, only to find that it doesn't help when all the functions don't create the same subset of things. >Why are you testing things your code doesn't do? I have no idea how you came to this conclusion.
- axaxs 5y agoHey, I didn't mean to come across as rude but when I reread it, it was. I wasn't expressing myself well, I apologize. I've been a Go mentor for years, and see how people take concepts they are used to and try to shoehorn them in, and complain they are ugly. In this case, when I first saw your examples, my thought was 'youre doing it wrong'. But not wrong as in incorrect, but now how the language was designed. If you truly care we can go through scenarios and I can tell you how I'd approach it. But in my past experience, when people find a paradigm they like, they tend to not listen.
- dnautics 5y agoThis is not really a problem in zig. If you really need a destructor, write a .deinit() function in your struct, and call that. For your case with mocks (note mock as a noun, not a verb, which is a bit of an antipattern anyways, see: http://blog.plataformatec.com.br/2015/10/mocks-and-explicit-contracts/ http://blog.plataformatec.com.br/2015/10/mocks-and-explicit-...). That aside: Go doesn't have generics so you can't really do this, but in the case of zig, your function would be parametrized on the type, so you would issue it a struct to be shared by the tests that has the common initialization upfront, make_ functions that return the pre-initialized stuff, followed by running the function, followed by no-op deinits, followed by whatever forensics you'd like to run later.
- Arnavion 5y ago>This is not really a problem in zig. If you really need a destructor, write a .deinit() function in your struct, and call that. This is missing the point. The point of a destructor is that it runs automatically. .deinit() is the same as the three cleanup_*()s in my example and thus has the same problems. >but in the case of zig, your function would be parametrized on the type, so you would issue it a struct to be shared by the tests that has the common initialization upfront The third example in my post shows why this is inadequate.
- dnautics 5y agoThen im not really sure what your point is, because I've written zig code that reuses code in a similar way to what you are talking about, with three different forms of parallelism calling into some common runner code each of which has different needs for resource cleanup times (single threaded, cooperative, and independent parallelism). It wasn't a problem. Have you tried doing what you propose in zig? Ultimately, the only difference between a .deinit() and a destructor is that one is explicit and the other is not. There are cases where you can easily make a mistake for both an explicit destructor and an implicit destructor.
- lhorie 5y agoBut that's because you're doing continuation passing style. You could do this instead: func make_foo() *Foo { return &Foo { bar: make_bar(), baz: make_baz(), quux: make_quux(), } } func cleanup_foo(foo *Foo) { cleanup_bar(foo.bar) cleanup_baz(foo.baz) cleanup_quux(foo.quux) } // elsewhere foo := make_foo() defer cleanup_foo(foo) // [rest of logic goes here]
- Arnavion 5y agoPlease see the third example I gave, which talks about the set of mocks not being the same for every test.
- lhorie 5y agoThere are several options Poor man's: func make_foo_with_mock_bar() *Foo { return &Foo { bar: make_mock_bar(), baz: make_baz(), quux: make_quux(), } } Explicit injection: func make_foo(bar: Bar, baz: Baz, quux: Quux) *Foo { return &Foo { bar: bar, baz: baz, quux: quux, } } Or use some DI framework Even if you have reservations about where verbosity lies, I don't see how this affects the cleanup side.
- Arnavion 5y agoYou've just restated your original comment, without addressing the fact that this does not work with the third example in my original comment.
- ghoward 5y agoThese are great examples. I knew that I preferred destructors over defer, but I could not explain why. You put it into words. I think people have a bias against destructors because of C++, but they really are pretty nice. Speaking as a guy that uses C instead of C++, destructors are the one thing I would add to C.
- Arnavion 5y agoThey certainly have their problems. They're functions that get invoked without any code to explicitly invoke them. You have to consider whether a destructor can be invoked on a partially constructed object. You have to consider whether a destructor can be invoked more than once. You have to consider providing the ability to not call the destructor for performance. etc etc It's understandable why a language trying to be close to C in spirit does not have them.
- hnakamur 5y agoI agree. For example, most of Rust libraries uses the drop handler (something like desctructors) to close a file descriptor. However it has problems since in async model closing is also async and it may return an error. So in tokio-uring explicit async close will be provided. https://github.com/tokio-rs/tokio-uring/pull/1/files#diff-3dc5dd454e080eb849ee5efaf79df2585fbe0a06804d69249b4c81da05a63875R375-R400 https://github.com/tokio-rs/tokio-uring/pull/1/files#diff-3d... And you can call close like: file.close().await.unwrap(); https://github.com/tokio-rs/tokio-uring/pull/17/files#diff-bd6920ff81a489bba8cfe35dc49b5ad7bb4a4f52086f9f8766d25a8f8d791709R71 https://github.com/tokio-rs/tokio-uring/pull/17/files#diff-b...
- pcwalton 5y agoNote that there are ideas to fix this issue in the async foundations WG. It's arguably more of a problem with async as implemented in Rust than with the concept of RAII.
- ghoward 5y agoYes, those are all problems with destructors, but they are well understood. However, in a language I am currently creating, calling a destructor on a partial object is impossible because the object is created in "one" step. In other words, constructors themselves don't have access to the object in a partially created state. It does make things much easier.
- AlexCoventry 5y agoThe common approach in go is something like func newMocks( ) ( foo Foo, bar Bar, baz Baz, cleanup func(), ) and making the caller responsible for calling the cleanup function, possibly in a defer. To me, this seems like a good trade off between making a little more work for the caller and keeping things explicit and under the caller's control.
- Arnavion 5y agoThis doesn't work with the third example, ie where the set of mocks is not the same for every test. This suggestion devolves to the original code that we were trying to refactor.
- Mawr 5y agoThen the original code is perfectly fine and does not need refactoring. You create the mocks you need with cleanup defers and you're done. You could just make a function that creates all the mocks needed for all tests, but you don't like that. So the only option left is to specify which mocks you need for each test... which is exactly what the original code is doing. You're running into a wall because you're trying to abstract code that's already maximally abstracted.
- cmrdporcupine 5y ago100%. And another problem with defer is that it only works in function scope, not block scope. This bit me in a piece of Go I had to refactor into multiple functions recently when all I wanted to do was a few lines change around a 'for' loop. The code review ended up being more substantial and it made the code less readable than just being able to sprinkle in some additional defers. With the C++ destructor RAII pattern I can use it at any lexical scope I need it at, which is super handy.
- Jtsummers 5y agoZig's defers, at least, are block scoped. So they are executed at the end of whatever block they're created in, in contrast to Go's version of defer which is function scoped only.
- naikrovek 5y agoRAII is very obviously a bad idea, and I'm shocked that more people can't see this. it's "fine" until it isn't, and when it bites you, it crushes your arm. you are unlikely to ever forget this after it happens. it's like driving a car fast while blindfolded; you will crash, and it will hurt, it's just a matter of when.
- pcwalton 5y agoCare to explain why? In Rust RAII works very well, and the common failure mode of RAII--use-after-free--is statically prevented by the borrow check.
- cmrdporcupine 5y agoUse-after-free is definitely the major footgun portion of RAII in C++ (in my experience) but seems to me like the same issue would be present in any non-GC'd language that offers a 'deferral' mechanism, asynchronous or not (unless it has ownership/borrow management like Rust as you point out).
- marton78 5y agoYour claims would benefit from you substantiating them.
- axaxs 5y agoI'm not arguing against destructors, but this example is ultra contrived. Why would you defer cleaning up objects you are returning? That's no longer your responsibility. Instead you'd return an object, and make it the callers responsibility to close it. Destruct all three in the close call. It may not fit your mental model, and that's fine. But let's not pretend it's a serious hurdle.
- fauigerzigerk 5y ago>It may not fit your mental model... No, it's not a psychological problem. This is real complexity, and it's one of the most serious and widespread issues in software development. The key issue is that we often have rules that say: Whenever A happens then B must happen as well. It is absolutely critical that rules like this are expressed in exactly one place and that enforcement is opt-out not opt-in. Everything else is error prone and creates significant mental burden.
- oconnor663 5y agoI wonder if "conditionally send a local variable over a channel" is another good example of something that's hard to clean up with defer. One upside of destructors is that adding destructor cleanup to a type isn't necessarily a breaking change. (In Rust terms, as long as the type isn't "Copy".) But in languages with defer or with/using statements, it is. Another upside of destructors is that the cleaned-up state can be mostly hidden from other code. But when cleanup is a regular function, it's easier to access an object after cleanup, and you have to reason more about what happens in that case. (A Rust example here might be "What happens if you use a Box after it's freed?" The answer is that you just can't do that, even though Box doesn't have a freed/null state.)
- j1elo 5y agoWait, the moment a creator function returns its created values, it is also returning their ownership. So at least with that concrete DRY refactor you are showing, you're effectively not only moving code but also responsibilities around. But I get your point: with destructors you are free to change the ownership responsibilities, because the link between a variable and its releasing is implicit and carried over by the runtime, instead of explicitly being cared for by the code.