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

Condition 'admin!=null' is always 'true' #132

Open
lukasz-cpu opened this issue Sep 13, 2021 · 2 comments
Open

Condition 'admin!=null' is always 'true' #132

lukasz-cpu opened this issue Sep 13, 2021 · 2 comments
Labels
question Further information is requested

Comments

@lukasz-cpu
Copy link

lukasz-cpu commented Sep 13, 2021

https://github.com/xaecbd/KafkaCenter/blob/30e980fdae80de0be65a855786d8263e4b9ab7eb/KafkaCenter-Core/src/main/java/org/nesc/ec/bigdata/service/KafkaAdminService.java#L76

	public boolean kafkaIsHeath(String brokerAddr) {
		boolean flag = false;
		KafkaAdmins admin = null;
		try {
			Properties props = new Properties();
			props.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG, brokerAddr);
		    admin = new KafkaAdmins(props);
			if(admin!=null) {
				flag = true;
			}
		} catch (Exception e) {
			LOGGER.warn("connect kafka error.",e);
		}finally {
			try {
				if(admin!=null) {
					admin.close();
                }
			} catch (IOException ignored) {}
		}
		return flag;

	}
if(admin!=null) {
			flag = true;
		}

Probably this conditions always evaluate to true

@TrumanDu
Copy link
Contributor

@lukasz-cpu if connect kafka cluster has error. flag to false.

@TrumanDu TrumanDu added the question Further information is requested label Sep 14, 2021
@lukasz-cpu
Copy link
Author

lukasz-cpu commented Sep 14, 2021

@TrumanDu

I am not writing about the whole method, but about a piece of code

My point is that in line 75 https://github.com/xaecbd/KafkaCenter/blob/30e980fdae80de0be65a855786d8263e4b9ab7eb/KafkaCenter-Core/src/main/java/org/nesc/ec/bigdata/service/KafkaAdminService.java#L75

Will never be null in the admin variable. It's not possible for a constructor to return null, and even if an exception would be thrown from the constructor, the next line won't be called.

So checking it later

https://github.com/xaecbd/KafkaCenter/blob/30e980fdae80de0be65a855786d8263e4b9ab7eb/KafkaCenter-Core/src/main/java/org/nesc/ec/bigdata/service/KafkaAdminService.java#L76

is redundant...

If the program fails to create an object new KafkaAdmins(props) immediately jumps to the line where the exception is throw, line: 79.

https://github.com/xaecbd/KafkaCenter/blob/30e980fdae80de0be65a855786d8263e4b9ab7eb/KafkaCenter-Core/src/main/java/org/nesc/ec/bigdata/service/KafkaAdminService.java#L79

For a better understanding of the problem, I am attaching a screen from Sonar

asd

I hope that now my comment will be better to understand, kind regards! :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants