Code Kukarektoring - Githubissues

AlexDaniel commented 9 years ago

Kukarek!

Ky6uk commented 9 years ago

Two whitespaces instead four;
"else if" on same line of block ending;
Silly comments (yes, I am absolutely serious);
Don't support oldest Perl;
Theoretically will eat much memory and work slowly by regexes and using more line buffers;
Special characters limitation;

So sorry, but I can't merge.

AlexDaniel commented 9 years ago

What?

Since I have changed every line of this code, and since there was no code style whatsoever, I decided that I have a golden opportunity to change the style to something that makes sense. I have made another commit that changes it back to your liking.
Same as above.
This is not an issue, because there were no error messages at all. Feel free to change them!
Doesn't support perl 5.8? Then check out this. Here is a much more readable quote from here:

In Perl 5.13 (the development track for Perl 5.14), the experimental feature called "autoderef" appeared.

Autoderef was used in the original code, which means that the whole code has >5.14 restriction. I've just specified v5.10 explicitly, which is a good choice anyway. People on wikipedia say that versions older than 5.18 are not really supported. Perl 5.14 is in debian wheezy, 5.10.1 is in squeeze. Considering the approximate chicken life expectancy of 6 years, this is not an issue. There is just no reason to support 5.8. And if you want do that, then just change given-when (which was added in 5.10) to if-elsif series, but don't forget about autoderef since it has a higher restriction.

So you think that this (/\G(Ko|kO|Kudah|kudah|Kukarek)/gc) will require more memory than five elsifs with a lot of string concatenations? Well, I don't know what to say here...
"Special characters limitation" - what kind of limitation?

Ky6uk commented 9 years ago

Plz listen a some words about memory usage as I see it.

You are using readline-syntax for reading some piece of input. So you always read one line per cycle and store this line into memory. DIfferently my code is read only one character and store into memory only minimally needed data all the time.

Additionally I have the side-effect - possibility for controls all input characters. So I can ignore all of crap independently of crap types.

Yunoo commented 9 years ago

Ky6uk, those are just micro optimizations, so you just can ignore them.

AlexDaniel commented 9 years ago

@Ky6uk Indeed, it seems like you are making up imaginary performance problems.

I've done some tests! At first glance my version performs about twice slower than yours on small files, however it seems like this extra time is spent on given-when. I don't know why yet. If I replace it with simple elsifs, then it is a tad faster or at least the same speed. I will provide more info next week, commits will appear in pull request #12 .

AlexDaniel commented 9 years ago

So, I was rather busy lately... The current version in https://github.com/Ky6uk/PETOOH/pull/12 performs a little bit better than your code (about 1 ms faster when running your test case). Of course, this is not enough to measure the actual performance difference. I hoped that I would have some time to write a bigger test case so that the difference would be obvious, but it seems like I will not do it ever. However, I am pretty sure that the pull request is good enough just for the refactoring itself, and I am surprised that you fail to see the beauty of the refactored code.

Also, the initial performance uhm problem uhm was due to the 'use experimental'. I have removed the given-when statement, as well as some "silly comments". I think that it is ready to go.

If there are any other style questions you don't agree with, then just fix it yourself. The point of my work was not to popularize adequate code style, but to fix redundancy in the code. So feel free to change it any way you like after you pull it.

Ky6uk / PETOOH

Code Kukarektoring #7