fix: value-based equality and hashCode for BleCharacteristic#267
fix: value-based equality and hashCode for BleCharacteristic#267xianjianlf2 wants to merge 1 commit into
Conversation
BleCharacteristic.operator== compared the `properties` list with `!=`, which is List identity comparison, and hashCode used `properties.hashCode` (also identity based). As a result two characteristics with equal content but distinct list instances were never equal and produced different hash codes, breaking the equality contract and Set/Map lookups. Use `listEquals` for comparison and `Object.hashAll` for hashing, matching the pattern already used in ManufacturerData.
There was a problem hiding this comment.
Code Review
This pull request fixes the equality and hashCode implementation of BleCharacteristic to compare properties by value using listEquals and Object.hashAll, and adds corresponding unit tests. The reviewer suggested using Object.hash instead of the bitwise XOR (^) operator to combine hash codes to prevent potential hash collisions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| int get hashCode => | ||
| uuid.hashCode ^ Object.hashAll(properties) ^ metaData.hashCode; |
There was a problem hiding this comment.
Using the bitwise XOR (^) operator to combine hash codes is prone to hash collisions (e.g., if fields have identical hash values, they cancel each other out). It is a best practice in Dart to use Object.hash to combine multiple hash codes safely.
| int get hashCode => | |
| uuid.hashCode ^ Object.hashAll(properties) ^ metaData.hashCode; | |
| int get hashCode => Object.hash( | |
| uuid, | |
| Object.hashAll(properties), | |
| metaData, | |
| ); |
There was a problem hiding this comment.
@xianjianlf2 could you please check this comment?
Problem
BleCharacteristic.operator==compares thepropertieslist with!=, which is List identity comparison in Dart, andhashCodeusesproperties.hashCode, which is also identity-based. As a result, two characteristics with identical content are never equal unless they share the exact same list instance:This breaks the
==/hashCodecontract and makesBleCharacteristicunreliable as aSetelement orMapkey (e.g. deduplicating characteristics across discovery runs).Fix
operator==now comparespropertieswithlistEquals.hashCodenow usesObject.hashAll(properties).This matches the pattern already used by
ManufacturerDatain this package.Testing
test/ble_characteristic_equality_test.dartcovering: equal content ⇒ equal + same hash, differing properties ⇒ unequal, differing uuid ⇒ unequal, and Set/Map key usage.flutter test test/ble_characteristic_equality_test.dartlocally (Flutter 3.44.1 / Dart 3.12.1): 4 tests, all passing.Changelog
Added an entry under the unreleased
2.1.1section inCHANGELOG.md.