feat: add array/base/join

[adityacodes30]

Resolves #1327 .

Description

What is the purpose of this pull request?

This pull request adds array/base/join.

Related Issues

Does this pull request have any related issues?

This pull request:

• closes [RFC]: add @stdlib/array/base/join #1327

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

[kgryte]

You should add tests for edge cases. E.g., how are nulls and undefineds serialized? Complex number arrays? what happens for single element arrays? What about when null or undefined is the last element? What about NaN values? Etc etc. These tests should be replicated for generic, accessor, and typed arrays where appropriate.

[adityacodes30]

About NaN values being treated, the current implementation converts it to a NaN string which is also the default behaviour according to mdn docs. Do we want to do it any differently?

[adityacodes30]

Also , in case of Float64 arrays and like , when their method is invoked (it treats null and undefined as per its own join implementation) , it does not produce expected behaviour treating null or undefined as an empty string as our implementation here

var x = new Float64Array( [ 1.0, 2.0, null, undefined ] );
var s = join( x, ',' );
console.log(s);
> '1,2,0,NaN'

how should i handle the writing the testcases here as they do not objectively express join , rather direct to the internal methods

[kgryte]

As mentioned above, test edge cases as appropriate. As null and undefined have no equivalent in typed arrays, don't add tests for them for Float64Array and Int32Array.

For NaN, yes, the built-in behavior is correct. Meaning, except for null and undefined, the serialized value should be equivalent to calling x[i].toString().

BTW: given that null and undefined checks are unnecessary for typed arrays, so we should add additional branching logic similar to

if ( hasMethod( x, 'join' ) {
	// Defer to method...
	return out;
}
if ( obj.accessorProtocol ) {
	// Handle accessor arrays...
	return out;
}
if ( obj.dtype === 'generic' || obj.dtype === null ) {
	// Handle generic indexed arrays and array-like objects with explicit handling of `null` and `undefined`...
	return out;
}
// Handle indexed typed arrays where handling of `null` and `undefined` is not necessary...
return out;

[kgryte]

@adityacodes30 Thank you for working on this. At the moment, this PR needs several changes in order to move forward.

As some general advice, I would encourage you to not copy-and-paste without carefully considering whether what you are doing is applicable. You copied array/base/slice, it seemed, and there are a variety of key differences between that package and this one. As such, you need to be careful and think critically about what needs to be modified.

[kgryte]

@adityacodes30 Also, please note how I updated your OP and the PR title. The more you can reduce maintainer noise, the quicker you'll be to receive a review and the more likely that your future PRs will be merged.

[adityacodes30]

got it @kgryte, thanks for the very descriptive review - does make things a lot clearer for this pr and going forward

[adityacodes30]

@kgryte i have made the requested changes, let me know if any other edits are needed

[Planeshifter]

Left a few comments, but after these are addressed it should be ready to land, I think.

[Planeshifter]

LGTM! Will merge once CI has passed.

Thanks for your contributions!