OP-1150 OP-1151 OP-1153 OP-1154 OP-1155 Refactoring smells #1175
Conversation
…ted for resolution
…ted for resolution
…ted for resolution
Dharmil4602
changed the title
| @@ -508,4 +507,8 @@ else if (dateOut.isBefore(ad.getAdmDate()) || dateOut.equals(ad.getAdmDate())) { | |||
| throw new OHDataValidationException(errors); | |||
| } | |||
| } | |||
| public boolean isAdmissionActiveOnDate(Admission admission, LocalDateTime checkDate) { | |||
There was a problem hiding this comment.
I see little value in extracting the conditional into a method given the overhead that is required to invoke the method. I don't believe this should be done.
| @@ -0,0 +1,61 @@ | |||
| package org.isf.permissions.test; | |||
There was a problem hiding this comment.
Missing license header in multiple files. Please see the .license-header directory in this repo.
| import org.mockito.Mock; | ||
| import org.mockito.MockitoAnnotations; | ||
| import java.util.List; | ||
| import static org.junit.jupiter.api.Assertions.*; |
There was a problem hiding this comment.
The project standards do not support package level imports. Please see the .ide-settings directory in this repo for import ordering and code formatting. This applies to multiple files in this PR.
| Permission result = permissionIoOperations.insertPermission(permission); | ||
|
|
||
| // Assert | ||
| assertNotNull(result); |
There was a problem hiding this comment.
The project standards are to use aspectj; please adopt the standard. This applies to multiple files in this PR.
…hod refactoring with build success and better readability
Dharmil4602
changed the title
mwithi
changed the title
mwithi
changed the title
| @@ -0,0 +1,434 @@ | |||
| package org.isf.dicom.manager; | |||
There was a problem hiding this comment.
missing license header
| import com.drew.metadata.exif.ExifDirectoryBase; | ||
| import com.drew.metadata.exif.ExifIFD0Directory; | ||
| import static org.isf.dicom.manager.DicomLoader.getSeriesDateTime; | ||
| import static org.isf.dicom.manager.DicomLoader.getStudyDateTime; |
There was a problem hiding this comment.
static imports are at the first ones in the list.
| @@ -0,0 +1,28 @@ | |||
| package org.isf.utils.exception; | |||
There was a problem hiding this comment.
missing license header
| @@ -0,0 +1,28 @@ | |||
| package org.isf.utils.exception; | |||
| import org.junit.jupiter.api.Test; | |||
| import static org.junit.jupiter.api.Assertions.assertEquals; | |||
There was a problem hiding this comment.
project standards are to use aspectj (I.e., assertThat())
|
At a minimum please Code Style so there are fewer issues that we need to comment on. |
|
@Dharmil4602 please read my comment at https://openhospital.atlassian.net/browse/OP-1154?focusedCommentId=15187 |
mwithi
changed the title
See OP-1150, OP-1151, OP-1153, OP-1154 and OP-1155.
This pull request addresses the "smell refactoring" by various refactoring techniques like Rename variable, move method. The refactoring effort aims to enhance code readability and maintainability by breaking down complex conditional statements into more modular and understandable components.
Changes Made:
Issue [Complex Conditional]: https://openhospital.atlassian.net/browse/OP-1150
Issue [Feature Envy]: https://openhospital.atlassian.net/browse/OP-1153