Skip to content

Conversation

graycreate
Copy link
Member

Summary

Changes

  • Renamed variable: totalStrtotalCountRaw for better clarity about its purpose
  • Added null/empty check: Validates the raw string before attempting to parse
  • Improved exception handling: Uses specific NumberFormatException instead of generic Exception
  • Added error logging: Logs parsing failures with the problematic value for debugging
  • Added descriptive comment: Documents that the field contains raw HTML data with thousand separators

Testing

  • Tested with nodes containing large topic counts (100,000+)
  • Verified proper handling of null/empty values
  • Confirmed error logging works when parsing fails

Related

🤖 Generated with Claude Code

- Rename totalStr to totalCountRaw for better clarity
- Add null/empty check before parsing
- Use specific NumberFormatException instead of generic Exception
- Add error logging for failed parsing attempts
- Add descriptive comment explaining the field purpose

This is a follow-up to PR #82, implementing improvements suggested by GitHub Copilot review.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings September 18, 2025 15:26
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the node topic count parsing logic in NodeTopicInfo.java to improve error handling and code clarity. The changes enhance robustness when parsing topic counts from HTML data that may contain formatting like thousand separators.

  • Renamed variable for better clarity and added null/empty validation
  • Replaced generic exception handling with specific NumberFormatException handling
  • Added error logging for debugging parsing failures

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


@Pick("span.topic-count strong")
public String totalStr;
private String totalCountRaw; // Raw topic count string from HTML (may contain thousand separators)
Copy link
Preview

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

The field totalCountRaw is changed from public to private but there's no corresponding public getter method. This could break existing code that directly accesses the field. Consider adding a getter method or maintaining public visibility if external access is needed.

Copilot uses AI. Check for mistakes.

return Integer.parseInt(totalStr.replaceAll("[^0-9]", ""));
} catch (Exception e) {
// Remove thousand separators (commas) and any other non-numeric characters
String cleanedNumber = totalCountRaw.replaceAll("[^0-9]", "");
Copy link
Preview

Copilot AI Sep 18, 2025

Choose a reason for hiding this comment

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

If cleanedNumber becomes an empty string after removing all non-numeric characters, Integer.parseInt() will throw a NumberFormatException. Add a check to ensure the cleaned string is not empty before parsing.

Suggested change
String cleanedNumber = totalCountRaw.replaceAll("[^0-9]", "");
String cleanedNumber = totalCountRaw.replaceAll("[^0-9]", "");
if (cleanedNumber.isEmpty()) {
return 0;
}

Copilot uses AI. Check for mistakes.

@graycreate graycreate merged commit fa630c3 into main Sep 18, 2025
5 checks passed
@graycreate graycreate deleted the feature/improve-node-topic-parsing branch September 18, 2025 15:28
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.

1 participant