Moving things out of using statements

[Avery Pennarun]

What exactly does this mean?

> versaplexd/vxapi.cs: Move a few lines of code out of 'using' stmt.
>
> Since we want to 'Cancel' commands, we want as few lines of code as
> possible in 'Using' statements where Dbi handles are involved (since we
> want the relevant Dbi objects to be disposed of as quickly as possible).
>
> Move a few lines of code in ExecRecordset.

I can see the part of the logic where the statements don't *need* to
be inside the using{} block, since they don't actually rely on the
variable being used. And I can see the part where you might as well
dispose of that object sooner rather than later.

But where's the part where this has actually suddenly become
important? You can abort a using block halfway through (by returning
or throwing an exception, for example), so that's not a problem. And
the statements you moved don't look like they'd cause significant
delay.

Was it causing a problem of some sort?

I guess I don't object to this change so much as wonder what motivated it.

Have fun,

Avery

[Lukasz Kosewski]

Hola,

On Wed, Nov 26, 2008 at 1:18 PM, Avery Pennarun <ave...@versabanq.com> wrote:
> I can see the part of the logic where the statements don't *need* to
> be inside the using{} block, since they don't actually rely on the
> variable being used. And I can see the part where you might as well
> dispose of that object sooner rather than later.
>
> But where's the part where this has actually suddenly become
> important? You can abort a using block halfway through (by returning
> or throwing an exception, for example), so that's not a problem. And
> the statements you moved don't look like they'd cause significant
> delay.
>
> Was it causing a problem of some sort?

Err... well. Originally, I thought it was an issue, because what I
assumed happened is that 'Cancel' on an existing IDbCommand
immediately raised an exception flag. The documentation for 'Cancel'
isn't great, unfortunately.

Empirical testing showed that, in fact, if the command is currently in
the ExecuteReader() phase, *that* throws an exception, otherwise, any
following calls to Read() (rows, etc.) do.

Given my initial thoughts, moving things out of using() statements
made sense, because that shortened the lifespan of commands (there's
no point 'Cancel'ing if we're all done our DB query and are exiting a
function). However, given the new data, it changes nothing.

I guess I can pull those changes back, though they don't functionally
affect the code, really.

Your call,

Luke

[Peter McCurdy]

Well, given that they don't depend on the variable in the using block,
there's even less reason to move them back than there was to move them
out. Might as well leave things that tiny bit more obvious.

--
Peter McCurdy
Co-founder, The Navarra Group
http://navarra.ca