Re: A better file copy function (was Re: copying files )

On Sat, 20 Aug 2005 02:57:24 -0700, "Are you Crazy!"
<You@xxxxxxxxxxxxxxxxxxxx> wrote:

>The WinAPI call will work, but as you can see there is no real way to
>handle the errors.

How so? What is wrong with GetLastError?

> While I think Duncan is a smart-ass,

Hey, if you don't want people to critique your code, you are in the
wrong place ;~). Seriously though, one of the main benefits I have
gained from posting here over the years is to get feedback from other
people. Very handy to see how others can improve the code, especially
if (as in my case) your work environment provides few opportunities
for formal code reviews.

>// Function StreamCopy(Source,Dstination,MoveOption:boolean) : integer
>//
>// Returns the number of bytes copied, or returns the IO Error code and
>// additionaly returns the portion of the function in which the copy
>// failed, as a negative number

I am still puzzled by this approach of converting exceptions to error
codes. I asked about it before but didn't see an answer, and I am
curious enough to ask again. If you want to use error codes, why
bother with exceptions? If you want to use exceptions, why convert
them to error codes?

I am also puzzled by this double-encoded returned value. You are
imposing a significant parsing and error checking requirement on every
caller of your function. Can you explain what benefit you gain to
offset the added complexity?

>// XXX indicates the OS resurend error code for the particular
>// operation.

Isn't that the same value you would have got back from GetLastError?
Why is it easier to extract the code from your return value than use
the same value supplied directly from using the Win API?

> // Note that I have used the value of zero, this will
> // ensure the function starts at the very begining of the
> // stream and copies until EOF. Using InStream.Size, while
> // correct, does not gaurantee that InStream will be at the
> // begining of the file.

Sort of. It isn't the use of InStream.Size that guarantees the file
pointer will be at the beginning of the file, it is the fact that you
just used TFileStream.Create to open InStream. TFileStream.Create
ends up calling the CreateFile WinAPI function, and that IS guaranteed
to position the file pointer to the start of the file. The value of
zero will work as you say though.

> result := OutStream.CopyFrom(InStream,0);
> OutStream.Free ;
> InStream.Free ;

There is a bug here. If an exception is raised during the copy - say I
remove a floppy disk you are copying from or to - the streams are not
freed. When you create objects in this way you should always use try
.... finally blocks to ensure that they are freed correctly, or you get
memory leaks.

> if MoveOption then
> try
> DeleteFile(Source);
> except
> on E:EInOutError do
> begin
> // Something failed on the delete
> result := (e.ErrorCode + 4000) * -1 ;
> exit;

This is the first of 4 exit statements. None of them is needed. The
code will flow through to the end of the function from here anyway.

--
Duncan
.