Skip to content
This repository was archived by the owner on Mar 23, 2023. It is now read-only.

[Runtime] Implementation of str.isspace(), isupper(), islower(), istitle()#245

Merged
trotterdylan merged 2 commits intogoogle:masterfrom
corona10:string_operations
Feb 12, 2017
Merged

[Runtime] Implementation of str.isspace(), isupper(), islower(), istitle()#245
trotterdylan merged 2 commits intogoogle:masterfrom
corona10:string_operations

Conversation

@corona10
Copy link
Copy Markdown
Contributor

@corona10 corona10 commented Feb 10, 2017

Implementation of str.isspace(), str.isupper(), str.islower(), str.istitle().

For the istitle() operation, I adopted a CPython's implementation from here.

@corona10 corona10 changed the title [Runtime] Implement str.isspace(), isupper(), islower(), istitle() [Runtime] Implementation of str.isspace(), isupper(), islower(), istitle() Feb 10, 2017
Copy link
Copy Markdown
Contributor

@trotterdylan trotterdylan left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I have a few comments.

Comment thread runtime/str.go Outdated
}

func isSpace(c byte) bool {
return c == ' ' || c == '\n' || c == '\t' || c == '\v' || c == '\f' || c == '\r'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may be more efficient as:

switch c {
  case ' ', '\n', '\t', '\v', '\f', '\r':
    return true
  default:
    return false
}

Comment thread runtime/str.go
}

func isLower(c byte) bool {
return 'a' <= c && c <= 'z'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you replace the other identical comparisons in this file (e.g. in isAlpha and strTitle, toUpper) with a call to this function? Ditto for isUpper().

Comment thread runtime/str.go Outdated
}

if len(s) == 1 {
if isUpper(s[0]) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return getBool(isUpper(s[0])).ToObject(), nil

First, I change some functions to work with
isLower() and isUpper() for identical operations.
Second, I modified isSpace() to be more efficiently by
using switch-case statment not using multiple comparision.
Copy link
Copy Markdown
Contributor

@trotterdylan trotterdylan left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for the changes. Merging shortly.

@trotterdylan trotterdylan merged commit d8cb498 into google:master Feb 12, 2017
@corona10 corona10 deleted the string_operations branch February 12, 2017 16:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants