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

Refactoring Parts of Java code #1007

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Lonewolfreturns
Copy link

@Lonewolfreturns Lonewolfreturns commented Mar 31, 2024

Description

Hello

This is Akash. I am using your repo for learning more about Code smells. I was able to identify some in some of the documents.
I did some changes and refactored the codes. Hope you go through them and make the necessary changes in the main.

Thanking you in advance
AKASH

Checklist:

  • I have read the Contribution Guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

Contributor License Agreement

By submitting this pull request, I acknowledge and agree that my contributions will be included in Stirling-PDF and that they can be relicensed in the future under the MPL 2.0 (Mozilla Public License Version 2.0) license.

(This does not change the general open-source nature of Stirling-PDF, simply moving from one license to another license)

addEndpointToGroup("Javascript", "compare");
addEndpointToGroup("Javascript", "adjust-contrast");
private void initialize() {
addEndpointsToGroup(
Copy link
Member

Choose a reason for hiding this comment

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

This is cleaner code but I dislike the readability of it, the group name looks like a endpoint here
Perhaps dont use elispe operator and just new a new String[] for each?

Comment on lines +134 to +146
private File createTempFile(MultipartFile multipartFile) throws IOException {
File tempFile = GeneralUtils.convertMultipartFileToFile(multipartFile);
tempFile.deleteOnExit();
return tempFile;
}

private String getMergedFileName(MultipartFile[] files) {
return files[0].getOriginalFilename().replaceFirst("[.][^.]+$", "") + "_merged.pdf";
}

private void cleanupTempFiles(List<File> tempFiles) {
for (File file : tempFiles) {
file.delete();
Copy link
Member

Choose a reason for hiding this comment

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

I feel like these are all operations that could fit within the Util class and used elsewhere if needed?

pagesToCheck = pagesToCheck.replaceAll("\\s+", "");

String[] splitPoints = pagesToCheck.split(",");
for (String splitPoint : splitPoints) {
if (splitPoint.contains("-")) {
// Handle page ranges
Copy link
Member

Choose a reason for hiding this comment

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

Why the removal of comments here? what smell was this?

Copy link

sonarcloud bot commented Apr 1, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

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.

None yet

2 participants