Skip to content

Conversation

6U-U9
Copy link
Contributor

@6U-U9 6U-U9 commented Mar 23, 2022

What is the purpose of the change

Constant charset defined by String (for example, "UTF-8") can be replaced with the predefined StandardCharsets.UTF_8 code.
The code after the fix may work faster, because the charset lookup becomes unnecessary.

Brief changelog

Replace Charset.forName("UTF-8") with StandardCharsets.UTF_8

@Git-Yang
Copy link
Member

Can an ISSUE be created for this PR?
In addition, can the code formatting of SimpleProducer be adjusted?

@6U-U9
Copy link
Contributor Author

6U-U9 commented Mar 30, 2022

#4068 I have created an issue for this PR.
And accoding to the massage sending process in SimplePullConsumer, there is no need for specify Charset in SimpleProducer, so I removed them.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 51.93% when pulling be4de18 on 6U-U9:develop into 4574d59 on apache:develop.

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2022

Codecov Report

Merging #4024 (be4de18) into develop (4574d59) will decrease coverage by 0.00%.
The diff coverage is 88.88%.

@@              Coverage Diff              @@
##             develop    #4024      +/-   ##
=============================================
- Coverage      47.90%   47.90%   -0.01%     
- Complexity      5001     5002       +1     
=============================================
  Files            633      633              
  Lines          42523    42523              
  Branches        5573     5573              
=============================================
- Hits           20371    20369       -2     
- Misses         19654    19660       +6     
+ Partials        2498     2494       -4     
Impacted Files Coverage Δ
...c/main/java/org/apache/rocketmq/common/MixAll.java 44.44% <0.00%> (ø)
...java/org/apache/rocketmq/acl/common/AclSigner.java 81.48% <100.00%> (ø)
...apache/rocketmq/acl/common/SessionCredentials.java 56.94% <100.00%> (ø)
...er/transaction/queue/TransactionalMessageUtil.java 80.00% <100.00%> (ø)
...apache/rocketmq/common/message/MessageDecoder.java 77.65% <100.00%> (ø)
...a/org/apache/rocketmq/filter/util/BloomFilter.java 62.63% <100.00%> (ø)
...cketmq/remoting/protocol/RemotingSerializable.java 62.50% <100.00%> (ø)
...cketmq/remoting/protocol/RocketMQSerializable.java 86.45% <100.00%> (ø)
...etmq/tools/command/message/SendMessageCommand.java 74.32% <100.00%> (ø)
...va/org/apache/rocketmq/store/FlushDiskWatcher.java 81.25% <0.00%> (-9.38%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4574d59...be4de18. Read the comment docs.

System.out.printf("Producer startup OK%n");

{
Message message = producer.createBytesMessage("OMS_HELLO_TOPIC", "OMS_HELLO_BODY".getBytes(Charset.forName("UTF-8")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use StandardCharsets.UTF_8 ? IMO, some os may use gbk as default decoder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's necessary to specify the encode charset for message body.
As long as one encode and decode the message with the same charset, he shall get the correct message.

@yuz10 yuz10 merged commit d100d3c into apache:develop Apr 17, 2022
GenerousMan pushed a commit to GenerousMan/rocketmq that referenced this pull request Aug 12, 2022
…rsets.UTF_8 (apache#4024)

* Replace Charset.forName("UTF-8") with StandardCharsets.UTF_8

* Remove the specification of Charset according to SimplePullConsumer
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.

6 participants