Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The funny thing is, as far as I have seen:
You are the only one who enforces this.
Almost everyone else uses
if str != ""
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah because it was quite some time ago decided to use
len(str) == 0
so I'm manually linting it 😆There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also vote for
s != ""
, although there is no difference when the code gets compiled.s != ""
len(s) != 0
The first one is much shorter, and easy to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a(nother) vote on that?
I think that's the fastest way to resolve this, and then everyone knows how to handle this case in the future,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind for any of options, ex golang stdlib also uses both we just need to keep to the one in our code base.
If I remember correctly len was decided to use because you don't have to think if it's string, byte array or rune array, it will work equally good for all cases. That affects also refactoring later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem however is that as far as I have seen, both are used currently inside the code.
Sometimes
str != ""
is used, and sometimes (most likely when you reviewed the PR)len(str) != 0
is used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this point is not that important. When we use a
string
, we do not news := make()
, we do not iteratefor x := range s
, we do not updates[k] = v
, and astring
is immutable while array/map are mutable. They differ so much so we do not have to uselen
to all of them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not the one to propose it back in the days so don't shoot the messenger 😋 I was just pointing out that that's what was agreed on and when I review and see that I just point it out. I'm used to using it myself and for myself it's easier to use len than compare to empty string as I don't have to think so much on a type used. Sadly I can't seem to find where it was discussed as it was so long ago 😂