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

Added SuppressWarnings annotation #47

Closed
wants to merge 1 commit into from
Closed

Conversation

mondain
Copy link
Contributor

@mondain mondain commented Dec 3, 2014

Added @SuppressWarnings("unused") annotation to prevent dead code confusion regarding the StringUtils.isNullOrEmpty(eSelectedEndpointID) check done in the "if" block and within a "debug" block of code inside the previously mentioned "if" block. Lines 800 and 814.

Added @SuppressWarnings("unused") annotation to prevent dead code confusion regarding the StringUtils.isNullOrEmpty(eSelectedEndpointID) check done in the "if" block and within a "debug" block of code inside the previously mentioned "if" block. Lines 800 and 814.
@gpolitis
Copy link
Member

gpolitis commented Dec 3, 2014

Hey @mondain how did you manage to get the warning, I'm not seeing anything here.

@mondain
Copy link
Contributor Author

mondain commented Dec 3, 2014

It showed up in eclipse 3.8.1 on linux.

@ibauersachs
Copy link
Member

Eclipse 3.8 is rather old, but: if the code is actually unused it should be removed from the file or if it is indeed used (e.g. through reflection) then it should be marked as such. Simply adding a warning suppression without a justification is not the way to go.

@mondain
Copy link
Contributor Author

mondain commented Dec 4, 2014

The code block is not unused per se, but it is only called when debug logging is enabled; this is the full section of the code:

if (newEndpoint.getID().equals(eSelectedEndpointID)
                        || (SimulcastManager.SIMULCAST_LAYER_ORDER_INIT > SimulcastManager.SIMULCAST_LAYER_ORDER_LQ
                                && StringUtils.isNullOrEmpty(eSelectedEndpointID)))
                {
                    // somebody is watching the new endpoint or somebody has not
                    // yet signaled its selected endpoint to the bridge, start
                    // the hq stream.

                    if (logger.isDebugEnabled())
                    {
                        Map<String,Object> map = new HashMap<String,Object>(3);

                        map.put("e", e);
                        map.put("newEndpoint", newEndpoint);
                        map.put(
                                "maybe",
                                StringUtils.isNullOrEmpty(eSelectedEndpointID)
                                    ? "(maybe) "
                                    : "");

                        StringCompiler sc
                            = new StringCompiler(map)
                                .c("{e.id} is {maybe} watching {newEndpoint.id}.");

                        logger.debug(
                                sc.toString().replaceAll("\\s+", " "));
                    }

                    startHighQualityStream = true;
                    break;
                }

Observe that "StringUtils.isNullOrEmpty(eSelectedEndpointID)" is in both the "if" and "log" sections; it is what is being "unused".

@gpolitis
Copy link
Member

gpolitis commented Dec 4, 2014

Clearly the method that you have annotated as unused is not unused and it is not only called when debug logging is enabled. The only thing that's called only when debug logging is enabled is the debug message construction. I really don't see any unused code here, maybe there's a problem with the dead code detection facility in eclipse 3.8.1?

@mondain
Copy link
Contributor Author

mondain commented Dec 4, 2014

The unused annotation doesn't mean that the overall method is unused, it is covering the detected dead code section within; since the annotation is not valid for placement above the "if" block. I cannot comment on the detection in Eclipse, but I can say that the code is confusing and could be streamlined. This is not a critical item for sure, but more or less an annoyance within the IDE.

@gpolitis
Copy link
Member

gpolitis commented Dec 4, 2014

Putting an unused annotation doesn't make the code less confusing and there is no dead code anywhere in that method and the method is used. If you have any ideas on how to make the code less confusing please let us know.

@gpolitis gpolitis closed this Dec 4, 2014
@mondain
Copy link
Contributor Author

mondain commented Dec 4, 2014

True and fair enough; will do.

bbaldino pushed a commit to bbaldino/jitsi-videobridge that referenced this pull request Jan 22, 2020
* wip: FIR/PLI aggregation/translation

* converts a val into a const

* reduces nesting

* removes duplication

* avoid creating too many fir command sequence numbers

* simplifies code

* replaces Object with Any

* avoids extra work for plis

* adds rtcp feedback information to new PayloadType instances

* makes the keyframe request wait interval dynamic

* minor non-functional changes in keyframe requester
bbaldino pushed a commit to bbaldino/jitsi-videobridge that referenced this pull request Jan 22, 2020
bbaldino pushed a commit to bbaldino/jitsi-videobridge that referenced this pull request Sep 24, 2020
* wip: FIR/PLI aggregation/translation

* converts a val into a const

* reduces nesting

* removes duplication

* avoid creating too many fir command sequence numbers

* simplifies code

* replaces Object with Any

* avoids extra work for plis

* adds rtcp feedback information to new PayloadType instances

* makes the keyframe request wait interval dynamic

* minor non-functional changes in keyframe requester
JonathanLennox pushed a commit to JonathanLennox/jitsi-videobridge that referenced this pull request Jun 1, 2022
* wip: FIR/PLI aggregation/translation

* converts a val into a const

* reduces nesting

* removes duplication

* avoid creating too many fir command sequence numbers

* simplifies code

* replaces Object with Any

* avoids extra work for plis

* adds rtcp feedback information to new PayloadType instances

* makes the keyframe request wait interval dynamic

* minor non-functional changes in keyframe requester
This was referenced Jul 15, 2022
@bgrozev bgrozev mentioned this pull request Jul 25, 2022
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.

3 participants