sillsdev / sillsdev/libpalaso

VerseRef.GetBBBCCCVVV does really unexpected things at the upper limit

Open
#1,479 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
46
Forks
50
Avg merge
6d 16h
Merged PRs (30d)
7

Description

I can't think of any scenario where using modulus to prevent a verse or chapter number from going out of range would give a a helpful result. Worse yet, at 999 (the purported max), we actually seem to have an unintentional off-by-one error, and 999 gets treated as 0. I would argue that if a verse or chapter number has been set to a value > 999, it should just be clamped to 999, in much the same way we are dealing with negative values.
It could possibly be argued that out-of-range values should throw an exception, or at least that a flag could be passed indicating whether the caller wants to clamp or throw.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the VerseRef.GetBBBCCCVVV entry point and inspect how chapter and verse values are handled at the upper boundary, especially the modulus behavior at 999. Confirm the desired behavior for values above 999 and the existing negative-value behavior, then add or update boundary coverage so valid maximum values remain 999 and out-of-range values are clamped.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.