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

A more helpful way of creating EAV attributes #4266

Closed
adragus-inviqa opened this issue Apr 22, 2016 · 5 comments
Closed

A more helpful way of creating EAV attributes #4266

adragus-inviqa opened this issue Apr 22, 2016 · 5 comments

Comments

@adragus-inviqa
Copy link
Contributor

adragus-inviqa commented Apr 22, 2016

Hi. Me again.

I was wondering if this would be ok for a future PR: https://github.com/adragus-inviqa/m2-eav-attribute-configurator.

It would finally provide us with some OOP sweetness in an important part of our day-to-day work: adding EAV attributes. I can't tell how many times I've got those wrong. It's about time the IDE would help with that.

The addition is BC-friendly and can be used with the old code as well. But, for an even more future-proof code, I'd rather get rid of the interfaces, so additional methods can be added without breaking some contract. That is, if those configurations are to be changed at some point, which I think they probably will.

@Vinai
Copy link
Contributor

Vinai commented Apr 22, 2016

Nice idea!
Just as an idea, what do you think about adding type methods to get rid of the fixed string constants?
Something like

$attributeConfig = $this->productAttributeConfigurationFactory
    ->create()
    ->unique()
    ->usedInGrid()
    ->useFrontendInputText()
    ->withStoreScope()
    ....

The product types, backend types and additional options are too generic, but the value scopes and the input types are pretty fixed. There still could be a generic method if something different is needed, too.

@adragus-inviqa
Copy link
Contributor Author

Sure, why not. We should make this as friendly as possible.

Still, I'm thinking that the input types are too many to make each of them a separate method, however much I would like that.

@Vinai
Copy link
Contributor

Vinai commented Apr 22, 2016

It would be so easy to see input types are available, I think it would be perfect - self-documenting code :)

@adragus-inviqa
Copy link
Contributor Author

adragus-inviqa commented Apr 22, 2016

I agree, but I still think they're too many methods. This increases the chances of the interface changing whenever you add a new input. Not very likely, but, when it does happen, it'll break the contract. Unless I ditch the interfaces and just use a concrete class, which I'm starting to think it's a good idea.

Also, isn't that the same case with backend types? Aka a standard set which can be added as methods.

@piotrekkaminski
Copy link
Contributor

Closing in favor of the referenced PR

magento-engcom-team pushed a commit to okorshenko/magento2 that referenced this issue May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants