mirror of
https://github.com/gnh1201/welsonjs.git
synced 2025-09-18 15:48:58 +00:00
updated pr-agent best practices with auto analysis
parent
caebf914d2
commit
18d6108674
|
@ -1,3 +1,145 @@
|
|||
<!-- PR --><table><tr><td> <b><a href='https://github.com/gnh1201/welsonjs/pull/320#issuecomment-3204803405'>PR 320</a></b> (2025-08-20)
|
||||
|
||||
</td></tr></table>
|
||||
|
||||
|
||||
|
||||
<!-- suggestion --><details><summary>[possible issue] Handle empty/invalid inputs safely</summary>
|
||||
|
||||
___
|
||||
|
||||
✅ Handle empty/invalid inputs safely
|
||||
|
||||
**Guard against empty coords after validation; current code will use Math.min/max on empty arrays, yielding Infinity/-Infinity and NaN density. Return false if no valid cells remain. Also coerce minDensity to a number and clamp between 0 and 1 to avoid unexpected truthy strings.**
|
||||
|
||||
[lib/extramath.js [73-106]](https://github.com/gnh1201/welsonjs/pull/320/files#diff-d76f6c7464ea00689b45d5f11872891148869f73e11c497d62d14602ea486bbdR73-R106)
|
||||
|
||||
```diff
|
||||
function clusteredCellsDensity(numbers, size, minDensity) {
|
||||
- if (!numbers || !numbers.length) return false;
|
||||
- if (typeof size !== 'number' || size <= 0) size = 4; // default grid size = 4
|
||||
- if (typeof minDensity === 'undefined') minDensity = 0.6;
|
||||
+ if (!Array.isArray(numbers) || numbers.length === 0) return false;
|
||||
+ if (typeof size !== 'number' || size <= 0) size = 4;
|
||||
+ if (typeof minDensity !== 'number' || isNaN(minDensity)) minDensity = 0.6;
|
||||
+ if (minDensity < 0) minDensity = 0;
|
||||
+ if (minDensity > 1) minDensity = 1;
|
||||
|
||||
- var maxIndex = size * size; // because it's 1-based (1..size*size)
|
||||
+ var maxIndex = size * size;
|
||||
|
||||
- // 1. Convert cell number → (x,y) coordinates with 1-based check
|
||||
var coords = [];
|
||||
for (var i = 0; i < numbers.length; i++) {
|
||||
- var n = numbers[i];
|
||||
- // must be within 1..maxIndex
|
||||
- if (typeof n !== 'number' || n < 1 || n > maxIndex) {
|
||||
- return false; // invalid index -> immediately return false
|
||||
- }
|
||||
- var idx = n - 1; // shift to 0-based
|
||||
+ var n = numbers[i];
|
||||
+ if (typeof n !== 'number' || !isFinite(n) || n % 1 !== 0 || n < 1 || n > maxIndex) {
|
||||
+ return false;
|
||||
+ }
|
||||
+ var idx = n - 1;
|
||||
coords.push({ x: idx % size, y: Math.floor(idx / size) });
|
||||
}
|
||||
+
|
||||
+ if (coords.length === 0) return false;
|
||||
|
||||
var xs = coords.map(function (c) { return c.x; });
|
||||
var ys = coords.map(function (c) { return c.y; });
|
||||
|
||||
- // 2. Compute bounding box of selected cells
|
||||
var minX = Math.min.apply(Math, xs), maxX = Math.max.apply(Math, xs);
|
||||
var minY = Math.min.apply(Math, ys), maxY = Math.max.apply(Math, ys);
|
||||
|
||||
- // 3. Compute area and density
|
||||
- var w = maxX - minX + 1;
|
||||
- var h = maxY - minY + 1;
|
||||
+ var w = (maxX - minX + 1);
|
||||
+ var h = (maxY - minY + 1);
|
||||
var boxArea = w * h;
|
||||
+ if (!isFinite(boxArea) || boxArea <= 0) return false;
|
||||
|
||||
var density = coords.length / boxArea;
|
||||
return density >= minDensity;
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Suggestion importance[1-10]: 8
|
||||
|
||||
__
|
||||
|
||||
Why: The suggestion correctly points out that if all numbers in the input array are invalid, `coords` will be empty, causing `Math.min`/`max` to return `Infinity`, leading to a `NaN` density.
|
||||
|
||||
___
|
||||
|
||||
</details>
|
||||
|
||||
|
||||
|
||||
<!-- suggestion --><details><summary>[general] Validate tile position inputs</summary>
|
||||
|
||||
___
|
||||
|
||||
✅ Validate tile position inputs
|
||||
|
||||
**Add input validation to prevent NaN/Infinity when tiles is zero/negative or index is out of range. Clamp col/row to non-negative and return null for invalid arguments to avoid propagating bad coordinates.**
|
||||
|
||||
[lib/extramath.js [108-119]](https://github.com/gnh1201/welsonjs/pull/320/files#diff-d76f6c7464ea00689b45d5f11872891148869f73e11c497d62d14602ea486bbdR108-R119)
|
||||
|
||||
```diff
|
||||
-function estimateTileStartPosition(index, tiles, spreadSize, gap) {
|
||||
+function estimateTileStartPosition(index, tiles, spreadSize, gap) {
|
||||
+ if (typeof tiles !== 'number' || tiles <= 0) return null;
|
||||
+ if (typeof spreadSize !== 'number' || !isFinite(spreadSize)) return null;
|
||||
+ if (typeof gap !== 'number' || !isFinite(gap) || gap < 0) gap = 0;
|
||||
+ if (typeof index !== 'number' || !isFinite(index) || index % 1 !== 0 || index < 1) return null;
|
||||
+
|
||||
+ var totalSlots = tiles * tiles;
|
||||
+ if (index > totalSlots) return null;
|
||||
+
|
||||
var tileSize = (spreadSize - gap * (tiles - 1)) / tiles;
|
||||
+ if (!isFinite(tileSize)) return null;
|
||||
|
||||
var i = index - 1;
|
||||
var col = i % tiles;
|
||||
var row = Math.floor(i / tiles);
|
||||
-
|
||||
- return {
|
||||
- x: col * (tileSize + gap),
|
||||
- y: row * (tileSize + gap)
|
||||
+
|
||||
+ return {
|
||||
+ x: Math.max(0, col) * (tileSize + gap),
|
||||
+ y: Math.max(0, row) * (tileSize + gap)
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
Suggestion importance[1-10]: 8
|
||||
|
||||
__
|
||||
|
||||
Why: The suggestion correctly identifies that if `tiles` is zero, a division by zero will occur, resulting in `NaN`/`Infinity` values, and proposes adding necessary input validation.
|
||||
|
||||
___
|
||||
|
||||
</details>
|
||||
|
||||
___
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='https://github.com/gnh1201/welsonjs/pull/318#issuecomment-3194328108'>PR 318</a></b> (2025-08-17)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -165,6 +307,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='https://github.com/gnh1201/welsonjs/pull/309#issuecomment-3172799544'>PR 309</a></b> (2025-08-10)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -209,6 +353,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='https://github.com/gnh1201/welsonjs/pull/307#issuecomment-3154116053'>PR 307</a></b> (2025-08-05)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -302,6 +448,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='3055522397'>PR 284</a></b> (2025-07-10)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -374,6 +522,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='3006712754'>PR 279</a></b> (2025-06-26)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -428,6 +578,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='2982794401'>PR 276</a></b> (2025-06-18)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -479,6 +631,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='2953770469'>PR 272</a></b> (2025-06-08)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -540,6 +694,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='2868572186'>PR 249</a></b> (2025-05-10)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -611,6 +767,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='2868568893'>PR 248</a></b> (2025-05-10)
|
||||
|
||||
|
@ -741,6 +899,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
<!-- PR --><table><tr><td> <b><a href='2850302530'>PR 245</a></b> (2025-05-05)
|
||||
|
||||
</td></tr></table>
|
||||
|
@ -836,6 +996,8 @@ ___
|
|||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
|
Loading…
Reference in New Issue
Block a user