Skip to content

Introduce abstract base classes for types #78

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

Closed
vladar opened this issue Dec 14, 2016 · 8 comments
Closed

Introduce abstract base classes for types #78

vladar opened this issue Dec 14, 2016 · 8 comments

Comments

@vladar
Copy link
Member

vladar commented Dec 14, 2016

The main goal of this (backwards-compatible) change is to provide better language-level support for type definitions in addition to ConfigBuilder #70 and use less Closures in code.

Example AbstractObjectType:

abstract class AbstractObjectType
{
    abstract public function getFields();

    abstract public function getInterfaces();

    abstract public function resolveField(
        $fieldName, 
        $sourceValue, 
        $args, 
        $context, 
        ResolveInfo $info
    );
}

AbstractInterfaceType:

abstract class AbstractInterfaceType
{
    abstract public function getFields();

    abstract public function resolveType($objectValue, $context, ResolveInfo $info);
}

Then we could use ConfigBuilder to actually define fields (or stick with arrays for those who prefers it).
It also enables serializable types as field resolution is separated from field definition.

This issue is pretty much created for discussion, so if anyone has any thoughts on the subject - you are very welcome.

@mcg-web
Copy link
Collaborator

mcg-web commented Dec 14, 2016

I love this idea but why not using interfaces instead of abstracts ?

@vladar
Copy link
Member Author

vladar commented Dec 14, 2016

@mcg-web We can use interfaces of course, but I suspect that type definitions will require some boilerplate code anyway. For example:

class AbstractObjectType 
{
    private $fields;

    public function getFields()
    {
        if (null === $this->fields) {
            $this->fields = [
                // ...
            ]; 
        }
        return $this->fields;
    }
}

We can automate it in abstract types.

Other thing is that naming is important :) ObjectType or InterfaceType names are already taken by existing implementation. And adding Interface suffix is something I'd like to avoid (just read InterfaceTypeInterface) %)

Other option is to put them in separate namespace, but then you will be annoyed by IDE autocomplete with identical names (Exception class names - I am looking at you!)

But this is all about brainstorming for now, so maybe we'll stick with interfaces in the end if we can address those issues somehow.

@mcg-web
Copy link
Collaborator

mcg-web commented Dec 14, 2016

Ok ok, anyway this will help to add flexibility to types definition(abstract or interface that not a big deal :-) )...

@tpetry
Copy link

tpetry commented Dec 20, 2016

Abstract would mean there could be no user defined root class :( Why not choose interfaces with boilerplate provided by traits?
Would be the most adaptable solution.

@vladar
Copy link
Member Author

vladar commented Dec 20, 2016

@tpetry Sounds like an argument %)

This is exactly the purpose of this issue - to discuss possible use-cases and ideas. Interfaces would work at the cost of more user-land code. So this is really about finding the right balance between ease of use and flexibility.

Can you share what traits would you use to reduce boilerplate? Maybe we can include some of them (or at least something similar) in the library so that users do not need to re-invent the same stuff all the time.

@tpetry
Copy link

tpetry commented Dec 20, 2016

I sm not in need of anyone. That was simply an argument to your position to favor an abstract class indtead of an interface because the abstract class could implement the (possible) boilerplate code someone had to do with an interface.

@fesor
Copy link

fesor commented Jan 2, 2018

@vladar you could have both abstract class, interface + traits or custom user-specific code. Abstract class could just implement interface + use traits. If for some reason users will want some other parent class, they could just use interface. Perfect balance from my point of view.

@vladar
Copy link
Member Author

vladar commented Nov 2, 2019

Closing stale issues. Open new follow-up issues if required.

@vladar vladar closed this as completed Nov 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants