Skip to content
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

new-log-viewer: Add bounds checking and search methods to support log filtering around a specific log event. #81

Merged
merged 16 commits into from
Oct 3, 2024
84 changes: 84 additions & 0 deletions new-log-viewer/src/utils/data.ts
davemarco marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,87 @@
import {Nullable} from "../typings/common";


/**
* Checks if 'target' is bounded by the first and last value in a sorted array of numbers.
*
* @param data A number array sorted in ascending order.
* @param target
* @return `true` if is `target` is within bounds; `false` otherwise or when the array is empty.
davemarco marked this conversation as resolved.
Show resolved Hide resolved
*/
const isWithinBounds = (data: number[], target: number): boolean => {
const {length} = data;
if (0 === length) {
return false;
}

return (target >= (data[0] as number)) && (target <= (data[length - 1] as number));
};

/**
* Performs binary search to find the smallest index `i` in the range [0, length) where the
* `conditionFn` is true. Assumes that the `conditionFn` is false for some prefix of the input
* range and true for the remainder. The most common use is to find the index `i` of a target
* value in a sorted array.
davemarco marked this conversation as resolved.
Show resolved Hide resolved
*
* @param length The length of the range to search.
* @param conditionFn A function that takes an index and returns `true` or `false`.
* @return The smallest index where `conditionFn(i)` is true. If no such index exists, returns
* `length`.
davemarco marked this conversation as resolved.
Show resolved Hide resolved
* @example
* const arr = [1, 3, 5, 7, 10, 15, 20];
* const result = binarySearch(arr.length, (i) => arr[i] >= 10);
* console.log(result); // Output: 4 (since arr[4] is 10).
*/
const binarySearch = (length: number, conditionFn: (index: number) => boolean): number => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename conditionFn to binaryPredicate? I think that's usually what it's called.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed to predicate

// Generic implementation based on Go standard library implementation.
// Reference: https://pkg.go.dev/sort#Search
let i = 0;
let j = length;

davemarco marked this conversation as resolved.
Show resolved Hide resolved
while (i < j) {
const mid = Math.floor((i + j) / 2);

davemarco marked this conversation as resolved.
Show resolved Hide resolved
if (false === conditionFn(mid)) {
i = mid + 1;
} else {
j = mid;
}
}

return i;
};

/**
* Finds the largest index `i` in a sorted array `data` such that `data[i] <= target`. Uses binary
* search for efficiency.
Comment on lines +68 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Finds the largest index `i` in a sorted array `data` such that `data[i] <= target`. Uses binary
* search for efficiency.
* Finds the index `i` in a sorted array `data` such that `data[i]` is the "first nearest" element
* to `target`, where "first nearest" is defined as follows:
* - If `target` is in or greater than the range of `data`'s elements, then `i` is the largest index
* such that `data[i] <= target`.
* - Otherwise, `target` is less than the range of `data`'s elements, so `i` is 0 since `data[0]` is
* the array element closest to `target`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see review comment

*
* @param data A number array sorted in ascending order.
davemarco marked this conversation as resolved.
Show resolved Hide resolved
* @param target
* @return The largest index where `data[i] <= target`. There are 2 edge cases where returns:
davemarco marked this conversation as resolved.
Show resolved Hide resolved
* - 0 if `target` is less than `data[0]`.
* - `null` if array is empty.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the 0 return case confusing:

If we're looking for i such that data[i] <= target, then

  1. if target is not in the bounds of data, we return 0.
  2. if data[0] <= target, we also return 0.

Since these two cases are indistinguishable, I feel like we should instead return length?

Copy link
Contributor Author

@davemarco davemarco Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explained in person. Maybe we can call the function findNearestIndexLessThanOrEqual or findNearestIndex. I feel like nearest is more vague, so the edge case makes more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also sort of inline with getMapValueWithNearestLessThanOrEqualKey later in the file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can fix this by clamping the value by the bounds of the array before calling the function. I know you already rewrote the docs... Let me try this, and ill request your review

* @example
* const arr = [2, 3, 5, 7, 10, 15, 20];
* const result = getLargestIdxLte(arr, 8);
* console.log(result); // Output: 3 (since arr[3] is 7).
*/
const getLargestIdxLte = (data: number[], target: number): Nullable<number> => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is a bit confusing to me. How about findLessThanOrEqualElement? (I know what Lte means but it could be misinterpreted as LTE, lol.)

Copy link
Contributor Author

@davemarco davemarco Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about findNearestIndexLessThanOrEqual or findNearestIndex

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about findFirstNearestElement?

  • "first nearest" is an abstract concept so hopefully users of the function will read the docstring before using it.
  • I think using "element" instead of "index" is a bit more intuitive since we really care about the element; it's just that the function indicates the found element by returning an index.

const {length} = data;

if (0 === length) {
return null;
}

// Binary search to find the first index where data[i] > target.
const firstGreaterIdx: number = binarySearch(length, (i) => data[i] as number > target);

if (0 === firstGreaterIdx) {
return 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return 0;
// `target` < `data[0]` so the first nearest element is `data[0]`.
return 0;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see review comment

}

return firstGreaterIdx - 1;
};

/**
* Finds the key in a map based on the provided value.
*
Expand Down Expand Up @@ -38,7 +119,10 @@ const getMapValueWithNearestLessThanOrEqualKey = <T>(
map.get(lowerBoundKey) as T;
};


export {
getLargestIdxLte,
getMapKeyByValue,
getMapValueWithNearestLessThanOrEqualKey,
isWithinBounds,
};
Loading