feat: add `toSorted` method to `array/fixed-endian-factory` #3302

ahmad-kashkoush · 2024-11-30T20:25:54Z

Resolves #3160

Description

What is the purpose of this pull request?

This pull request:

adds support for a toSorted method to @stdlib/array/fixed-endian-factory.

Related Issues

Does this pull request have any related issues?

This pull request:

resolves [RFC]: add toSorted method to array/fixed-endian-factory #3160

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

Read, understood, and followed the contributing guidelines.

@stdlib-js/reviewers

stdlib-bot · 2024-11-30T20:28:08Z

Coverage Report

Package	Statements	Branches	Functions	Lines
array/fixed-endian-factory	$1237 / 1488$ $+ 83.13 %$	$170 / 182$ $+ 93.41 %$	$25 / 35$ $+ 71.43 %$	$1237 / 1488$ $+ 83.13 %$

The above coverage report was generated for the changes in this PR.

…orted

kgryte · 2024-12-04T03:29:34Z

lib/node_modules/@stdlib/array/fixed-endian-factory/benchmark/benchmark.to_sorted.js

+		}
+	}
+	b.toc();
+	if ( !isTypedArray( out ) ) {


This is not a valid check, as out is not an actual typed array instance.

kgryte · 2024-12-04T03:29:54Z

lib/node_modules/@stdlib/array/fixed-endian-factory/benchmark/benchmark.to_sorted.length.js

+			}
+		}
+		b.toc();
+		if ( !isTypedArray( out ) ) {


Same comment.

kgryte · 2024-12-04T03:31:29Z

lib/node_modules/@stdlib/array/fixed-endian-factory/lib/main.js

+			throw new TypeError( format( 'invalid invocation. `this` is not %s %s.', CHAR2ARTICLE[ dtype[0] ], CTOR_NAME ) );
+		}
+		if ( arguments.length === 0 ) {
+			compareFcn = function defaultCompareFcn( a, b ) {


We don't typically use function expressions, only declarations. I suggest refactoring accordingly. It is also not necessary to have this function in a nested scope and can be lifted to the top most scope.

kgryte · 2024-12-04T03:33:29Z

lib/node_modules/@stdlib/array/fixed-endian-factory/lib/main.js

+			minVal = obuf[ GETTER ]( i * BYTES_PER_ELEMENT, this._isLE );
+			for ( j = i+1; j < len; j++ ) {
+				val = obuf[ GETTER ]( j * BYTES_PER_ELEMENT, this._isLE );
+				if ( compareFcn( minVal, val ) > 0 ) {


Rather than rolling your own sort function, copy to a temporary generic array, sort using the built-in #.sort method, and then copy to an output array.

kgryte · 2024-12-04T03:35:01Z

lib/node_modules/@stdlib/array/fixed-endian-factory/test/test.to_sorted.js

+	expected = [ 1.0, 2.0, 3.0, 4.0, 5.0 ];
+	out = arr.toSorted( compareFcn );
+	values = [];
+	fcn();


Rather than use fcn to generate a values array, just use array/base/assert/has-same-values and compare to an expected Float64ArrayFE.

kgryte · 2024-12-04T03:35:16Z

lib/node_modules/@stdlib/array/fixed-endian-factory/test/test.to_sorted.js

+	expected = [ 1.0, 2.0, 3.0, 4.0, 5.0 ];
+	out = arr.toSorted();
+	values = [];
+	fcn();


Same comment.

kgryte · 2024-12-04T03:35:51Z

lib/node_modules/@stdlib/array/fixed-endian-factory/README.md

+var arr = new Float64ArrayFE( 'little-endian', [ 3.0, 1.0, 2.0 ] );
+// returns <Float64ArrayFE>
+
+var out=arr.toSorted( compareFcn );


Suggested change

var out=arr.toSorted( compareFcn );

var out = arr.toSorted( compareFcn );

You need spaces around operators here and below.

kgryte · 2024-12-04T03:36:11Z

lib/node_modules/@stdlib/array/fixed-endian-factory/README.md

+var out=arr.toSorted( compareFcn );
+// returns <Float64ArrayFE>
+
+var v=out.get(0);


Suggested change

var v=out.get(0);

var v = out.get( 0 );

Spaces around arguments here and below.

kgryte · 2024-12-04T03:36:23Z

lib/node_modules/@stdlib/array/fixed-endian-factory/README.md

@@ -607,6 +607,39 @@ var idx = arr.lastIndexOf( 5.0 );
 // returns -1
 ```

+<a name="method-toSorted"></a>
+
+#### TypedArrayFE.prototype.toSorted( compareFcn )


The compareFcn is optional.

kgryte · 2024-12-04T03:36:50Z

lib/node_modules/@stdlib/array/fixed-endian-factory/README.md

@@ -607,6 +607,39 @@ var idx = arr.lastIndexOf( 5.0 );
 // returns -1
 ```

+<a name="method-toSorted"></a>


This method should be inserted into the docs such that methods are listed in alphabetical order.

kgryte · 2024-12-04T03:37:18Z

lib/node_modules/@stdlib/array/fixed-endian-factory/lib/main.js

+	* @throws {TypeError} first argument must be a function
+	* @returns {TypedArray} sorted array
+	*/
+	setReadOnly( TypedArray.prototype, 'toSorted', function sort( compareFcn ) {


Insert methods in alphabetical order.

- update method logic and insert it into correct place in both main.js and readme - update type checking in benchmarks - use hasSameValues in test file

ahmad-kashkoush · 2024-12-04T16:01:39Z

@kgryte @Planeshifter I have applyed all changes you've suggested in this PR and also in #3298

ahmad-kashkoush · 2024-12-15T18:09:42Z

@kgryte may you give it a review?

ahmad-kashkoush added 2 commits November 30, 2024 22:21

feat: add logic, tests, and benchmarks to toSorted method

b79d687

docs: add documentation for toSorted method

bb097df

Merge branch 'develop' of github.com:stdlib-js/stdlib into endian-toS…

872f8b2

…orted

Planeshifter changed the title ~~feat: add toSorted method to array/fixed-endian-factory~~ feat: add toSorted method to array/fixed-endian-factory Dec 4, 2024

kgryte added Needs Review Accepted Feature and removed Accepted labels Dec 4, 2024

kgryte reviewed Dec 4, 2024

View reviewed changes

kgryte added Needs Changes and removed Needs Review labels Dec 4, 2024

ahmad-kashkoush added 2 commits December 4, 2024 17:51

refactor: update toSorted method

6c18ce2

- update method logic and insert it into correct place in both main.js and readme - update type checking in benchmarks - use hasSameValues in test file

Merge remote-tracking branch 'rem/develop' into endian-toSorted

e61542a

ahmad-kashkoush requested a review from kgryte December 4, 2024 16:01

Planeshifter added the Potential Duplicate label Mar 15, 2025

kgryte mentioned this pull request Mar 16, 2025

feat: add toSorted method to array/fixed-endian-factory #6047

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `toSorted` method to `array/fixed-endian-factory` #3302

feat: add `toSorted` method to `array/fixed-endian-factory` #3302

ahmad-kashkoush commented Nov 30, 2024

stdlib-bot commented Nov 30, 2024 •

edited

Loading

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

kgryte Dec 4, 2024

ahmad-kashkoush commented Dec 4, 2024

ahmad-kashkoush commented Dec 15, 2024

	var out=arr.toSorted( compareFcn );
	var out = arr.toSorted( compareFcn );

feat: add toSorted method to array/fixed-endian-factory #3302

Are you sure you want to change the base?

feat: add toSorted method to array/fixed-endian-factory #3302

Conversation

ahmad-kashkoush commented Nov 30, 2024

Description

Related Issues

Questions

Other

Checklist

stdlib-bot commented Nov 30, 2024 • edited Loading

Coverage Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmad-kashkoush commented Dec 4, 2024

ahmad-kashkoush commented Dec 15, 2024

feat: add `toSorted` method to `array/fixed-endian-factory` #3302

feat: add `toSorted` method to `array/fixed-endian-factory` #3302

stdlib-bot commented Nov 30, 2024 •

edited

Loading