Skip to content

[RFC]: Add @stdlib/utils/every-in-by #822

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

Closed
3 tasks done
kgryte opened this issue Feb 1, 2023 · 18 comments · Fixed by #1409
Closed
3 tasks done

[RFC]: Add @stdlib/utils/every-in-by #822

kgryte opened this issue Feb 1, 2023 · 18 comments · Fixed by #1409
Labels
difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. Feature Issue or pull request for adding a new feature. Good First Issue A good first issue for new contributors! JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: Low Low priority concern or feature request. RFC Request for comments. Feature requests and proposed changes. Utilities Issue or pull request concerning general utilities.

Comments

@kgryte
Copy link
Member

kgryte commented Feb 1, 2023

Description

This RFC proposes adding a utility to test whether every "own" and inherited property of a provided object satisfies a predicate function.

This is the for-in equivalent of @stdlib/utils/every-by.

Package: @stdlib/utils/every-in-by
Alias: everyInBy

Related Issues

None.

Questions

No.

Other

No.

Checklist

  • I have read and understood the Code of Conduct.
  • Searched for existing issues and pull requests.
  • The issue name begins with RFC:.
@kgryte kgryte added RFC Request for comments. Feature requests and proposed changes. Feature Issue or pull request for adding a new feature. Good First Issue A good first issue for new contributors! labels Feb 1, 2023
@kgryte kgryte added Needs Discussion Needs further discussion. priority: Low Low priority concern or feature request. Utilities Issue or pull request concerning general utilities. JavaScript Issue involves or relates to JavaScript. difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. labels Feb 23, 2024
@the-r3aper7
Copy link
Contributor

the-r3aper7 commented Feb 24, 2024

would you explain this more i am not able to understand this?

@kgryte
Copy link
Member Author

kgryte commented Feb 24, 2024

By example,

function predicate( value, key, obj ) {
	return ( value > 0 );
}

var o = {
	'a': 1,
	'b': 2,
	'c': 3,
	'd': 4
};

var bool = everyInBy( o, predicate );
// returns true

o.a = -2;
bool = everyInBy( o, predicate );
// returns false

Pay attention to the OP. Namely, it should also work for inherited properties.

@the-r3aper7
Copy link
Contributor

the-r3aper7 commented Feb 25, 2024

i have created this test function will this work or should i make changes?

function predicate( value, key, obj ) {
	return ( value > 0 );
}

function everyByIn(obj, predicate, thisArg) {
  var out

  if ( !isObject( obj ) ) {
		throw new TypeError( format( 'invalid argument. First argument must be a object. Value: `%s`.', obj ) );
	}
	if ( !isFunction( predicate ) ) {
		throw new TypeError( format( 'invalid argument. Second argument must be a function. Value: `%s`.', predicate ) );
	}

  for (const key in obj) {
    if (obj.hasOwnProperty(key)) {
      out = predicate.call(thisArg, obj[key], key, obj)
      if (!out) {
        return false
      }
    } 
  }
  return true
}

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

You don't want the hasOwnProperty check, as this function should also check inherited properties, as mentioned in the OP.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

You can also inline the predicate.call expression into the conditional. No need for a temporary variable in this case.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

Otherwise, yes, what you have proposed is a good start. Feel free to open a PR with the implementation and associated package files.

@Planeshifter Should we go ahead and rename the package to @stdlib/object/every-in-by?

@the-r3aper7
Copy link
Contributor

cool i am excited on this one i will make changes regarding to it. then comment it before making a PR.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

Sounds good! Thanks for working on it!

@the-r3aper7
Copy link
Contributor

no worries happy to contribute.

@the-r3aper7
Copy link
Contributor

the-r3aper7 commented Feb 25, 2024

here is the updated function.

function everyByIn(obj, predicate, thisArg) {
  
  var key
  
  if (!isObject(obj)) {
    throw new TypeError(format('invalid argument. First argument must be an object. Value: `%s`.', obj));
  }
  if (!isFunction(predicate)) {
    throw new TypeError(format('invalid argument. Second argument must be a function. Value: `%s`.', predicate));
  }

  // Use a for...in loop to iterate through all enumerable properties, including inherited ones
  for (key in obj) {
    if (!predicate.call(thisArg, obj[key], key, obj)) {
      return false;
    }
  }

  return true;
}

@the-r3aper7
Copy link
Contributor

everything is working fine locally.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

That looks reasonable. Note that we author everything in ES5, so no const.

@the-r3aper7
Copy link
Contributor

i have updated the comment code.

@the-r3aper7
Copy link
Contributor

the-r3aper7 commented Feb 25, 2024

That looks reasonable. Note that we author everything in ES5, so no const.

from next time i will keep that in my mind.

@the-r3aper7
Copy link
Contributor

shall i make a PR now or wait.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

Go ahead and open a PR.

@kgryte
Copy link
Member Author

kgryte commented Feb 25, 2024

You'll obviously need to add docs, tests, and benchmarks similar to other packages.

@the-r3aper7
Copy link
Contributor

ok, i will make PR by tonight.

Planeshifter added a commit that referenced this issue Mar 2, 2024
PR-URL: #1409
Closes: #822 

---------

Signed-off-by: Philipp Burckhardt <[email protected]>
Co-authored-by: Philipp Burckhardt <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: 2 May require some initial design or R&D, but should be straightforward to resolve and/or implement. Feature Issue or pull request for adding a new feature. Good First Issue A good first issue for new contributors! JavaScript Issue involves or relates to JavaScript. Needs Discussion Needs further discussion. priority: Low Low priority concern or feature request. RFC Request for comments. Feature requests and proposed changes. Utilities Issue or pull request concerning general utilities.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants