Allow the + operator to work with more types. Throw error on incomptible types #18
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the add function in the binary arithmetic operators to support additional data types (arrays and objects) and introduces error handling for incompatible type combinations.
Changes:
- Added explicit handling for number addition to ensure numeric types are added correctly
- Added array concatenation support using
concat() - Added object merging support using spread operator
- Throws an error for incompatible type combinations instead of attempting silent coercion
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( | ||
| typeof a === 'object' && | ||
| typeof b === 'object' && | ||
| !Array.isArray(a) && | ||
| !Array.isArray(b) | ||
| ) { | ||
| return { ...a, ...b }; | ||
| } |
There was a problem hiding this comment.
This logic may not handle null values correctly. When both a and b are null, typeof null === 'object' is true, so this condition will be met and return an object spread of null values, which results in an empty object {}. This behavior may be unexpected. Consider adding null checks before the object merge logic.
| // If both values are arrays then we want to concatenate the arrays. | ||
| if (Array.isArray(a) && Array.isArray(b)) { | ||
| return a.concat(b); | ||
| } | ||
|
|
||
| // If both values are objects then we want to merge the objects. | ||
| if ( | ||
| typeof a === 'object' && | ||
| typeof b === 'object' && | ||
| !Array.isArray(a) && | ||
| !Array.isArray(b) | ||
| ) { | ||
| return { ...a, ...b }; | ||
| } |
There was a problem hiding this comment.
The new behavior for adding arrays (concatenation) and objects (merging) lacks test coverage. The test suite has comprehensive coverage for other operators, but there are no tests verifying that adding two arrays concatenates them or that adding two objects merges them. Consider adding tests to verify this new functionality works as expected.
src/operators/binary/arithmetic.ts
Outdated
| // Otherwhise return an error indicating that the values of mixed types cannot be added. | ||
| throw new Error(`Cannot add values of different types: ${typeof a} and ${typeof b}`); |
There was a problem hiding this comment.
The new error throwing behavior lacks test coverage. There are no tests verifying that the function throws an error when attempting to add incompatible types (e.g., number + array, string + object). Consider adding tests to ensure the error is thrown correctly with the expected message.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot Extend the unit tests for the add function to cover all branches. |
|
@Sander-Toonen I've opened a new pull request, #19, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Add comprehensive unit tests for add function covering all branches Co-authored-by: Sander-Toonen <5106372+Sander-Toonen@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.