-
Notifications
You must be signed in to change notification settings - Fork 584
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
feat(collections): add joinToString #1223
Changes from all commits
8e60ef0
18f146a
1c09899
ba24675
2d8b39a
1ab556e
aef8e42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,75 @@ | ||||||||||
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. | ||||||||||
|
||||||||||
/** | ||||||||||
* Options for joinToString | ||||||||||
*/ | ||||||||||
export type JoinToStringOptions = { | ||||||||||
separator?: string; | ||||||||||
prefix?: string; | ||||||||||
suffix?: string; | ||||||||||
limit?: number; | ||||||||||
truncated?: string; | ||||||||||
}; | ||||||||||
|
||||||||||
/** | ||||||||||
* Transforms the elements in the given array to strings using the given selector. | ||||||||||
* Joins the produced strings into one using the given `separator` and applying the given `prefix` and `suffix` to the whole string afterwards. | ||||||||||
* If the array could be huge, you can specify a non-negative value of `limit`, in which case only the first `limit` elements will be appended, followed by the `truncated` string. | ||||||||||
* Returns the resulting string. | ||||||||||
* | ||||||||||
* Example: | ||||||||||
* | ||||||||||
* ```ts | ||||||||||
* import { joinToString } from "https://deno.land/std@$STD_VERSION/collections/mod.ts"; | ||||||||||
* import { assertEquals } from "https://deno.land/std@$STD_VERSION/testing/asserts.ts"; | ||||||||||
* | ||||||||||
* const users = [ | ||||||||||
* { name: "Kim" }, | ||||||||||
* { name: "Anna" }, | ||||||||||
* { name: "Tim" }, | ||||||||||
* ]; | ||||||||||
* | ||||||||||
* const message = joinToString(users, (it) => it.name, { | ||||||||||
* suffix: " are winners", | ||||||||||
* prefix: "result: ", | ||||||||||
* separator: " and ", | ||||||||||
* limit: 1, | ||||||||||
* truncated: "others", | ||||||||||
* }); | ||||||||||
* | ||||||||||
* assertEquals(message, "result: Kim and others are winners"); | ||||||||||
* ``` | ||||||||||
*/ | ||||||||||
export function joinToString<T>( | ||||||||||
array: readonly T[], | ||||||||||
selector: (el: T) => string, | ||||||||||
{ | ||||||||||
separator = ",", | ||||||||||
prefix = "", | ||||||||||
suffix = "", | ||||||||||
limit = -1, | ||||||||||
truncated = "...", | ||||||||||
}: Readonly<JoinToStringOptions> = {}, | ||||||||||
): string { | ||||||||||
let result = ""; | ||||||||||
|
||||||||||
let index = -1; | ||||||||||
while (++index < array.length) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: I am personally not the biggest fan of implicit mutation with increment operators, as it gets missed quite easily when reading the code, hiding the core step of the loop. I always find it a lot cleaner to just have a separate index += 1 line There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this not a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i need to access the index of the element. i cant use forEach since I need to have the option to break the loop. I saw this pattern (using while for loop through the arrays) in lodash a lot. so I used that here too. but I'm open to any suggestion that makes the code more clear. |
||||||||||
const el = array[index]; | ||||||||||
|
||||||||||
if (index > 0) { | ||||||||||
result += separator; | ||||||||||
} | ||||||||||
|
||||||||||
if (limit > -1 && index >= limit) { | ||||||||||
result += truncated; | ||||||||||
break; | ||||||||||
} | ||||||||||
|
||||||||||
result += selector(el); | ||||||||||
} | ||||||||||
|
||||||||||
result = prefix + result + suffix; | ||||||||||
|
||||||||||
return result; | ||||||||||
Comment on lines
+72
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: Could be shortened to directly
Suggested change
|
||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. | ||
|
||
import { assertEquals } from "../testing/asserts.ts"; | ||
import { joinToString } from "./join_to_string.ts"; | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] no mutation", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
joinToString(arr, (it) => it.name); | ||
|
||
assertEquals(arr, [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] identity", | ||
fn() { | ||
const arr = ["Kim", "Anna", "Tim"]; | ||
|
||
const out = joinToString(arr, (it) => it); | ||
|
||
assertEquals(out, "Kim,Anna,Tim"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] normal mapppers", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name); | ||
|
||
assertEquals(out, "Kim,Anna,Tim"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] separator", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { separator: " and " }); | ||
|
||
assertEquals(out, "Kim and Anna and Tim"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] prefix", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { | ||
prefix: "winners are: ", | ||
}); | ||
|
||
assertEquals(out, "winners are: Kim,Anna,Tim"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] suffix", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { | ||
suffix: " are winners", | ||
}); | ||
|
||
assertEquals(out, "Kim,Anna,Tim are winners"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] limit", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { | ||
limit: 2, | ||
}); | ||
|
||
assertEquals(out, "Kim,Anna,..."); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] truncated", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { | ||
limit: 2, | ||
truncated: "...!", | ||
}); | ||
|
||
assertEquals(out, "Kim,Anna,...!"); | ||
}, | ||
}); | ||
|
||
Deno.test({ | ||
name: "[collections/joinToString] all options", | ||
fn() { | ||
const arr = [ | ||
{ name: "Kim", age: 22 }, | ||
{ name: "Anna", age: 31 }, | ||
{ name: "Tim", age: 58 }, | ||
]; | ||
const out = joinToString(arr, (it) => it.name, { | ||
suffix: " are winners", | ||
prefix: "result: ", | ||
separator: " and ", | ||
limit: 1, | ||
truncated: "others", | ||
}); | ||
|
||
assertEquals(out, "result: Kim and others are winners"); | ||
}, | ||
}); | ||
majidsajadi marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of that type, as it allows to set
truncated
without settinglimit
, which does not make sense. I think this should beor something similar to leverage the compiler catching wrong usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we use your suggested type:
this is the default value for the
truncate
option. what if the user overrides this with onlylimit
since thetruncated
suppose to be optional. thetruncated
would beundefined
.I know I can handle the default values in the function body but I guess the code would be a little bit messy and I don't know if we want to go that way. the implemented type is just like the Kotlin version of this method so I guess its some what okay.
@LionC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @majidsajadi . Let's keep the implementation simple. I think it's mostly obvious that truncated is only meaningful with
limit
set in this particular case.