Documented and refactored #2
Conversation
| public final ReactApplicationContext context; | ||
|
|
||
| private BarcodeScannerThread scannerthread = null; | ||
|
|
||
| /* | ||
| * Constructor | ||
| * The OS is checked for Zebra/Motorola Enterprise features before creating the scanner thread. |
There was a problem hiding this comment.
It may be good to explain what happens if this check returns false.
There was a problem hiding this comment.
Nothing functional happens if the check is false. The scanner thread is just never created and app runs, creates the GUI and all that, but can't make the calls to the scanner because it doesn't exist physically or software-wise.
| } | ||
|
|
||
|
|
||
| /* | ||
| * Allows this class to be accessed in the React Native application via `React.NativeModules.BarcodeScannerManager' |
There was a problem hiding this comment.
This comment seems highly contextualized. Really, this method just returns a string that represents the name of the class. Is it important for us to know here the way in which React Native utilizes this method?
There was a problem hiding this comment.
I thought the context would be helpful to someone trying to figure out the how and why. It wasn't obvious to me why this was important at first, so I included the context. This method also would not exist here, except that ReactContextBaseJavaModule requires it to be overridden. It's not a normal Java or Android thing.
| @Override | ||
| public void onCatalystInstanceDestroy() { | ||
| if (this.scannerthread != null) { | ||
| this.scannerthread.onCatalystInstanceDestroy(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Helper method for getting the EMDKManager object by registering this class as a listener callback. |
There was a problem hiding this comment.
I'm not a huge fan of this implementation (totally not your fault). I think it would be useful if it threw an exception in the constructor if the required manufacturer is not found. Then none of these methods would require this check.
if (this.scannerthread != null)
As it stands if the required manufacturer is not present everything just fails silently. It might be good to call out the fact that this method has no effect on devices that aren't made by the required manufacturers. Or maybe just make that extra clear in your comment at the top i.e. instead of
The OS is checked for Zebra/Motorola Enterprise features before creating the scanner thread.
The OS is checked for Zebra/Motorola Enterprise features before creating the scanner thread.
If the OS lacks these required features, the instance returned by this constructor will perform
a NOOP for all method calls.
| @Override | ||
| public void onCatalystInstanceDestroy() { | ||
| if (this.scannerthread != null) { | ||
| this.scannerthread.onCatalystInstanceDestroy(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Helper method for getting the EMDKManager object by registering this class as a listener callback. | ||
| * This allows this application to have acces to EMDK features. |
| /** | ||
| * Helper method for using the hardware button to trigger the scanner. | ||
| * | ||
| * @param condig Scanner configuration passed from React Native app. |
There was a problem hiding this comment.
Should this only be specific to a React Native app caller? Also, what are the possible options for this configuration?
There was a problem hiding this comment.
The whole point of this repo is that there are methods exposed for a React Native caller. If we were writing a Java app to use the EMDK, then we would just use the API that Zebra provides. If you wanted to call read() from a Java context, you can just call it from the scannerThread.
The ScannerConfig object, which this ReadableMap should be emulating is a bit complex, but I can certainly add a link to Zebra's docs about it. https://techdocs.zebra.com/emdk-for-android/5-0/api/barcode/ScannerConfig/
android/src/main/java/nl/kega/reactnativeemdk/BarcodeScannerThread.java
Outdated
Show resolved
Hide resolved
| * This method should be used when only using a software button to | ||
| * trigger the scanner. | ||
| * | ||
| * @param condig Scanner configuration passed from React Native app. |
There was a problem hiding this comment.
What are the possible values for the configuration and is the React Native app the only caller type for this method?
There was a problem hiding this comment.
android/src/main/java/nl/kega/reactnativeemdk/BarcodeScannerThread.java
Outdated
Show resolved
Hide resolved
android/src/main/java/nl/kega/reactnativeemdk/BarcodeScannerThread.java
Outdated
Show resolved
Hide resolved
android/src/main/java/nl/kega/reactnativeemdk/BarcodeScannerThread.java
Outdated
Show resolved
Hide resolved
This is just adding Javadoc blocks above the methods and pulling out some duplicate code into it's own method, BarcodeScannerThread.configureScanner().