[Mono-list] System.Text.ASCIIEncoding patch

[Piers Haken]

I'd say, first write the test cases you talked about, then if they pass
on the original code then nothing really needs to change. Unless, of
course, your code is significantly faster. I agree that GetMaxByteCount
needs some work, but it might be best to fix this so that it can work
correctly for all the Encoding-derived classes if at all possible.

I'll try to get the class status page to more accurately handle the case
when a missing method is implemented in a base class. Sorry about that.

Piers.

-----Original Message-----
From: Mike Kestner [mailto:mkestner@speakeasy.net] 
Sent: Friday, March 15, 2002 6:10 AM
To: Dietmar Maurer
Cc: Mono List; miguel@ximian.com
Subject: Re: [Mono-list] System.Text.ASCIIEncoding patch


On Fri, 2002-03-15 at 03:15, Dietmar Maurer wrote:
> On Fri, 2002-03-15 at 06:17, Mike Kestner wrote:

> It is a long time ago when I wrote those classes, but I thought 
> ASCIEncoding is already working. Why do you overwrite GetBytes, 
> GetChar, ... - most of them should be handled by the base class 
> (Encoder).

Well, I got started on the exercise because the class-status page showed
the class having a lot of unimplemented methods.  The mscorlib overrides
all the methods that I've implemented.  

If it was working already, I don't know how anyone else would have known
that without a public test suite. Also, the previous implementation of
GetMaxByteCount didn't exactly inspire the image of a complete,
well-tested implementation.  I suppose I could have written the tests
before filling in the class, but it sure looked like a stubbed
implementation at first glance.

Anyway, as to the overriding of the Get(Byte|Char)s methods, using
internal calls to an iconv based converter seems a bit heavy when a
simple comparison to 0x7f in managed code is all that is needed.  Yeah,
this is an optimization of unprofiled code, but it was a trivial one.

So, now that we've had the philosophical discussion, does anyone have
any comments on the code I posted.  If not, I'll commit it this weekend
with a test suite.

Mike


_______________________________________________
Mono-list maillist  -  Mono-list@ximian.com
http://lists.ximian.com/mailman/listinfo/mono-list