Skip to content

Rewrote null check to use mempty#68

Merged
garyb merged 2 commits into
purescript:masterfrom
Risto-Stevcev:master
Oct 8, 2016
Merged

Rewrote null check to use mempty#68
garyb merged 2 commits into
purescript:masterfrom
Risto-Stevcev:master

Conversation

@Risto-Stevcev
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/Data/String.purs Outdated
-- | Returns `true` if the given string is empty.
null :: String -> Boolean
null s = length s == zero
null = eq mempty
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.

Might as well use "" here and avoid the dictionary lookup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I knew there must've been a reason why mempty isn't being used here. Is a JSON lookup amortized O(1)?

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 8, 2016

I'm actually baffled as to why it was implemented the way it was! The length approach is how it'd have to be implemented in Haskell (due to type String = [Char]), but comparison with "" would always be better for us.

It might be better to implement this without point-free though, as string equality can be inlined if it's fully applied: null s = s == "".

@garyb
Copy link
Copy Markdown
Member

garyb commented Oct 8, 2016

Thanks!

@garyb garyb merged commit 6843c74 into purescript:master Oct 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants