Add string validation functions #84

Merged
merged 17 commits into from Jan 8, 2017

Projects

None yet

3 participants

@Planeshifter
Member
Planeshifter commented Jan 4, 2017 edited

Checklist

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

  • Read, understood, and followed the contributing guidelines, including the relevant style guides.
  • Read and understand the Code of Conduct.
  • Read and understood the licensing terms.
  • Searched for existing issues and pull requests before submitting this pull request.
  • Filed an issue (or an issue already existed) prior to submitting this pull request.
  • Rebased onto latest develop.
  • Submitted against develop branch.

Description

What is the purpose of this pull request?

This pull request:

  • contains a function to test whether a value is a lowercase string
  • contains a function to test whether a value is a uppercase string
  • contains a function to test whether a value is a capitalized string

Related Issues

Does this pull request have any related issues?

No.

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.


@stdlib-js/reviewers

Planeshifter added some commits Jan 4, 2017
@Planeshifter Planeshifter Add utility to test for an uppercase string 1ca9845
@Planeshifter Planeshifter Add utility to test for a lowercase string 4e04ba1
@Planeshifter Planeshifter Add utility to test for a capitalized string 78a1bc1
@Planeshifter Planeshifter Add modules to namespace
2bd009d
@Planeshifter Planeshifter requested a review from kgryte Jan 4, 2017
+
+## Notes
+
+* This module first validates that a `value` is a `string`. For all other types, the function returns `false`.
@kgryte
kgryte Jan 4, 2017 Member

The function validates that a value is a string. For all other...

+
+// VARIABLES //
+
+var CAPITALS_REGEX = /^[A-Z]/;
@kgryte
kgryte Jan 4, 2017 Member

Should we support other langs?

+
+## Notes
+
+* This function first validates that a `value` is a `string`. For all other types, the function returns `false`.
@kgryte
kgryte Jan 4, 2017 Member

The function validates that...

+var isLowercase = require( '@stdlib/utils/is-lowercase' );
+
+var bool = isLowercase( 'hello' );
+// returns false
@kgryte
kgryte Jan 4, 2017 Member

This should be true.

+// returns false
+
+bool = isLowercase( '' );
+// returns true
@kgryte
kgryte Jan 4, 2017 Member

Is this correct?

+// => true
+
+console.log( isLowercase( '' ) );
+// => true
@kgryte
kgryte Jan 4, 2017 Member

Should an empty string be considered lowercase?

+* // returns true
+*/
+function isLowercase( value ) {
+ return isString( value ) && ( value.valueOf() === value.toLowerCase() );
@kgryte
kgryte Jan 4, 2017 Member

If checking for primitives, why use valueOf()? Also can use lowercase string util.

+ t.equal( isLowercase( values[i] ), false, 'returns false when provided '+values[i] );
+ }
+ t.end();
+});
@kgryte
kgryte Jan 4, 2017 Member

If an empty string is supposed to pass, should also include a test stating such behavior.

+
+## Notes
+
+* This module first validates that a `value` is a `string`. For all other types, the function returns `false`.
@kgryte
kgryte Jan 4, 2017 Member

The function validates that a...

+var bool = isUppercase( 'HELLO' );
+// returns true
+
+bool = isUppercase( '' );
@kgryte
kgryte Jan 4, 2017 Member

How can an empty string be both uppercase and lowercase at the same time?

@Planeshifter
Planeshifter Jan 4, 2017 Member

Refactored the implementation. The function now checks not only that the input string is equal to the uppercased string, but also different from the lowercased string. This way, it returns false for strings consisting only of say punctuation as well as the empty string. It's not anymore possible for both functions to return true for a given string. This seems to be in line with how analogous functions in other languages like Java behave.

@kgryte
kgryte Jan 5, 2017 Member

Sounds good. Thanks!

+// => true
+
+console.log( isUppercase( '' ) );
+// => true
@kgryte
kgryte Jan 4, 2017 Member

Same result as isLowercase.

+* // returns false
+*/
+function isUppercase( value ) {
+ return isString( value ) && ( value.valueOf() === value.toUpperCase() );
@kgryte
kgryte Jan 4, 2017 Member

If value is a primitive, why use valueOf()? Can use uppercase string utility.

+ t.equal( isUppercase( values[i] ), false, 'returns false when provided '+values[i] );
+ }
+ t.end();
+});
@kgryte
kgryte Jan 4, 2017 Member

If an empty string really is supposed to be uppercase, should include a test stating such behavior.

@kgryte kgryte changed the title from String validation functions to Add string validation functions Jan 4, 2017
@kgryte kgryte removed the basic label Jan 4, 2017
Planeshifter added some commits Jan 4, 2017
@Planeshifter Planeshifter Fix lint errors 9855c2d
@Planeshifter Planeshifter Fix lint errors
a7ec731
@Planeshifter Planeshifter Update wording and fix return value 5a594d2
@codecov-io
codecov-io commented Jan 4, 2017 edited

Current coverage is 88.5462% (diff: 100%)

Merging #84 into develop will increase coverage by 0.0051%

@@            develop        #84   diff @@
==========================================
  Files          2307       2313     +6   
  Lines         30195      30226    +31   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          26735      26764    +29   
- Misses         3460       3462     +2   
  Partials          0          0          

Sunburst

Diff Coverage File Path
•••••••••• 100% lib/node_modules/@stdlib/utils/lib/index.js
•••••••••• 100% new ...node_modules/@stdlib/utils/is-uppercase/lib/index.js
•••••••••• 100% new ...dules/@stdlib/utils/is-uppercase/lib/is_uppercase.js
•••••••••• 100% new ...s/@stdlib/utils/is-capitalized/lib/is_capitalized.js
•••••••••• 100% new ...node_modules/@stdlib/utils/is-lowercase/lib/index.js
•••••••••• 100% new ...dules/@stdlib/utils/is-lowercase/lib/is_lowercase.js
•••••••••• 100% new ...de_modules/@stdlib/utils/is-capitalized/lib/index.js

Powered by Codecov. Last update ef23207...4c589ab

Planeshifter added some commits Jan 4, 2017
@Planeshifter Planeshifter Refactor to handle special characters and empty string 837f942
@Planeshifter Planeshifter Update description
90db3a1
@@ -0,0 +1,74 @@
+# isCapitalized
+
+> Test if a value is a capitalized string.
@kgryte
kgryte Jan 5, 2017 Member

Test if a value is a string having an uppercase first character.

+
+#### isCapitalized( value )
+
+Tests if a `value` is a capitalized `string`.
@kgryte
kgryte Jan 5, 2017 Member

Tests if a value is a string having an uppercase first character.

+
+## Notes
+
+* This module validates that a `value` is a `string`. For all other types, the function returns `false`.
@kgryte
kgryte Jan 5, 2017 Member

The function validates...

+var isCapitalized = require( './../lib' );
+
+console.log( isCapitalized( 'Hello' ) );
+// => false
@kgryte
kgryte Jan 5, 2017 Member

Should this not be true?

+ t.end();
+});
+
+tape( 'the function returns `true` if provided a capitalized string', function test( t ) {
@kgryte
kgryte Jan 5, 2017 Member

"Capitalized string" has different connotations; e.g., all uppercase, each word having a capital letter, first letter is uppercase, etc. Instead, to be more explicit,

the function returns true if provided a string having an uppercase first character

@Planeshifter
Planeshifter Jan 5, 2017 Member

Good point. Will change.

+ t.end();
+});
+
+tape( 'the function returns `false` if not provided a capitalized string', function test( t ) {
@kgryte
kgryte Jan 5, 2017 Member

the function returns false if not provided a string having an uppercase first character

+{
+ "name": "@stdlib/utils/is-capitalized",
+ "version": "0.0.0",
+ "description": "Test if a value is a capitalized string.",
@kgryte
kgryte Jan 5, 2017 Member

Test if a value is a string having an uppercase first character.

+// MAIN //
+
+/**
+* Tests if a value is a capitalized string.
@kgryte
kgryte Jan 5, 2017 Member

Tests if a value is a string having an uppercase first character.

+'use strict';
+
+/**
+* Test if a value is a capitalized string.
@kgryte
kgryte Jan 5, 2017 Member

Test if a value is a string having an uppercase first character.

+
+## Notes
+
+* This module validates that a `value` is a `string`. For all other types, the function returns `false`.
@kgryte
kgryte Jan 5, 2017 Member

The function validates...

+var bool = isUppercase( 'HELLO' );
+// returns true
+
+bool = isUppercase( '' );
@kgryte
kgryte Jan 5, 2017 Member

Sounds good. Thanks!

Planeshifter added some commits Jan 5, 2017
@Planeshifter Planeshifter Update wording 35654ac
@Planeshifter Planeshifter Update module descripton to clarify behavior
fbc4598
@Planeshifter Planeshifter Update wording
d79da2f
+*/
+function isCapitalized( value ) {
+ if ( isString( value ) ) {
+ return CAPITALS_REGEX.test( value );
@kgryte
kgryte Jan 5, 2017 Member

Instead of RegExp, what about

var ch = value[ 0 ];
return ( ch === uppercase( ch ) && ch !== lowercase( ch ) );
+ values = [
+ 'In all the tree-tops you feel scarcely a breath',
+ 'HELLO WORLD',
+ void 0,
@kgryte
kgryte Jan 5, 2017 Member

Can we add an empty string, a string containing a punctuation character, and another containing only numbers?

+ values = [
+ 'abc',
+ 'hello World',
+ void 0,
@kgryte
kgryte Jan 5, 2017 Member

Can we add an empty string, a string containing a punctuation character, and another containing only numbers?

+ values = [
+ 'abc',
+ 'Hello World',
+ void 0,
@kgryte
kgryte Jan 5, 2017 Member

Can we add an empty string, a string containing a punctuation character, and another containing only numbers?

Planeshifter added some commits Jan 6, 2017
@Planeshifter Planeshifter Add test cases f9e6c64
@Planeshifter Planeshifter Refactor implementation
7f51476
+* // returns false
+*/
+function isLowercase( value ) {
+ if ( isString( value ) ) {
@kgryte
kgryte Jan 6, 2017 edited Member

This can be simplified to

return (
    isString( value ) &&
    value === lowercase( value ) &&
    value !== uppercase( value )
);
+* // returns false
+*/
+function isUppercase( value ) {
+ if ( isString( value ) ) {
@kgryte
kgryte Jan 6, 2017 edited Member

This can be simplified to

return (
    isString( value ) &&
    value === uppercase( value ) &&
    value !== lowercase( value )
);
+
+ for ( i = 0; i < values.length; i++ ) {
+ bool = isUppercase( values[ i ] );
+ t.equal( bool, true, 'returns true when provided '+values[ i ] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

+ ];
+
+ for ( i = 0; i < values.length; i++ ) {
+ t.equal( isUppercase( values[i] ), false, 'returns false when provided '+values[i] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

+
+ for ( i = 0; i < values.length; i++ ) {
+ bool = isLowercase( values[ i ] );
+ t.equal( bool, true, 'returns true when provided '+values[ i ] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

+ ];
+
+ for ( i = 0; i < values.length; i++ ) {
+ t.equal( isLowercase( values[i] ), false, 'returns false when provided '+values[i] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

+
+ for ( i = 0; i < values.length; i++ ) {
+ bool = isCapitalized( values[ i ] );
+ t.equal( bool, true, 'returns true when provided '+values[ i ] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

+ ];
+
+ for ( i = 0; i < values.length; i++ ) {
+ t.equal( isCapitalized( values[i] ), false, 'returns false when provided '+values[i] );
@kgryte
kgryte Jan 6, 2017 Member

Use t.strictEqual to ensure strict equality.

Planeshifter added some commits Jan 7, 2017
@Planeshifter Planeshifter Simplify code e840d57
@Planeshifter Planeshifter Use strict equality checks
4c589ab
+// returns true
+
+bool = isUppercase( '' );
+// returns true
@kgryte
kgryte Jan 7, 2017 Member

This should be

// returns false

correct?

@Planeshifter
Planeshifter Jan 7, 2017 Member

Yes. Fixed.

@Planeshifter Planeshifter Fix return value
e186e5a
@kgryte
kgryte approved these changes Jan 8, 2017 View changes
@kgryte kgryte merged commit e2ec892 into develop Jan 8, 2017

1 of 5 checks passed

continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
ci/circleci Your tests passed on CircleCI!
Details
@kgryte kgryte deleted the feature/string branch Jan 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment