8000 [HttpFoundation][Session] Added a convenient method to get AttributeBagInterface by gmponos · Pull Request #21251 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation][Session] Added a convenient method to get AttributeBagInterface #21251

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[HttpFoundation][Session] Added a convenient method to get AttributeBagInterface #21251

wants to merge 1 commit into from

Conversation

gmponos
Copy link
Contributor
@gmponos gmponos commented Jan 12, 2017 •
edited
Loading

Added a convenient method inside Session for getting internally the AttributeBagInterface. This way it also helps autocompletion in some IDE.

Q A
Branch? 3.2
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets no
License MIT
Doc PR n/a

@gmponos gmponos force-pushed the session_attribute_bag_method branch from b7ace51 to 6b3ec7a Compare January 12, 2017 19:54
@gmponos
Added a convenient method inside Session for getting internally the A…
6f533f7 
…ttributeBagInterface and have also autompletion
@gmponos gmponos force-pushed the session_attribute_bag_method branch from 6b3ec7a to 6f533f7 Compare January 12, 2017 20:02
@gmponos gmponos changed the base branch from master to 3.2 January 12, 2017 20:03
@gmponos gmponos changed the title Added a convenient method inside Session [HttpFoundation][Session] Added a convenient method to get AttributeBagInterface Jan 12, 2017
@xabbuh
Copy link
Member
xabbuh commented Jan 13, 2017

How does this help users of the HttpFoundation component given that it adds an additional method call?

@gmponos
Copy link
Contributor Author
gmponos commented Jan 14, 2017 •
edited
Loading

It does not help the user of the component. It's more of a

  • DocBlock matter. User can easily spot that he gets an AttributeBagInterface. When I visited the class to see internally how it works I had to spent some more time to figure out why does it call a function from an Interface that does not support it.
  • Offers better autocompletion in IDE.
  • If you ever change something you will need to change one line rather n lines.

As I said it does not offer something to the user. It's a matter of taste. I will understand if you don't like it.

Furthermore I was also sceptical if the method should be private or public.

@nicolas-grekas nicolas-grekas added this to the 3.x milestone Jan 16, 2017
@fabpot
Copy link
Member
fabpot commented Mar 1, 2017

Thank you @gmponos.

@fabpot fabpot closed this in 576ae1c Mar 1, 2017
@nicolas-grekas nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Enhancement HttpFoundation Status: Needs Review
Projects
None yet
Milestone
3.3
Development

Successfully merging this pull request may close these issues.
6 participants
@gmponos @xabbuh @fabpot @javiereguiluz @nicolas-grekas @carsonbot
0