search

AltBeacon / android-beacon-library

Allows Android apps to interact with BLE beacons
Apache License 2.0
2.83k stars 836 forks source link

java.lang.IndexOutOfBoundsException from Beacon.getId3() and Eddystone #448

Open ghost opened 7 years ago

ghost commented 7 years ago

Expected behavior

If application encounters an Eddystone beacon, it will only have two IDs and thus id3 in a beacon object will not contain a value. Calling getId3, which is supposed to be a convenience method, should behave nicely in this situation, ideally returning an object whose value means "not present" as opposed to simply returning null. Then calling methods like toHexString on such an Identifier object could return an empty String.

Actual behavior

If you call getId3() on a Beacon object created for an Eddystone beacon you get an IndexOutOfBoundsException like this:

java.lang.IndexOutOfBoundsException: Invalid index 2, size is 2
at java.util.ArrayList.throwIndexOutOfBoundsException(ArrayList.java:255)
at java.util.ArrayList.get(ArrayList.java:308)
at org.altbeacon.beacon.Beacon.getId3(Beacon.java:337)

Steps to reproduce this behavior

Define a Region with Identifier objects set to null so that all beacons are detected. Place an Eddystone beacon in range. Code which calls getId3() on a Beacon object will fail with this Exception.

Mobile device model and OS version

Nexus 5, Android 6

Android Beacon Library version

2.9.1

IMPORTANT: This forum is reserved for feature requests or reproducible bugs with the library itself. If you need help with using the library with your project, please open a new question on StackOverflow.com.

Thanks! :-)

davidgyoung commented 7 years ago

Good point, @bluetooth-mdw.

The simplest solution would be to have the method return null. However, this may still cause a NullPointerException later on in application code, and improvement for sure, but not as convenient as your suggestion of returning an Identifier instance that wraps a null using the Null Object pattern.

Adding a special null Identifier instance concerns me a bit, because it seems like it would lead to a breaking API change. Would the API then require passing this in when creating regions? Like this: new Region("my region", Identifier.nullInstance(), Identifier.nullInstance(), Identifier.nullInstance()) instead of new Region("my region", null, null, null)

Bottom line: I think the simplest incremental change is to return null, since passing null for Identifier instances is allowed for other cases of the API.

ghost commented 7 years ago

You're right. Nothing's ever simple :-)

Cheers

Martin