-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Missing flags getter added #10000
Missing flags getter added #10000
Conversation
Signed-off-by: Jan Peter Stotz <jpstotz@gmx.de>
I'm not sure I understand in what scenario would the getter help? You would have to get the You typically don't need this because in the right places you look only for certain flags. |
@sbordet I need to have access to the raw flags value. At the moment I have to use reflection to do so just to get it work. Therefore having a getter in place is IMHO always a good idea. I don't see a point here why HeaderParser should hide the |
Why?
There is no advantage to expose it that you have articulated, besides "I have to access it". The flags are used as a bitmap, so it is very easy to get confused and write code such as |
@sbordet For example I want to print the flags value in it's original form. Just hiding a field because developers exists who does not understand how bitwise flags work is IMHO a bad decision. In my opinion a better way to prevent an API from being used in a wrong way is by adding a detailed description. |
@sbordet The information is already available via return
(p.hasFlag(0x01) ? 0x01 : 0) |
(p.hasFlag(0x02) ? 0x02 : 0) |
(p.hasFlag(0x04) ? 0x04 : 0) |
(p.hasFlag(0x08) ? 0x08 : 0) |
(p.hasFlag(0x80) ? 0x10 : 0) |
(p.hasFlag(0x80) ? 0x20 : 0) |
(p.hasFlag(0x80) ? 0x40 : 0) |
(p.hasFlag(0x80) ? 0x80 : 0) ; So we are not hiding the data and if there is a user that wants them collectively, then I see no harm providing that. @jpstotz javadoc would be helpful, specially as it could have a reference to the |
@sbordet nudge, I'm not seeing the issue with a simple getter like this? |
The HeaderParser class is pretty handy when using
org.eclipse.jetty.http2:http2-common
as a library in other projects. Unfortunately it misses a getter to retrieve the flags and there are scenarios where you don't want to callhasFlag
for every possible flag...