fix #11 #12
fix #11 #12
Conversation
src/is_hiragana.rs
Outdated
| return false; | ||
| } | ||
| input.chars().all(is_char_hiragana) | ||
| !input.is_empty() && input.chars().all(is_char_hiragana) |
There was a problem hiding this comment.
I think the previous version is more readable
There was a problem hiding this comment.
Imo this is cleaner as the function represents a single bool expression that is written there. But I actually didn't intend to put this into this PR.
There was a problem hiding this comment.
The single bool expression is the reason why it's less readable.
What happens on empty input? -> need to look at the whole bool expression instead of one line
What happens on other input? -> need to look at the whole bool expression instead of one line
Clean is quite subjective, but according to the "Clean Code" book this is discouraged (The Stepdown Rule)
There was a problem hiding this comment.
Yeah its indeed pretty subjective and as I didn't intend to PR this part anyways I'm fine with keeping it the way it was. I just changed it while going through the code when adjusting the lib for my needs.
There was a problem hiding this comment.
can you update the PR? LGTM to me, except the bool expression
|
|
||
| [dev-dependencies] | ||
| speculate = "0.1.2" | ||
|
|
||
| [features] | ||
| enable_regex = ["regex"] | ||
| tokenize = ["itertools"] | ||
| default = [] |
There was a problem hiding this comment.
tokenize should be a default feature
There was a problem hiding this comment.
I personally use this only to convert kana and I think there lots of users of wanakana use it for this purpose only as well. But this also wasn't intended to be added to this pr.
There was a problem hiding this comment.
I have no problem to add this to the lib. Is there a problem with the itertools dependency? (no_std support?)
There was a problem hiding this comment.
I just prefer to have as less dependencies as possible in my code or better said I see it as a good practice to keep them as few as possible and I don't need tokenize in my current project.
src/is_japanese.rs
Outdated
| return false; | ||
| } | ||
| input.chars().all(is_char_japanese) | ||
| !input.is_empty() && input.chars().all(is_char_japanese) |
There was a problem hiding this comment.
I think the previous version is more readable
|
Thanks! |
Fixes #11