shannah / Java-Objective-C-Bridge

A thin bridge that allows for two-way communication from Java to Objective-C.
123 stars 25 forks source link

Remove logging in native code #38

Open Marcono1234 opened 3 years ago

Marcono1234 commented 3 years ago

The native code uses logging (NSLog) quite extensively. I am not familiar with Objective-C, but the documentation for this function says:

Logs an error message to the Apple System Log facility.

However, it is currently used for debug logging as well. And it is used in cases where a Java exception is thrown anyways so logging seems redundant (instead the exception message should be improved, if necessary).

Therefore I would recommend completely removing logging.

Note: It appears the build process was slightly incorrect, using an outdated libjcocoa.dylib, see https://github.com/shannah/Java-Objective-C-Bridge/pull/35/commits/8266cd96e04299c119b576e4df584bc202058115. Therefore you might not have seen logging output yet.

mojo2012 commented 3 years ago

Removing logging doesn't seem like a good idea. Why would you want that? It helps you to follow the flow in the native code (can't really debug it or at least nobody knows how to)

Marcono1234 commented 3 years ago

Ideally the project would be stable enough at some point to not require any logging at all, or where appropriate detailed exceptions are thrown. Log ouput might also reveal sensitive information, it would be better to let the user choose how they want to handle exceptions and what (if any) they want to log.

Additionally depending on your setup you might not even be able to read the log if you run this library without capturing the standard output/error stream.

Currently with these log statements the build log looks like this: Build log screenshot (see https://github.com/shannah/Java-Objective-C-Bridge/runs/1678219378) I assume it does not look much better if you would run it in production.

shannah commented 3 years ago

I agree. That's too verbose. But I also agree that it can be nice to have verbose logging like this during debugging. Perhaps a setDebugMode(boolean) to enable/disable this logging would be best.