Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some constructors return the "wrong" type #649

Open
ExpandingMan opened this issue Mar 13, 2024 · 1 comment
Open

some constructors return the "wrong" type #649

ExpandingMan opened this issue Mar 13, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@ExpandingMan
Copy link

ExpandingMan commented Mar 13, 2024

This may not be considered a "bug", but it's a design pattern that I think is undesirable from an API standpoint.

There are some places in this package in which a function that "looks like" a constructor, in that it follows the Julia convention of having a camel-case and having the same name as an existing type, returns an object of a completely different type. The examples I have run into are in constructors for various types of ODEProblem which can be found here. There may be other examples of this, but I haven't gone through the package exhaustively.

While obviously Julia does not strictly enforce this convention for constructors, I would argue that it should and, at the very least, the results can be very confusing. For example, I discovered this issue when code I wrote was resulting in method errors because I had written methods for DynamicalODEProblem rather than ODEProblem because I was calling the function DynamicalODEProblem to create the object and I could see that this was indeed a real type (I would argue that if DynamicalODEProblem did not exist this function should be renamed so as not to have the misleading camel-case).

@ExpandingMan ExpandingMan added the bug Something isn't working label Mar 13, 2024
@ChrisRackauckas
Copy link
Member

Yeah, it's an odd design decision I made back in 2016. I don't know if I agree with it now, but it would be breaking to move away from it unless we make a DynamicalODEProblem type, which is not off the table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants