-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Using Response constants instead of plain integers #31394
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
Comments
Well, in the core, we already rejecting multiple time using the constants rather than the HTTP codes themselves, as we find the code more readable (for people knowing their meaning in HTTP) than having to remember what each constant name represent as actual HTTP code. |
Btw, I don't see how using the constant would change anything for you in that case. The question you wanted to answer even asked for |
@gabiudrescu for reference, this was proposed several times in the past, but it was always rejected for the reasons given by @stof: |
thank you, guys, for your fast response. when I searched before for any other open issues with similar keywords, I couldn't find all the links mentioned above by @javiereguiluz. though, I must admit, I didn't check the closed ones. @stof I told the story of how I ended up opening this issue to give some context. basically, what I wanted to tell is that we started from searching our codebase from the constants from Response class and couldn't find them being used at all. searching for 301 or 302 in our codebase also retrieved a couple of results that were unrelated and then I started searching controller by controller. that's how I discovered that in vendor integers are used instead of constants - answering me the question "why aren't no 302 redirects based on the constant, either in src or in vendor". I can relate with your decision of using integers instead of constants for brevity and because you got used with the codes. But my suggestion to use constants instead was because I thought it would be easier for a developer to search where is a redirect code used by using the IDE function to find usages of a certain constant. That way, I could have answered to the question: where in our codebase we are using 301 redirects and where we are using 302 much easier - instead of relying on a integer search. But, the issue is very small and without much gain - if there is a consensus over using integers instead of constants for HTTP codes, fine by me - I will close the issue. |
well, in your own codebase, you can still use constants. That's why they have been added even though there are not used in core: giving choice to each project. |
cool. thanks for the feedback. |
Symfony version(s) affected: all
Description
Our (new) marketing team asked a pretty simple question: why are we using HTTP 302 redirect codes on almost all our redirects.
So I dived in the codebase and realized that we don't use
Response::HTTP_FOUND
or302
as integer at all. Puzzled, I started checking controller by controller and found out that we rely mostly onredirectToRoute
(https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php#L112) from Framework Bundle.Which internally is using a hardcoded integer instead of the constant from Response class (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Response.php#L37)
I wonder if there is any reason for using plain integers instead of constants.
Possible Solution
I would suggest replacing all hardcoded HTTP response codes with constants from Response class from HttpFoundation.
The text was updated successfully, but these errors were encountered: