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

[Enhancement] Remove unnecessary code #9147

Open
1 task done
mawen12 opened this issue Jan 20, 2025 · 0 comments · May be fixed by #9148
Open
1 task done

[Enhancement] Remove unnecessary code #9147

mawen12 opened this issue Jan 20, 2025 · 0 comments · May be fixed by #9148

Comments

@mawen12
Copy link
Contributor

mawen12 commented Jan 20, 2025

Before Creating the Enhancement Request

  • I have confirmed that this should be classified as an enhancement rather than a bug/feature.

Summary

The method checkConfig in DefaultMQPullConusmerImpl and DefaultMQPushConsumerImpl exists unused code about producer group.

Validators#checkGroup has checked the group not empty, so don't need to check in checkConfig.

public class DefaultMQPullConusmerImpl {
    // ...
    private void checkConfig() throws MQClientException {
        // because this method has check group not empty
        Validators.checkGroup(this.defaultMQPullConsumer.getConsumerGroup());

        // this check should be unnecessary
        if (null == this.defaultMQPullConsumer.getConsumerGroup()) {
            throw new MQClientException(
                "consumerGroup is null"
                    + FAQUrl.suggestTodo(FAQUrl.CLIENT_PARAMETER_CHECK_URL),
                null);
        }
        // ...
    }
    // ...
}
public class Validators {
    // ...
    public static void checkGroup(String group) throws MQClientException {
        // check group is not null or empty
        if (UtilAll.isBlank(group)) {
            throw new MQClientException("the specified group is blank", null);
        }
        // ...
    }
    // ...
}

Motivation

producer group don't need repeat check.

Describe the Solution You'd Like

Delete unnecessary code.

Describe Alternatives You've Considered

No.

Additional Context

No response

@mawen12 mawen12 linked a pull request Jan 20, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant