Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2006-06

A suggestion about streams

4 messages · 2 people ·

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Sergei Gorelkin

Hello,

Currently, TStream.Read and TStream.Write methods are abstract. This means that each and every descendant of TStream must override these methods with their implementation.

FCL has many read-only or write-only TStream descendants, which override Read or Write methods just to indicate that method is unusable, i.e. raise some kind of exception.

Why not implement TStream.Read and TStream.Write like this:

procedure TStream.Read(...);
begin
  raise EStreamError.CreateFmt('Reading is not supported by %s',
                               [ClassName]);
end;

Since ClassName will be the actual name of the class which executed the call, such implementation will give correct and meaningful error message for every descendant class.

Then, all FCL code can be cleaned from similar procedures.

What is your opinion? It looks that such modification will not cause any incompatibilities with existing code. And if you are not against, I can do the required patching.

Best regards, Sergei

Reply by email

Michael Van Canneyt
to Sergei Gorelkin
Sergei Gorelkin wrote 27 lines
Hello, Currently, TStream.Read and TStream.Write methods are abstract. This means that each and every descendant of TStream must override these methods with their implementation. FCL has many read-only or write-only TStream descendants, which override Read or Write methods just to indicate that method is unusable, i.e. raise some kind of exception. Why not implement TStream.Read and TStream.Write like this: procedure TStream.Read(...); begin raise EStreamError.CreateFmt('Reading is not supported by %s', [ClassName]); end; Since ClassName will be the actual name of the class which executed the call, such implementation will give correct and meaningful error message for every descendant class. Then, all FCL code can be cleaned from similar procedures. What is your opinion? It looks that such modification will not cause any incompatibilities with existing code. And if you are not against, I can do the required patching.

There is a difference for the implementor of streams: By keeping the method abstract, you FORCE the implementation of a method in a descendent.

The compiler will warn you if you try to instantiate a class which has an abstract method. If you implement the base method as virtual method, you will lose this warning, but you will not be aware that you have forgotten to override a method.

Michael.

Reply by email

Sergei Gorelkin
to Michael Van Canneyt

On Friday, June 30, 2006, 6:46:48 PM, Michael Van Canneyt wrote:

MVC> There is a difference for the implementor of streams: MVC> By keeping the method abstract, you FORCE the implementation of a method MVC> in a descendent.

MVC> The compiler will warn you if you try to instantiate a class which has an MVC> abstract method. If you implement the base method as virtual method, you MVC> will lose this warning, but you will not be aware that you have forgotten MVC> to override a method.

This is, of course, right. But is it important in particular case of TStream? Stream implementors basically have to implement only 3-4 methods, so it's rather hard to forget to override a method. And even if one does forget, that will pop out on the first run.

The majority of FCL streams (encrypting, compressing, buffering, pipes, etc) are unidirectional, and only few (handle/file and memory) allow both reading and writing. Calling an invalid method (e.g. Read for write-only stream) is typically a programmer's (not user's) error and should never happen in a well written program. For this reason, IMO, keeping a separate handler and error message for each unidirectional stream merely increases the program size.

Best regards, Sergei

Reply by email

Michael Van Canneyt
to Sergei Gorelkin
Sergei Gorelkin wrote 23 lines
On Friday, June 30, 2006, 6:46:48 PM, Michael Van Canneyt wrote: MVC> There is a difference for the implementor of streams: MVC> By keeping the method abstract, you FORCE the implementation of a method MVC> in a descendent. MVC> The compiler will warn you if you try to instantiate a class which has an MVC> abstract method. If you implement the base method as virtual method, you MVC> will lose this warning, but you will not be aware that you have forgotten MVC> to override a method. This is, of course, right. But is it important in particular case of TStream? Stream implementors basically have to implement only 3-4 methods, so it's rather hard to forget to override a method. And even if one does forget, that will pop out on the first run. The majority of FCL streams (encrypting, compressing, buffering, pipes, etc) are unidirectional, and only few (handle/file and memory) allow both reading and writing. Calling an invalid method (e.g. Read for write-only stream) is typically a programmer's (not user's) error and should never happen in a well written program. For this reason, IMO, keeping a separate handler and error message for each unidirectional stream merely increases the program size.

Hm. Yes. You can supply a patch, if you want. But then a patch which fixes all streams...

Michael.

Reply by email