Could `convertToURI` be implemented in parent class Extractor?

[fabiodrg]

I get it that not all fields in an extractor have to be encoded. But I wonder if it would be bad at all to encode everything and automatically in the Extractor class. Just iterate over the object keys and encode the values. Perhaps, we can also add some heuristics to determine whether we should encode the field or not, although I am not sure there's any benefit (in execution time).

The motivation to implement convertToURI in the Extractor class is to reduce implementation time and avoid errors in future extractor implementations. It results in more maintainable code as well. Do it once for all extractors. An extractor can always override the method for peculiar situations btw.

[msramalho]

I think this makes sense and indeed can be beneficial, however it should be done recursively since some objects have to encode properties of inner objects like this: event.subject.name = encodeURIComponent(event.subject.name);

Additionally, this would probably require a manual testing of each extractor, but will be beneficial in the future.

[fabiodrg]

Ah, recursion, that's a good point! I don't think we will have objects with depth inheritance chains, so the recursion shouldn't be a problem performance-wise. Iterating object properties have some quirks as well, which I don't recall, but documentation in MDN should cover that in detail. Some methods iterate properties defined by the object, but also propagate through the inheritance chain (upwards). That mixed with the recursive approach could end badly.. or not end at all :smile: . So, whoever implements this, be aware :)

[fabiodrg]

This issue doesn't seem relevant anymore since #86 . At the moment, the convertToURI method in the Extractor class has no use. I should create a PR to either mark the methods as deprecated or simply remove all related code.