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

Optimizations for a variable and size() for length(). #250

Closed
wants to merge 3 commits into from

Conversation

HyukjinKwon
Copy link
Member

I forgot to correct the variable in a past PR. This PR re-use the rowArray rather than creating it everytime. This was mentioned before in #169 (comment).

In addition, according to IntelliJ IDE, we might have to use length() instead of size().

This inspection reports array.size and string.size calls.

While such calls are legimimate, they require an additional implicit conversion to SeqLike to be made.

Considering the common use cases, calling length on arrays and strings may provide significant advantages.

Lastly, I gave a () for buildScan().

@codecov-io
Copy link

Current coverage is 86.66%

Merging #250 into master will not affect coverage as of ff0d5ef

@@            master    #250   diff @@
======================================
  Files           12      12       
  Stmts          540     540       
  Branches       160     160       
  Methods          0       0       
======================================
  Hit            468     468       
  Partial          0       0       
  Missed          72      72       

Review entire Coverage Diff as of ff0d5ef

Powered by Codecov. Updated on successful CI builds.

@HyukjinKwon
Copy link
Member Author

I see this was also fixed in Spark, apache/spark@e97fc7f.

@falaki Would you maybe merge this?

@falaki falaki closed this in 0c2551a Mar 21, 2016
rachelwarren pushed a commit to rachelwarren/spark-csv that referenced this pull request Sep 29, 2016
I forgot to correct the variable in a past PR. This PR re-use the `rowArray` rather than creating it everytime. This was mentioned before in databricks#169 (comment).

In addition, according to IntelliJ IDE, we might have to use `length()` instead of `size()`.
```
This inspection reports array.size and string.size calls.

While such calls are legimimate, they require an additional implicit conversion to SeqLike to be made.

Considering the common use cases, calling length on arrays and strings may provide significant advantages.
```

Lastly, I gave a `()` for `buildScan()`.

Author: hyukjinkwon <gurwls223@gmail.com>

Closes databricks#250 from HyukjinKwon/optimization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants