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

extended Bounded type #1544

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Conversation

jessekelly881
Copy link

@jessekelly881 jessekelly881 commented Jul 26, 2021

Extends Bounded type. It's currently a work in progress, but I am looking for feedback. The thinking with these functions is that Bounded effectively represents a range of values. So, testing whether a value is within a range, if ranges overlap, clamping a value to a range, etc., makes sense.

import * as n from 'fp-ts/number';
import { top, bottom, fromRange, isWithin, clamp, toTuple } from 'fp-ts/Bounded';

const bound = fromRange(n.Ord)(0)(10);

top(bound) // 0
bottom(bound) //10

isWithin(bound)(1) //true
isWithin(bound)(11) //false

toTuple(bound) // [0, 10]

clamp(bound)(12) // 10

@jessekelly881 jessekelly881 changed the title WIP: extended Bounded type WIP: extended Bounded type (Looking for feedback) Jul 26, 2021
@jessekelly881 jessekelly881 changed the title WIP: extended Bounded type (Looking for feedback) WIP: extended Bounded type Jul 26, 2021
Copy link
Collaborator

@mlegenhausen mlegenhausen left a comment

Choose a reason for hiding this comment

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

Good work.

You need to change the @since comments to 2.12.0.

src/Bounded.ts Outdated Show resolved Hide resolved
src/Bounded.ts Outdated Show resolved Hide resolved
@jessekelly881 jessekelly881 changed the title WIP: extended Bounded type extended Bounded type Jul 30, 2021
src/Bounded.ts Outdated Show resolved Hide resolved
src/Predicate.ts Outdated Show resolved Hide resolved
src/Predicate.ts Outdated Show resolved Hide resolved
src/Predicate.ts Outdated Show resolved Hide resolved
src/Bounded.ts Outdated Show resolved Hide resolved
src/Bounded.ts Outdated Show resolved Hide resolved
src/Bounded.ts Outdated Show resolved Hide resolved
src/Eq.ts Outdated Show resolved Hide resolved
@jessekelly881
Copy link
Author

Fixed issues.

@mlegenhausen
Copy link
Collaborator

Looks good. Just another idea when creating a Bound why not have a function that always returns a valid bound like your fromRange but one that never fails.

export const bounded = <T>(O: Ord.Ord<T>) => (b: T) => (t: T): Bounded<T> =>
  Ord.geq(O)(B.bottom, B.top) ? ({ ...O, bottom: b, top: t }) : ({ ...O, bottom: t, top: b })

What do you think?

@jessekelly881
Copy link
Author

jessekelly881 commented Aug 3, 2021

Thanks! Hmm. This seems useful; although, I can't quite imagine a use case for it at the moment. Maybe called something like coerceBound to clarify that the order might not be the same as the order of the provided arguments?

@jessekelly881
Copy link
Author

jessekelly881 commented Aug 3, 2021

Added coerceBound, reverse, and isEmpty. Although, isEmpty might not be the best name for this function as a bound where top === bottom has exactly one value that is within the bound. Any thoughts? I'm leaning towards isSingular, isFlat, or isClosed.

@mlegenhausen
Copy link
Collaborator

Use case for me would be as smart constructor where I do not care about validation and just want a valid Bounded.

I find isSingular a good choice.

@jessekelly881
Copy link
Author

jessekelly881 commented Aug 5, 2021

Ok. I like the idea of a smart constructor. And isSingular is a good name. I updated the pr. Is there anything else that you can think of that needs changing?

test/Bounded.ts Outdated Show resolved Hide resolved
test/Bounded.ts Outdated Show resolved Hide resolved
test/Bounded.ts Outdated Show resolved Hide resolved
@jessekelly881
Copy link
Author

Fixed.

@jessekelly881
Copy link
Author

Hmm. Any idea what might be up with the automated testing? It seems like an issue with prettier but running prettier --write doesn't fix the issue and the error message isn't specific enough. Also, I'm considering adding a function that constructs a Bounded type from an instance of Ord and a ReadonlyNonEmptyArray of values. What are your thoughts? Something like this:

import * as A from 'fp-ts/ReadonlyNonEmptyArray';
import * as T from 'fp-ts/Tuple';
import * as n from 'fp-ts/number';
import { Bounded } from 'fp-ts/Bounded';
import { Ord } from 'fp-ts/Ord';
import { pipe } from 'fp-ts/function';

const fromArray = <X>(ord: Ord<X>) => (arr: A.ReadonlyNonEmptyArray<X>) => pipe(
    arr,
    x => [x, x] as [A.ReadonlyNonEmptyArray<X>, A.ReadonlyNonEmptyArray<X>],
    T.bimap(A.max(ord), A.min(ord)), // [min, max]
    t => ({ ...ord, bottom: t[0], top: t[1] }) as Bounded<X>
)

// Returns instance of Bounded where { bottom: 1, top: 5 }
fromArray(n.Ord)([1, 2, 3, 4, 5])

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

Successfully merging this pull request may close these issues.

2 participants