[jitsi-dev] [jitsi-videobridge] Added SuppressWarnings annotation (#47)


#1

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.
You can merge this Pull Request by running:

  git pull https://github.com/mondain/jitsi-videobridge patch-5

Or you can view, comment on it, or merge it online at:

  https://github.com/jitsi/jitsi-videobridge/pull/47

-- Commit Summary --

  * Added SuppressWarnings annotation

-- File Changes --

    M src/org/jitsi/videobridge/simulcast/SimulcastReceiver.java (1)

-- Patch Links --

https://github.com/jitsi/jitsi-videobridge/pull/47.patch
https://github.com/jitsi/jitsi-videobridge/pull/47.diff

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47


#2

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

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65459509


#3

It showed up in eclipse 3.8.1 on linux.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65468713


#4

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.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65549498


#5

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".

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65624549


#6

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?

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65625670


#7

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.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65638207


#8

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.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65638742


#9

Closed #47.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#event-202646091


#10

True and fair enough; will do.

···

---
Reply to this email directly or view it on GitHub:
https://github.com/jitsi/jitsi-videobridge/pull/47#issuecomment-65639174