Skip to content

feat: add test cases for handling empty arrays and non-array input in functions #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 29, 2025

Conversation

josdejong
Copy link
Collaborator

See #8

This PR adds test cases for handling empty arrays and non-array input in functions sum, prod, min, max, average, and, and or.

Discussion points:

  1. sum on an empty array: throw or return 0?
  2. min on an empty array: throw or return null?
  3. max on an empty array: throw or return null?
  4. and on an empty array: throw, return true, or return false?
  5. or on an empty array: throw or return false?
  6. What to do with sorting data that cannot be sorted? Throw error, or leave it unsorted?

Right now, this PR throws in all cases 1-6.

@lateapexearlyspeed I would love to hear your thoughts on these discussion points 😄.

… functions `sum`, `prod`, `min`, `max`, `average`, `and`, and `or`
@lateapexearlyspeed
Copy link

Although exception is suitable to be used to represent an 'unexpected' input, I doubt here should consider how to make user use it easily. The developer may already take carefully json query they created but they cannot assume the struct of coming json data. I also found out some existing json query language treat this kind of case and return some default value rather than throw.
Anyway, I have no strong opinion on it, so please decide it @josdejong :)

@josdejong
Copy link
Collaborator Author

Thanks for your feedback. You're right, less exceptions in general is preferrable.

  • I think an empty array in case of and and or will not happen in practice (since you will use operators), and also, there is no logic return value in case of an empty array, so let's keep them throwing an exception.
  • The function sum can just return 0 in case of an empty array, which is also common in programming languages, so let's change that.
  • Then, the min or max value of an empty list is "nothing". Different progamming languages do different things in this case: throw, return null, return an empty array. We can return null, similar to say getting a non-existing property on an object, which right now also returns null. And it is always possible to add a custom function minOrThrow and maxOrThrow if you want alternative behavior.
  • From a user perspecive, it is better to leave non-sortable data as-is instead of throwing an exception.

So the behavior will be:

  1. sum on an empty array: return 0
  2. min on an empty array: return null
  3. max on an empty array: return null
  4. and on an empty array: throw
  5. or on an empty array: throw
  6. sort on data that cannot be sorted: leave it unsorted

@josdejong josdejong merged commit d496f54 into develop Apr 29, 2025
4 checks passed
@josdejong josdejong deleted the feat/8-extend-test-suite branch April 29, 2025 09:47
Copy link

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants