Extending base methods on base types is bad practice

[tkelman]

This means other people's totally unrelated code changes behavior depending on whether or not your package is loaded. Generally if a method is imported from Base, you should only add methods to it if at least one of the arguments is of a type defined within this package.

[rened]

Thanks for you input!

I am aware of this. But this package is specifically designed to be quite "at the bottom" and actually change some of the behaviors of these base methods. No types are defined in FuntionalData, all behavior is meant for base types.

[tkelman]

Could you do one of:

1. Define the methods local to FunctionalData without importing them from Base. You could provide a macro that does local replacement to cut down on verbosity.
2. Use different names
3. If neither of the above, warn users in your documentation that this changes the behavior of base functions? This has gotten a nickname of "type piracy" and would result in likely discouragement by the developers of the language against using this package.

[rened]

One thing I did not mention: Care has been taken to only use type combinations which to not interfere with how the methods are defined in base, eg I define map(data, f), while Base's version is map(f, data), etc ...

Only in very few cases (like take) this does not work.

Thanks for your suggestions! (2) is tricky, as the functionality is mostly basic plumbing, and many simple, natural verbs (map) are taken and are hard to replace with something sensible.

I will look into (1), that might be possible. But doesn't this mean tons of name-clash warnings all the time?

And (3) is of course always good.

[rened]

By (1), do you mean a change from:

export map
import Base.map
map(a, f::Function) = .....

to

export map
map(a, f::Function) = .....
map(a...) = Base.map(a...)

I am just testing this and this does seem to work.

Is the differente that in the first case I change the behaviour for all loaded packages and in the second only for the ones that have using FunctionalData? (which is my intention, of course)

[tkelman]

Right. Though exporting it, as in the second case, may result in "both Base and FunctionalData export map, uses of it in ... need to be qualified" errors.

The macro suggestion would work by having @fctnl ... replace any instance it finds of map with FunctionalData.map which then wouldn't need it to be exported.

[rened]

I will look into this! Do you have an example / docs link for @fctnl please? I can't find any mention of it in the docs.

[tkelman]

That was just a proposed name, you'd have to implement such a macro by recursively walking the Expr, checking if any :call nodes are calling functions in a list of things you want to replace, and modifying the result.

[rened]

got it ;-)