Skip to content

Conversation

@mohammedhashim790
Copy link
Contributor

Hello ,

I have made the following changes in DesigniteJava.

Type Description
Refactoring Improved SRP by moving file reading/listing logic to a single file.
Extract Class Introduced Arguments (abstract class) with RegularArgumentParser & CLIArgumentParser for ease of development.
Extract Method Moved shared logic of createOption between CLI & Regular Arg Parser to Arguments class.
Complex Condition Simplification Wrapped getLogFileName & ensureOutFolderExists within setOutputDirectory and FileManager.createFileIfNotExists.
Move Method Moved method such as readFileToString, listFilesFromFolder, listFiles to FileManager in the application.

@mohammedhashim790
Copy link
Contributor Author

Hello,

I have fixed the following,

  1. Removed the inline class comments from Argument Parser as the docs has been provided on sub classes.
  2. Closing the duplicate merge request Refactored Code Smell Detectors and Resolved Code Issues #86 as the changes are synced to master branch.

Providing the updated changes of #86 below for your reference.

Fixes

Refactoring Technique Class Issue Fix
Unfactored Hierarchy ImplementationSmellDetector, DesignSmellDetector ImplementationSmellDetector and DesignSmellDetector classes had an unfactored hierarchy, leading to redundancy and complexity. Resolved the unfactored hierarchy by consolidating the ImplementationSmellDetector and DesignSmellDetector classes into a single class, CodeSmellDetector, to streamline the structure.
Extract Class CodeSmellDetector, ImplementationSmellDetector, DesignSmellDetector Code smells were handled separately in ImplementationSmellDetector and DesignSmellDetector, leading to duplication. Consolidated ImplementationSmellDetector and DesignSmellDetector into a single class, CodeSmellDetector, to reduce redundancy and centralize logic.
Pull Up Method and Variable CodeSmellDetector, ImplementationSmellDetector, DesignSmellDetector Duplicate methods (addToSmells, initializeCodeSmell) and variables (smells) across subclasses. Pulled up the smells variable, addToSmells, and initializeCodeSmell methods into the parent class CodeSmellDetector to promote code reuse and reduce duplication.
Introduce Variable for Magic Number ImplementationSmellDetector Magic number 16 was being used directly in the code, violating best practices. Introduced a new constant variable LONG_RADIX = 16 in the ImplementationSmellDetector class to replace the magic number.
Rename Method ImplementationSmellDetector Method getBooleanRegex() was unclear and inconsistent. Renamed getBooleanRegex() to a more descriptive name to improve clarity.
Rename Method Resolver Method infereParameterized() had a typo in its name. Renamed infereParameterized() to inferParameterized() to correct the spelling and improve readability.

@tushartushar
Copy link
Owner

Hi @mohammedhashim790
I dont think we should apply the first two refactorings. We would like to keep the classes separate for implementation and design smells. If there is any duplicate code (unfactored hierarchy) then we may move that to the super class (pull up methods/fields). Rest all look reasonable.

@mohammedhashim790
Copy link
Contributor Author

mohammedhashim790 commented Mar 21, 2025

Hello Prof. @tushartushar,

I realized that there are common methods (addToSmells() and initialiseCodeSmell()) and a variable (smells) used in both Implementation and Design Smell Detectors. Hence, I created a new class called CodeSmellDetector that inherits these identifiers, allowing them to be used for both types of Smell Detectors.

Additionally, I applied an unfactored hierarchy to the smell detectors since they are unique in implementation but share common properties and methods with different implementations.

Please let me know your thoughts. Would be happy to know about it.

Screenshot for reference.

image

.DS_Store Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this.

@tushartushar
Copy link
Owner

Hi @mohammedhashim790
I am in general fine with the changes. Once you remove the .DS_Store file, I will merge the PR.

@mohammedhashim790
Copy link
Contributor Author

Hi @mohammedhashim790 I am in general fine with the changes. Once you remove the .DS_Store file, I will merge the PR.

Hello Prof. @tushartushar ,

I removed the .DS_Store file and added it to .gitignore to prevent MAC-specific files from being included in the future.

Thanks,

@tushartushar
Copy link
Owner

Thanks @mohammedhashim790
The CI is failing due to failed test cases. Can you please fix tests?

@mohammedhashim790
Copy link
Contributor Author

Thanks @mohammedhashim790 The CI is failing due to failed test cases. Can you please fix tests?

Hello Prof. @tushartushar ,

Due to refactoring of FileNames and paths, the tests failed. The build failure has been fixed and the following changes were done.

Fixes

File Reason for Failure Fix
SM_Method_CalledMethodsTests.java Global refactoring of Logger to DJLogger changed test values. Renamed DJLogger back to Logger as the class still exists on TestMethods.
SM_PackageTest.java The src directory now contains only Designite.java, while it initially had InputArgs too. Changed expected type count from 2 to 1.
SM_ProjectTest.java Additional package ArgumentParser increased package count. Updated expected package count from 21 to 22.

All tests have successfully passed the required stage, and I've attached the report for your reference.

Let me know if you need any further updates!

Screenshot 2025-03-21 at 17 24 26

@tushartushar tushartushar merged commit 3321c56 into tushartushar:master Mar 21, 2025
1 check passed
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.

2 participants