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

String Additions #166

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions README.md
Expand Up @@ -188,6 +188,28 @@ supporting a number of use-cases, including:
`loop('la ', 0, 8) => 'la la la' // no tailing space`
* Tailing: `loop('/path/to/some/file.txt', -3) => 'txt'`
* Reversing: `loop('top', 3, 0) => 'pot'`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style comment: generally avoid "allows you to" in favour of more precise language. I realize that loop is documented that way, but we should correct that separately.

`capitalize` allows you to capitalize every first letter of every word in a given
string. For example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capitalize capitalizes the first letter of each word in a given string.

It's probably worth noting that for characters that have no capitals (e.g. Chinese, Japanese, Korean), this has no effect. String.toUppercase() adds the note "This function uses the language independent Unicode mapping and thus only works in some languages."


`capitalize('this was a triumph!') => 'This Was A Triumph!'`

`replace` allows you to replace a porton of a string starting from a specified index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/porton/portion/ or perhaps:
replace performs a substring replacement starting at the specified index.

Optionally, you can specifiy the length to overwrite the original string from the specified
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/specifiy/specify/. Consider rewording as:
the optional overwrite parameter specifies the number of characters from the starting index to overwrite in the original string.

index with the input string. Examples:

`replace("I'm doing a note here", 5, 'making', overwrite: 5)` => "I'm making a note here"`

`replace('Huge suckess!', 8, 'c') => 'Huge success!'`

`insert` inserts a string into another string at the given index. Example:

`insert('this a triumph!', 4, 'was ') => 'this was a triumph!'`

`remove` allows you to remove a specified length of text from a string at
a specified index. Example:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider:
remove performs a substring deletion with the specified starting index and length.


`remove('Whaaaaaat?', 2, 5) => 'What?'`

[quiver.strings]: http://google.github.io/quiver-dart/#quiver/quiver-strings

Expand Down
55 changes: 55 additions & 0 deletions lib/strings.dart
Expand Up @@ -241,3 +241,58 @@ bool equalsIgnoreCase(String a, String b) =>
*/
int compareIgnoreCase(String a, String b) =>
a.toLowerCase().compareTo(b.toLowerCase());

/**
* Returns a copy of the [input] where the first (a-z) letter of each word is
* capitalized.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define "word" here -- specifically what constitutes a word boundary.

*/
String capitalize(String input) =>
input.replaceAllMapped(new RegExp(r'(^| )(\w)'), (Match m) => m.group(0).toUpperCase());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works for ASCII 0x20 space characters. Consider supporting a variety of whitespace characters with:
new RegExp(r'(^|\s)(\w)'
or alternatively, treat all non-word characters as word breaks with:
new RegExp(r'(^|\W)(\w)'

Downsides:
Using whitespace: 'this is a test--with a catch' will result in the 'w' remaining lower-case.
Using non-word chars: 'capitalization is &#!%ing difficult.' will result in the 'ing' being capitalized to 'Ing'.

Of the two, I tend to prefer the second approach, mostly because the only misbehaving edge-case I can think of is the one above, which is likely rare enough that we can live with it.


/**
* Returns a new string where [replacement] is inserted at the [index] of
* the [original] string.
*
* If [overwrite] is given, only the specified number of characters will be
* overwritten. (Default: length of [replacement])
*
* If the [replacement] is longer than [original], the returned string will
* be longer than [original].
*/
String replace(String original, int index, String replacement, {int overwrite}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (overwrite < 0) throw new ArgumentError(overwrite);

StringBuffer str = new StringBuffer();
if (index > 0) {
str.write(original.substring(0, index));
}

if (overwrite == null) {
overwrite = replacement.length;
}

str.write(replacement);
if (overwrite > 0) {
if (overwrite < original.length) {
str.write(original.substring(index + overwrite));
}
} else {
str.write(original.substring(index));
}
return str.toString();
}

/**
* Removes a specified [length] of the [original] string starting from the
* given [index].
*/
String remove(String original, int index, int length) {
return replace(original, index, '', overwrite: length);
}

/**
* Inserts the value of [insertion] at the given [index] character of
* the [original] string.
*/
String insert(String original, int index, String insertion) {
return replace(original, index, insertion, overwrite: 0);
}

6 changes: 3 additions & 3 deletions pubspec.yaml
@@ -1,5 +1,5 @@
name: quiver
version: 0.19.0-dev.3
version: 0.19.0-dev.4
authors:
- Justin Fagnani <justinfagnani@google.com>
- Yegor Jbanov <yjbanov@google.com>
Expand All @@ -9,11 +9,11 @@ authors:
- John McDole <codefu@google.com>
description: A set of utility libraries for Dart
homepage: https://github.com/google/quiver-dart
documentation: http://google.github.io/quiver-dart/
environment:
sdk: '>=1.0.0'
documentation: http://google.github.io/quiver-dart/
dependencies:
path: '>=1.0.0 <2.0.0'
dev_dependencies:
unittest: '>=0.11.0'
matcher: '>=0.10.0'
unittest: '>=0.11.0'
39 changes: 39 additions & 0 deletions test/strings_test.dart
Expand Up @@ -311,4 +311,43 @@ main() {
});

});

group('capitalize', () {
test('should return "This Was A Triumph!" after capitalizing non-capitalized the input string', () {
expect(capitalize('this was a triumph!'), 'This Was A Triumph!');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding the following tests:

test('should correctly handle multiple spaces between words', () {
  expect(capitalize('this was  a   huge    success.'), 'This Was  A   Huge    Success.');
});
test('should correctly handle a variety of word breaks', () {
  // whitespace word breaks
  expect(capitalize('this\twas\na\rhuge\vsuccess.'), 'This\tWas\nA\rHuge\vSuccess.');
  // non-whitespace word breaks
  expect(capitalize('this!was@a#huge%success.'), 'This!Was@A#Huge%Success.');
});
test('should modify only the first character of each word', () {
  expect(captalize('tHIS WAS a tRiUMpH.'), 'THIS WAS A TRiUMpH.');
});
test('should leave uncapitalizable characters unmodified', () {
  expect(capitalize('大成功 でした。'), '大成功 でした。');
});

});

group('insert', () {
test('should return "abc" after inserting "b" into the string "ac" at index [1]', () {
expect(insert('ac', 1, 'b'), 'abc');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests for edge cases such as:

expect(insert('', 0, 'abc'), 'abc');
expect(insert('abc', 1, ''), 'abc');

and for failure cases such as:

expect(() => insert('ab', -1, 'c'), throws);
expect(() => insert('ab', 5, 'c'), throws);

});

group('replace', () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add tests for:

  • startIndex < 0: throws
  • startIndex == original.length: returns replacement
  • startIndex > original.length: throws
  • overwrite < 0: throws
  • overwrite == 0: behaves as an insert
  • overwrite > original.length - startIndex: replaces everything after startIndex

test('should return "abc" after replacing the index [1] character in the string "adc" with "b" '
'with length defaulted to the length of the replacement', () {
expect(replace('adc', 1 , 'b'), 'abc');
});
test('should return "abcde" after replacing the index [1] character in the string "adc" with "bcde" '
'with overwrite defaulted to the length of the replacement', () {
expect(replace('adc', 1 , 'bcde'), 'abcde');
});
test('should return "aabbcc" after replacing the index [1] character in the string "abc"'
'with "abbc" with overwrite = 1 (1 character overwritten)', () {
expect(replace('abc', 1, 'abbc', overwrite: 1), 'aabbcc');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above. Add tests for common edge cases and failure scenarios.

});

group('remove', () {
test('should return "abc" after remvoing the character at index [1] from "abbc"', () {
expect(remove('abbc', 1, 1), 'abc');
});

test('should return "abc" after removing 5 characters starting at index [1] from "abbbbbbc"', () {
expect(remove('abbbbbbc', 1, 5), 'abc');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above. Add tests for common edge cases and failure scenarios.

});


}