Skip to content

Conversation

@arturobernalg
Copy link
Member

No description provided.

integers[i] = (int) chars[i];
}
return integers;
return Arrays.stream(chars).map(aChar -> (int) aChar).toArray(Integer[]::new);
Copy link
Member

@garydgregory garydgregory Oct 31, 2022

Choose a reason for hiding this comment

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

You don't need streams @arturobernalg
Why not just:

        final Integer[] integers = new Integer[chars.length];
        Arrays.setAll(integers, i -> (int) chars[i]);

?

Copy link

Choose a reason for hiding this comment

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

If I may jump in, personally and strictly personally :), I find stream-map-reduce flow to be somehow more readable. I immediately recognize that we are converting one set of data to another set. It would help even more with proper formatting where each stream operation is in a separate line. It could be argued that using a functional paradigm for data manipulation as of late is - idiomatic in Java.

With setAll approach I (personally) need to think for a moment about what is actually happening.

After all, there is no right way -> both imperative and functional approaches are correct, only matters of personal preference.

intersection++;
}
}
int intersection = (int) setA.stream().filter(setB::contains).count();
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need the local variable?

Copy link
Member

Choose a reason for hiding this comment

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

Whichever way we go, we probably need to Javadoc the truncation (PR) or looping of ints (current master).

Copy link
Member Author

Choose a reason for hiding this comment

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

inline.
meanwhile I have change your remarks.

@garydgregory
Copy link
Member

The code does not compile @arturobernalg

@codecov-commenter
Copy link

Codecov Report

Merging #375 (8d054e7) into master (9692573) will increase coverage by 0.03%.
The diff coverage is 96.15%.

@@             Coverage Diff              @@
##             master     #375      +/-   ##
============================================
+ Coverage     97.12%   97.15%   +0.03%     
+ Complexity     2324     2315       -9     
============================================
  Files            84       84              
  Lines          5770     5695      -75     
  Branches        935      895      -40     
============================================
- Hits           5604     5533      -71     
+ Misses           87       83       -4     
  Partials         79       79              
Impacted Files Coverage Δ
...c/main/java/org/apache/commons/text/WordUtils.java 93.56% <0.00%> (+1.81%) ⬆️
...ava/org/apache/commons/text/AlphabetConverter.java 96.59% <100.00%> (-0.03%) ⬇️
...org/apache/commons/text/RandomStringGenerator.java 92.95% <100.00%> (-0.47%) ⬇️
.../main/java/org/apache/commons/text/StrBuilder.java 97.34% <100.00%> (-0.05%) ⬇️
...ain/java/org/apache/commons/text/StrTokenizer.java 98.77% <100.00%> (-0.02%) ⬇️
.../java/org/apache/commons/text/StringTokenizer.java 98.64% <100.00%> (-0.02%) ⬇️
...ava/org/apache/commons/text/TextStringBuilder.java 97.68% <100.00%> (-0.05%) ⬇️
...he/commons/text/matcher/AbstractStringMatcher.java 100.00% <100.00%> (ø)
...ache/commons/text/similarity/CosineSimilarity.java 100.00% <100.00%> (ø)
...pache/commons/text/similarity/HammingDistance.java 71.42% <100.00%> (-8.58%) ⬇️
... and 4 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@garydgregory
Copy link
Member

@arturobernalg there is nothing to review until the builds are green ;-)

@arturobernalg
Copy link
Member Author

@arturobernalg there is nothing to review until the builds are green ;-)
Hey @garydgregory
Yep. looks green now.
TY

@garydgregory garydgregory changed the title Use java8 Stream. Use Java 8 Stream Nov 1, 2022
@sergmain
Copy link

Just curious, what is the point to change the code which can be compiled directly to processor commands with code which makes tons of calls with stack usage?
And after implementing
"Project Valhalla: Value types, objects without identity but with an efficient memory layout and leading to better results of escape analysis".
everything will be reverted back?

@RockyMM
Copy link

RockyMM commented Jun 16, 2023

@sergmain in my perspective, streams do bring a lot in readability and maintainability.

Regarding performance, it's difficult to say with guessing. I think we need proper performance testing, but as a layman, I don't expect much difference in performance. Streams are already well optimized in Java and I trust that performance difference will be negligible.

@arturobernalg arturobernalg closed this by deleting the head repository Jun 21, 2023
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.

5 participants