Message8200

Good morning Santoso.

I took a look at your patch on #2075 and the special-casing of binary, 
octal and hex seems a good approach to me. The CPython equivalent looks 
quite hairy. Really it would be nice if Oracle reworked their 
implementation for the benefit of all.

I ran the regression test suite "ant regrtest", and I believe this code 
turns up two new failures, in test_builtin and test_types, both to do 
with binary formatting. Actually, I think this is just one bug. Would 
you be prepared to take a look and provide an alternative patch?

The regression tests are a gift from CPython, more or less, and an 
indispensable tool in checking one's work. It is sometimes hard to guess 
which area relevant, but I always run them before pushing a change set 
and this has saved me many a time where I've induced a failure in a 
seemingly unrelated test.

(On my machine regrtest always produces failures, mostly unlink failures 
and some os module failures, which are Windows symptoms, but I've got 
used to which ones fail. It's cleaner on Linux.) You can run these tests 
in isolation. I use "dist\bin\jython -m test.test_builtin", etc..

We have a coding standard here: 
https://wiki.python.org/jython/CodingStandards. I have to quibble with 
your omission of curly braces from "if" statements: we always put them 
in. We are strict about this kind of thing not because it is vital to 
the code, but because otherwise change sets include a lot of 
"non-substantive" change, especially where an IDE enforces a particular 
coder's taste. It obscures the change that is the purpose of the change set.

If you look around the code base you'll find lots of code that violates 
the standard, but it is mostly old code. Sometimes we make a change set 
that is "formatting only" to straighten this out. If you're using 
Eclipse, there's a formatter on the wiki page. I'll admit to having 
tweaked mine slightly from this.

Definitely optional: the code as written creates a result in a 
StringBuilder (good), then performs quite a few String manipulations 
(case change, regex operation, reverse, concatenations). This 
post-tweaking adds up to a lot of String copying -- although you never 
know how the compiler may have optimised it at run-time. As your purpose 
was a speed-up, this might be a satisfying area to look at again, to try 
to build the whole result right the first time in one buffer. It might 
even clarify the logic. I'm a bit of an optimisation junkie, so 
definitely just a suggestion.

Finally, thanks for your contributions. We really appreciate them, event 
if we're as bit slow to respond. I hope you're finding it a satisfying 
experience.For me,  Jython is an endless source of interesting puzzles.

Jeff

Jeff Allen

On 15/12/2013 00:08, Santoso Wijaya wrote:
> Santoso Wijaya added the comment:
>
> Jeff,
>
> That's right. I built my patch for #2075 on top of this one.
>
> _______________________________________
> Jython tracker <report@bugs.jython.org>
> <http://bugs.jython.org/issue2013>
> _______________________________________
>
>