Conditional check for data-source-ref is incorrect

[sheriumair]

This PR removes a redundant conditional check in the doParse method of the specified class. The condition dataSource != null is always evaluated as true because the getAttribute method always returns a value. By removing this redundant check, the code becomes more concise and easier to read while maintaining the error handling logic when necessary. This change aligns with best practices for code simplification and improves code quality.

[pivotal-cla]

@sheriumair Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@sheriumair Thank you for signing the Contributor License Agreement!

[sjohnr]

@sheriumair thanks for the PR!

It's possible the original code was incorrect (this code goes back to 2007), but the null check appears to have been intended to handle the situation where the data-source-ref attribute is missing.

while maintaining the error handling logic when necessary.

I'm not sure the fix is correct. Can you please update the PR to correctly fix the issue and add a test asserting the behavior you are fixing (e.g. an XML with missing data-source-ref)?

[sheriumair]

Hey @sjohnr ! Thanks for pointing out this issue in my PR. dataSource variable can be empty but not null. So if it is empty then we have to handle that case. I will correct this issue in my upcoming PR and write a test where data-source-ref is missing.

[sheriumair]

Hey @sjohnr! Can you kindly review the PR, please?

[sjohnr]

Thanks for the PR @sheriumair. Please see my comments below.

[sheriumair]

Hey @sjohnr. Thank you for your review. I have changed to hasText and added another test when data-source-ref attribute is entirely missing.

[sheriumair]

Hey @sjohnr ! Apologies for the oversight, I committed the code without running the Checkstyle analysis. I have now corrected the code to adhere to the coding standards and have ensured that it passes the Checkstyle checks. Please review the updated PR. Thank you for your understanding.

[sjohnr]

Thanks for the updates @sheriumair! Please see additional comments below.

[sjohnr]

Once you are finished with the above updates, would you mind please squashing your commits?

[sheriumair]

Hey @sjohnr! I have squashed my commits and changed the XML to multi line

[sjohnr]

Hi @sheriumair! Looks like there's still two commits? I think there should only be one.

[sheriumair]

Hey @sjohnr . I hope my latest PR solves all the issues.

[sheriumair]

Hey @sjohnr! Is there anything else that is left on my side?

[sjohnr]

@sheriumair I don't believe so. I will work on getting this merged tomorrow. If anything is remaining I will apply a polish commit. Thanks for your contribution!

[sjohnr]

Thanks @sheriumair! This has now been merged as 33ebd54 with polish commit 39dbd24.