[TW5] Wrote a new filter operator; please review and see what you think?

[Antaeus Feldspar]

Hi folks! I'm still working on the button-that-reads-from-a-field-and-writes-back-a-modified-value, but in the meantime, I also had need for a filter operator, and that one ended up getting completed first.

The name of it is "datebefore". For each tiddler passed as input, the filter checks the date in the tiddler's "created" field, or a different field if specified by the suffix, and passes it through if it is before the date given as a parameter. The ! prefix can reverse its meaning.

The filter can be found at http://afeldspar.tiddlyspot.com/#%24%3A%2Ffilters%2Fafeldspar%2Fdatebefore.js . Please let me know what you think. I'm also thinking that I might write a "how to write a filter operator" document based on what I learned to make this one, but I'm not sure where it should go after that, and it should definitely be reviewed by someone who actually understands why, for instance, that "/* jslint node: true, browser: true */" is there.

[Antaeus Feldspar]

Anyone? I'd kind of like to know if I've made any big mistakes.

[Felix Küppers]

Hi Antaeus,

this looks good on the first sight. Your datebefore filter is a nice idea similar (but not the same) to the "days" filter created by Rustem (https://github.com/Jermolene/TiddlyWiki5/pull/1909).

Correct me if I am wrong, but it is actually not a date comparison but a string comparison what you are doing. This is fine when dates are saved in a string format that allows lexicographical comparison as it is usually the case for TW.

I would not recommend to do .toLowerCase() as field names are not stored uppercase per convention and if a fieldname were passed in uppercase, it would not really help making it lowercase anyway.

One last thing: Your negation function equals 99% the normal filter call (without "!"), the only difference being the "<" vs ">=". I know many people wrote their filter functions this way but this creates rendundant code. Would be better to simply use something like this:


// cache variables
var isNegated = (operator.prefix === "!");
var operand = operator.operand
 
and later only use:

 source(function(tiddler,title) {
		    if (tiddler) {
                var tiddate = tiddler.getFieldString(fieldname);
                if (isNegated ? tiddate >= operand : tiddate < operand) {
                    results.push(title);
                }
            }
	    });

Hope this is enough feedback :)

-Felix

[Tobias Beer]

Hi Felix,

 

I know many people wrote their filter functions this way but this creates rendundant code. Would be better to simply use something like this:

Never seen or generated any performance test results myself, however:

Jeremy indicatd it would be (significantly?) more performant without the extra conditional.

Best wishes,

— tb 

[Felix Küppers]

Hi Tobias.

Never seen or generated any performance test results myself however:

Jeremy claims it will be significantly more performant without the extra conditional.

IMO avoiding redundancy is worth more than the minimal speed increment that results from a single comparison step. On this level one could also argue that it is not optimal to use ".forEach()" instead of a plain js for-loop since it will execute a function on every iteration step (in this case the performance drawback can be reduced by the optimizing compiler though, but this depends on the browser and whether the api-user provided callback function can be optimized).

Also what could be done to increment the speed of filters is to cache all object properties like e.g. "operator.operand.length" and access them as variables in the iterator function.

-Felix

[Jeremy Ruston]

Hi Felix

IMO avoiding redundancy is worth more than the minimal speed increment that results from a single comparison step.

I think filter operators are one place that is absolutely worth the effort to do some basic optimisatoin. Filter operators are called repeatedly during filter processing; for users with 10,000 tiddlers then even a modest improvement in filter performance is worth it.

On this level one could also argue that it is not optimal to use ".forEach()" instead of a plain js for-loop since it will execute a function on every iteration step (in this case the performance drawback can be reduced by the optimizing compiler though, but this depends on the browser and whether the api-user provided callback function can be optimized).

That’s pretty much exactly why we have $tw.utils.each. See the following for an interesting perspective:

https://github.com/codemix/fast.js/tree/master

Also what could be done to increment the speed of filters is to cache all object properties like e.g. "operator.operand.length" and access them as variables in the iterator function.

Definitely worth investigating.

Best wishes

Jeremy.

-Felix

--
You received this message because you are subscribed to the Google Groups "TiddlyWikiDev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to tiddlywikide...@googlegroups.com.
To post to this group, send email to tiddly...@googlegroups.com.
Visit this group at http://groups.google.com/group/tiddlywikidev.
To view this discussion on the web visit https://groups.google.com/d/msgid/tiddlywikidev/BLU436-SMTP127B8EEA7905F12B83198CECE300%40phx.gbl.
For more options, visit https://groups.google.com/d/optout.

[Felix Küppers]

Hi Jeremy

> I think filter operators are one place that is absolutely worth the
> effort to do some basic optimisatoin. Filter operators are
> called repeatedly during filter processing; for users with 10,000
> tiddlers then even a modest improvement in filter performance is worth it.

Thinking about it, I agree with you. Sometimes I just forget that TW can
take care of thousands of Tiddlers :) It's just a great piece of software!

> That’s pretty much exactly why we have $tw.utils.each. See the
> following for an interesting perspective:

Thanks for the link.

-Felix