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

Optional explicit interface implementations #22020

Closed
2 tasks
mindplay-dk opened this issue Aug 10, 2024 · 12 comments
Closed
2 tasks

Optional explicit interface implementations #22020

mindplay-dk opened this issue Aug 10, 2024 · 12 comments
Labels
Feature Request This issue is made to request a feature.

Comments

@mindplay-dk
Copy link
Contributor

mindplay-dk commented Aug 10, 2024

Describe the feature

Just to be clear, I'm aware this has been requested before:

But it doesn't seem like this was ever discussed at length - and no in-depth reasoning was presented, explaining why this feature should be considered, so I will try to do that.

To be clear, what I'm proposing is an optional explicit implements keyword, which would apply to struct declarations.

What I'm proposing is this:

struct MyStruct implements IFirst, ISecond {
    first string
    second string
}

interface IFirst {
    firts string
}

interface ISecond {
    second string
}

(Alternatively, implements could be abbreviated as : if the terser syntax feels more like V.)

The implements declaration would be checked locally - that is, this declaration will first validate that MyStruct actually implements all the fields and methods in each of the IFirst and ISecond interfaces, before validating or reporting errors about invalid arguments in call sites, or invalid assignments, etc.

Of course, we would prefer not to write explicit implementations everywhere, because it's convenient, and because it does work well most of the time - to be clear, I very much like the approach taken interfaces by Go and TypeScript, it's definitely preferable most of the time, just not all of the time. TypeScript doesn't strictly need this feature either, but it has this feature, because there are times when being clear and explicit is objectively better for clarity, as well as for error reporting.

Use Case

There are two reasons why this should be given proper consideration.

1. Code readability

This example from the documentation demonstrates a code readability problem:

struct PathError {
    Error
    path string
}

fn (err PathError) msg() string {
    return 'Failed to open path: ${err.path}'
}

fn try_open(path string) ! {
    // V automatically casts this to IError 👈🤔
    return PathError{
        path: path
    }
}

fn main() {
    try_open('/tmp') or { panic(err) }
}

The example actually highlights the problem with the comment "V automatically casts this to IError" - I understand this comment was added to the manual to explain what's happening, but it's not unthinkable someone would actually put a comment like this in their code, since it is not clear from reading the code.

There is nothing in the code explains:

  1. Why or how PathError is converted to IError.
  2. Why the fn (err PathError) msg() string method is actually present.

With an explicit implementation, the example is clear:

struct PathError implements IError { 👈
    Error
    path string
}

fn (err PathError) msg() string {
    return 'Failed to open path: ${err.path}'
}

fn try_open(path string) ! {
    return PathError{
        path: path
    }
}

fn main() {
    try_open('/tmp') or { panic(err) }
}

It's now clear that the PathError declaration is intended to implement IError - we don't need the comment in try_open to explain what's going on, and there is no mystery as to why the msg method exists, which previously looked like it might have been an unused method.

2. Error Reporting

From the example above, also consider the fact that error reporting can now report the error where the error is - for example, if you were to omit the msg method, you would get an error saying the msg method is required to correctly implement the IError interface. In other words, implements IError explains our intent to implement that interface, which the reader (and compiler) could otherwise only assume based on usage.

Let's consider the interfaces example from the documentation:

struct Dog implements Speaker {
    breed string
}

fn (d Dog) speak() string {
    return 'woof'
}

struct Cat implements Speaker {
    breed string
}

fn (c Cat) speak() string {
    return 'meow'
}

interface Speaker {
    breed string
    speak() string
}

Here, the addition of implements makes it clear that there is an intended relationship between Cat and Dog and the Speaker interface - you know this in advance when you see the first declaration. Contrast this with the example in the manual, where no relationship is established unless you read through the calling code, which I've intentionally omitted from the updated example here - there is no saying the calling code for an interface is located in the same file.

In terms of error reporting, we can now point to Dog or Cat as having a problem, if the required methods and fields aren't present, or have the wrong type, etc. - rather than pointing at many call sites having potential problems, we can now point at a single declaration as definitely having a problem, e.g. "missing field" or "missing method".

One, clear error versus many potential errors.

Proposed Solution

No response

Other Information

This feature could help with modularity and reuse as well.

For example, consider the example presented here, where you need to implement a Context type:

pub struct Context {
    veb.Context
pub mut:
    // In the context struct we store data that could be different
    // for each request. Like a User struct or a session id
    user       User
    session_id string
}

There is no problem with this example per se, but consider the case where I'd like to create a module that provides a reusable middleware, which will establish a session_id - in order to work with your custom context, this middleware would need you to add the session_id string to your Context type, and this would need to be part of an interface.

If you see the declaration above, you would have no idea why the session_id is there, and a pub struct Context implements ISessionContext declaration would help make that relationship clear and explicit - it explains why you would expect this Context implementation to work with the session middleware, and ensures we point to the source of that problem.

If some day we have an ecosystem of reusable middleware, it would be helpful to understand from someone's Context declaration how they intended for it to work with the integrated middleware components.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

Version used

0.4.7

Environment details (OS name and version, etc.)

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@mindplay-dk mindplay-dk added the Feature Request This issue is made to request a feature. label Aug 10, 2024
@Cergoo
Copy link

Cergoo commented Aug 10, 2024

This topic was probably widely discussed during the development of golang.
see like https://www.reddit.com/r/golang/comments/vccs39/implicit_vs_explicit_interfaces/

use comments for Code readability

// PathError implement IError
struct PathError {
    Error
    path string
}

@mindplay-dk
Copy link
Contributor Author

mindplay-dk commented Aug 10, 2024

This topic was probably widely discussed during the development of golang.

"probably" seems like an easy way to dodge the effort of giving this any serious consideration.

see like https://www.reddit.com/r/golang/comments/vccs39/implicit_vs_explicit_interfaces/

Well, there is nothing really surprising in this thread - there are arguments made both for and against implicit and explicit interface implementations, but it doesn't seem like anyone is really arguing against having both?

TypeScript gets praised here for having "good balance" by providing both - explicit interfaces when strong contracts are required, and implicit interfaces when flexibility is needed.

It is true that explicit interfaces can create "temporal dependency", and yes, sometimes that is a mistake - at other times, strong coupling is what you want, because a breaking change is what you want. The IError interface is an example of this - if this were to change, suddenly none of your error implementations are valid anymore, and there's nothing to even indicate that these were supposed to be IError implementations. (except the name or a comment, but that's based on a weak convention, which may not always exist or be possible - or which the reader may simply not know about.)

Regarding premature abstraction, those aren't really the examples I'm citing - in some cases, there are well known abstractions, and you want to make sure those constraints hold. In other words, a breaking change is what you want.

I'm not advocating for Java, nor for the type of large complex frameworks with tons of contracts you see in that ecosystem.

Having explicit interfaces implementation as an option will not created reduced flexibility, or make it harder to adapt existing code to new use cases. It's optional - you wouldn't use it except in those cases where coupling to an interface is wanted. It won't create unnecessary coupling between packages - again, because it's optional, it's up to you how much coupling you want.

Coupling isn't automatically "bad", it depends on the context - unnecessary coupling is bad, and we're seeing a lot of that in e.g. Java and C#, but I don't feel like this has been a problem in TypeScript, where both implicit and explicit interfaces are available. Personally, I rely heavily on implicit interfaces in TypeScript - in fact, I don't think the standard libraries/types for DOM/BOM/Node would be usable at all without implicit interface checks.

I see a lot of solid arguments in this thread against having explicit interfaces only.

But the closest the discussion came to the topic of having both, was one user mentioning TypeScript's approach, which allows optional explicit implementation of interfaces - but this was presented more as an observation rather than an argument for or against having both.

If you disagree, can you point out a specific argument?

use comments for Code readability

Comments aren't a good substitute for code - I would prefer to have something that can actually be checked. Comments get out of sync with the code, and often end up doing more harm than good.

@spytheman
Copy link
Member

@mindplay-dk thank you for the extremely clear explanation of the benefits of that feature 🙇🏻‍♂️ .

You are absolute right that it was proposed many times before, but afaik so far, no one has managed to provide so many convincing arguments for it.

@spytheman
Copy link
Member

Personally, I am all for it, as long as it is optional, for all the stated reasons.

@medvednikov
Copy link
Member

Thanks for the great write-up @mindplay-dk

I agree with you.

https://x.com/v_language/status/1830991393314972115

@penguindark
Copy link
Member

What are the implications for the compiler?
I'm ok with it but if it is only optional.
Only another pure personal note is about use interface for reusability, have seen what's happened to Java and C++ I'm not so sure that this is the right way to do.

@medvednikov
Copy link
Member

Yes, optional.

Interfaces will be declared the same way, and nothing will change. It won't be like Java and C++, it's just a hint that the developer gives to the compiler and to the reader.

@penguindark
Copy link
Member

If they remains optional and don't have too much impact on the compiler for me they are OK! :)

@JalonSolov
Copy link
Contributor

JalonSolov commented Sep 4, 2024

The implements keyword was added earlier today in commit #0090170

@medvednikov
Copy link
Member

Only took 12 minutes to implement. V compiler is amazing.

@mindplay-dk
Copy link
Contributor Author

whoa! 🤯😄

you guys needed a lot of persuasion, but dang you don't waste any time when you've decided!

thank you for taking this seriously and acting so fast! that was wild. 😄

@medvednikov
Copy link
Member

medvednikov commented Sep 4, 2024

Yeah :) I still need to push a fix that allows implementing multiple interfaces, and removes the new keyword. A simple ident can be used here, since it's the only place it's used in.

The fewer keywords, the better.

Will do that today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request This issue is made to request a feature.
Projects
None yet
Development

No branches or pull requests

6 participants