Using SLF4J and refactored code

[marci4]

Description

Most of the breaking changes in #753 are included
Refactored a lot of code

Related Issue

Fixes #670

How Has This Been Tested?

All tests still work!

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[PhilipRoman]

There are a lot of places where parameterized logging should be used. I'm sure there are more, but here are some examples:

log.warn("Removing connection which is not in the connections collection! Possible no handshake recieved! " + ws);

should become

log.warn("Removing connection which is not in the connections collection! Possible no handshake recieved! {}", ws);

This avoids string appending when the log statement is disabled. HotSpot can do that during its optimizations but Android is not so clever 🐌

There are a few places where it could be worth adding a check: if(log.isTraceEnabled())...
Like here:

if (inData.remaining() != 0) {
    log.trace(new String( inData.array(), inData.position(), inData.remaining()));
}

Basically, even if I have disabled the TRACE level, every time this code is executed, the String object is created (parameterized logging won't help here). I don't know how large this String could be or how often this logging statement is activated, but if there is any chance that the size is large, it would be better to avoid creating it unless TRACE is enabled. Also, maybe log a message along the string.

In src/main/java/org/java_websocket/protocols/Protocol.java there are two precompiled patterns named COMPILE and PATTERN. They should be given more descriptive names.

[marci4]

Hello @PhilipRoman,

thank you for taking a look at the changes.
Hope I got everything!

Best regards,
marci4

[PhilipRoman]

Sorry for being pedantic! Can't test it on mobile but I think that there shouldnt be a parameter in this statement (src/main/java/org/java_websocket/AbstractWebSocket.java):

log.error("Exception during connection lost restart: {}", e);

I believe this call uses the method

error(String, Throwable)

which is supposed to log a message and stack trace, instead of treating the throwable as a parameter. This shouldn't affect anything important so feel free to merge if you want! 👍

[marci4]

@PhilipRoman got everything. Thank you for pointing that out!