imbrn/v8n

Simplify built in rule declarations

sbarfurth opened this issue · 5 comments

Right now the built in rule declarations are an absolute code mess. There are many complex helper function chains and some of them are done in line. I would argue the best approach is to define them all in line, since most of them can be simple one-liners. This would clean the code-base a lot.

The output size would need to be compared to the current, since there would be some duplicate code. But to be fair, some of the helper functions have very complex implementations and if checks that can be omitted when writing inline.

This would cause no change in any API and could be part of 1.2.0.

imbrn commented

I agree with you that we need to do some refactoring to make code cleaner, but we must still strive to reuse logic between similar rules, but I think we could do that in a cleaner way though.

Should we though? It's hardly shorter to use a testPattern helper than to use .test on a RegExp, in fact a function is probably a bigger footprint. Just as an example.

imbrn commented

You're absolutely right. There are some rules we need to refactor indeed. And maybe testPattern should be one of them.

Can't think if a space efficient way though. There are a few things that probably require some sort of refactoring. For example the _test and _check methods on Context are awful to read and understand, especially with those double function calls.

I feel like they need to be documented line by line also, until a nicer way to write them can be found.

Regardless, I'll try to think of a better way to declare rules. Right now it seems that for rules without arguments it should be possible to be omit the first part of the return. They all just repeat () => value => ... which seems dumb considering there aren't arguments but there is boilerplate for them.

Just a few thoughts though, I have no idea about implementing any improvements yet, I just feel that it's necessary.

imbrn commented

The Rule behavior has a very complex logic, so its implementation is not something easy to refactor due its recursive nature. And the recursive approach is absolutely needed. I thought a lot about this implementation and that's definitely the best way to do that. I think I can split the process into some more steps for trying to clean it a little.

About the rules declaration, If you have some idea about how we can clean it, you can make a PR and we can discuss it better with code. But I don't think that's something you can do easily, but maybe you have some smart idea. That'll be useful.