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

Introduce LinkHelper #96

Merged
merged 4 commits into from Dec 9, 2013
Merged

Introduce LinkHelper #96

merged 4 commits into from Dec 9, 2013

Conversation

willdurand
Copy link
Owner

Get href values from configured links.

Useful to generate URLs based on configured relations.

Should fix #83 and #91

WDYT?

willdurand referenced this pull request Dec 8, 2013
Useful to generate URLs based on configured relations

Should fix #91
@adrienbrault
Copy link
Contributor

👍

@willdurand
Copy link
Owner Author

I will add a EL function that reuses such helper. It should fix the other issue.

@willdurand
Copy link
Owner Author

So, it needs more tests but it works.

@willdurand
Copy link
Owner Author

@adrienbrault I guess I'm done here.

Useful to generate URLs based on configured relations

Should fix #91
to extend the ExpressionLanguage with custom functions.

* Add 'link' function that relies on the LinkHelper class
* Add tests

Should fix #83
/**
* @author William Durand <william.durand1@gmail.com>
*/
class LinkHelper implements ExpressionFunctionInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you mix the helper and the ExpressionFunctionInterface ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because adding a proxy class that implements the interface is probably too much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so you only apply the SRP when it's not too much ?

Maybe the ExpressionFunctionInterface was too much and that it could have been done like in Symfony by extending the base ExpressionLanguage

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Injecting helpers into the extended EL class?

I dont think it violates SRP, it is a helper that provides one feature, exposed as a PHP method and as a EL function. Its responsability is to provide this feature. I could introduce a Hateoas\Expression\Function\Link class if you wish, but really it won't do much.

@adrienbrault
Copy link
Contributor

The EL function is pretty awesome ... because it is so annoying to repeat links everywhere!

👍

willdurand added a commit that referenced this pull request Dec 9, 2013
@willdurand willdurand merged commit 1b335f2 into master Dec 9, 2013
@willdurand willdurand deleted the url-generator-helper branch December 9, 2013 22:47
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

Successfully merging this pull request may close these issues.

Embedding and hrefs definition duplication
2 participants