Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify some code #3580

Closed
wants to merge 2 commits into from
Closed

Simplify some code #3580

wants to merge 2 commits into from

Conversation

OLPMO
Copy link

@OLPMO OLPMO commented Aug 27, 2019

Remove some redundant 'if' statement.Besides, I use .toArray(new MyClass[0]) replace .toArray(new MyClass[myList.size()]) for improving the efficiency of the code.
https://stackoverflow.com/questions/174093/toarraynew-myclass0-or-toarraynew-myclassmylist-size

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@OLPMO
Copy link
Author

OLPMO commented Aug 27, 2019

@googlebot I signed it!

1 similar comment
@OLPMO
Copy link
Author

OLPMO commented Aug 27, 2019

@googlebot I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@ljacqu
Copy link

ljacqu commented Aug 27, 2019

-1 on the LocalCache changes, just as commented in PR #3492 this fixes what is presumably reported by an IDE at the expense of readability for no real reason.

@OLPMO
Copy link
Author

OLPMO commented Aug 28, 2019

@ljacqu oh,sorry. I prefer the version that I commit because I would do the same thing in my daily coding.I can also understand that different projects have different coding styles. Do you mean that all the changes of 'if' statement is unnecessary? If you think so, I would revert them into the master version!

@OLPMO
Copy link
Author

OLPMO commented Aug 29, 2019

@ljacqu I think that 'sum == 0' needs to be preserved because 'sum == 0' itself has the meaning of isEmpty. Positive thinking is more conducive to people's understanding than negative thinking.If you do not agree with that. I can also revert other 'if' statement to the master version.
Look forward to your reply.

@ljacqu
Copy link

ljacqu commented Aug 29, 2019

Sorry if there was any misunderstanding, I‘m not in any way affiliated/involved with Guava – I just posted a comment as an individual passing by :)

@OLPMO
Copy link
Author

OLPMO commented Sep 3, 2019

@ljacqu That’s all right.I am very glad that you can reply to this pr.Thanks for your suggestion

cpovirk pushed a commit that referenced this pull request Oct 2, 2019
Fixes #3580

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272440567
@cpovirk cpovirk mentioned this pull request Oct 2, 2019
@cpovirk cpovirk closed this in #3635 Oct 2, 2019
cpovirk pushed a commit that referenced this pull request Oct 2, 2019
Fixes #3580

RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=272440567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants