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

Swap functions to data last #27

Open
brandonjpierce opened this issue Sep 27, 2024 · 12 comments
Open

Swap functions to data last #27

brandonjpierce opened this issue Sep 27, 2024 · 12 comments
Labels
refactor A code change that neither fixes a bug nor adds a feature

Comments

@brandonjpierce
Copy link
Contributor

brandonjpierce commented Sep 27, 2024

e.g. instead of

function toDigits(value, precision) {}

do

function toDigits(precision, value) {}

this way we can either auto curry this function or have it curried by default e.g.

const toDigit = (precision) => (value) => ...
// or
const curriedToDigit = autoCurry(toDigit);

and then in userland:

const toTenths = toDigit(2);
const value = toTenths(1.1234);
@brandonjpierce
Copy link
Contributor Author

Swapped math functions over in #23

@brandonjpierce brandonjpierce changed the title TODO: swap functions to data last Refactor: swap functions to data last Oct 23, 2024
@brandonjpierce
Copy link
Contributor Author

If you have questions on the ethos behind this change, please reach out to @belsrc

@brandonjpierce brandonjpierce added refactor A code change that neither fixes a bug nor adds a feature dev labels Oct 23, 2024
@brandonjpierce brandonjpierce changed the title Refactor: swap functions to data last Swap functions to data last Oct 23, 2024
@kalisjoshua
Copy link
Contributor

Is there an easy list of modules that need this treatment?

@brandonjpierce
Copy link
Contributor Author

I can get that list assembled, yes

@belsrc
Copy link
Contributor

belsrc commented Oct 23, 2024

For this, as you mentioned, there is two possible future use approaches. Which might be nice/easier to do in tandem. Which probably want to have a larger discussion on them (another issue? rfc-ish thing? 🤷 ). That being "do we want to auto curry them or just curry them or do nothing." They all have benefits and drawbacks.

@kalisjoshua
Copy link
Contributor

When possible consistency is my highest priority; only challenge is when performance contradicts consistency. As I understand things (so far) we are rather concerned with performance - out of necessity - so have we run into any consistency challenges for performance reasons? If not, I tend to prefer to do less over doing more; i.e. don't curry till it is needed.

@belsrc
Copy link
Contributor

belsrc commented Oct 23, 2024

Yea, thats part of the convo. Using Brandon's examples above:

const toDigit = (precision, value) => {} - Which requires downstream effort in order to curry it. Stuff like precision usual isnt something that changes and is prime for partial application.

const toDigit = (precision) => (value) => {} - Requires it to always be used in its curried form. Personally not an issue for me as () and ()() aren't that difficult. Though I can see how some would look at it weirdly.

const toDigit = autocurry((precision, value) => {}) - Best of both worlds as it can be used as () and ()() out of the box. Downside of another call on the stack.

@kalisjoshua
Copy link
Contributor

There are two independent issues:

Argument Ordering

Ordering the arguments list to place the most variable values at the end to support curry-ing/partial application.

Function Definition

Writing a function such that it is curry-ied or not.


Are we consistently creating library/reusable code in: a "curry"-ied style, an idiomatic style, or a mix of both?

@belsrc
Copy link
Contributor

belsrc commented Oct 23, 2024

Are we consistently creating library/reusable code in: a "curry"-ied style, an idiomatic style, or a mix of both?

This could also be part of the discussion, I suppose. Most of the things that are currently in curry format (() => () => {}) are things that Brandon ripped from one of my private repos 😆 . In which I tend to go with the traditional approach.

Aside: @brandonjpierce , could also use a ticket for possibly cleaning up some calls. Things like the below don't need to be spelled out and can instead simply be point-free.

export function toBoolean(val: unknown) {
  return isTrue(val);
}

// ↓

export const toBoolean =  isTrue;

(This one is literally just an alias)

@belsrc belsrc removed the dev label Nov 17, 2024
@kalisjoshua
Copy link
Contributor

After searching each file (I think) I haven't seen any functions that aren't "value last".

@kalisjoshua kalisjoshua self-assigned this Nov 22, 2024
@belsrc
Copy link
Contributor

belsrc commented Nov 23, 2024

Temporal stuff is the only thing that appears to not be current (didn't realize there was as many stubs in the repo).

@belsrc belsrc reopened this Nov 23, 2024
@kalisjoshua
Copy link
Contributor

I think it would be better to create an issue for each instance rather than a broadly scoped issue like this or fix them as each comes into the repo.

@kalisjoshua kalisjoshua removed their assignment Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

No branches or pull requests

3 participants