Fix problems with existing code in addition to minor modifications

In the first place

The coding of the system I was in charge of was crazy. However, large-scale renovation projects have not come down. Then, the flow is to fix small projects little by little.

Current coding

For example, the search business class has the following structure.

  1. Validation
  2. Execution of search logic
  3. Edit screen display items

Looking at this alone, it is (probably) a normal configuration, but the actual coding looks like the following.

//Validation
//null check
if(joken1 == null ||
   joken2 == null ||
   joken3 == null ||
   ...               ) {
  error.add("error");
  return false;
}
//Empty string check
if(joken1 != null &&
   joken2 != null &&
   joken3 != null &&
   ...              ) {
  if(joken1 == "" ||
     joken2 == "" ||
     ...            ) {
  error.add("error");
  return false;
  }
}

//Object creation
ObInfo obInfo = new ObInfo();
ObResult obResult = new ObResult();
int kensakuResult = 0;

//Judgment of search conditions
if (joken == CONST.JOKEN1 ||
    joken == CONST.JOKEN2 ||
    joken == CONST.JOKEN3 ||
    ...                      ){
//Search execution
  kensakuResult = obInfo.getKensakuResult(joken1, joken2, ... , obResult);
  if(kensakuResult == 0) {
    error.add("error");
    return false;
  }
}

//Editing screen display items
ObKensakuInfoBean obKensakuInfoBean = new ObKensakuInfoBean();
editKensakuInfo(obKensakuInfoBean, kensakuResult);

//Warning
if (kensakuResult == 2){
  error.add("Warning");
} else if (kensakuResult == 3){
  error.add("Warning");
} else if (kensakuResult == 4){
  ...
}

return true;

The above is an overview of the business logic that is first called when executing a search. There are actually more validations, item edits, and edit comments. This editorial comment is awkward, making code that is already hard to see even harder to see.

/*20XX ○○ Project START*/
// ObInfo obInfo = new ObInfo();
/*20XX ○○ Project END*/
/*20XX XX Project START*/
// ObInfo obInfo 
/*20XX XX Project END*/
   obInfo
/*20XX △△ Project START*/
//            = obGetInfo.getInfo(obInfo, obAuthInfo, obCode);
/*20XX △△ Project END*/
/*20XX □□ Project START*/
              = obGetInfo.getInfo(obINfo);
/*20XX □□ Project END*/

It's like that. You can tell from one place here, but since it's in the whole code, it's hard to figure out the whole code. Currently, SVN is used for source control, so comments are not required, but it is customary to leave comments.

problem

This code is also working. Perhaps the simple conditions were sufficient at first. However, gradually complicated conditions were required. And this code doesn't (for some reason) take into account the addition of conditions. Inputs, if conditions, and validations are all described inside the business logic. Therefore, it was necessary to modify the conditions and validation inside the business logic every time a repair was made. Naturally, it is the full screen of the repaired part. Moreover, the specifications were different, such as the conditions being described in another class depending on the business logic, being written directly in the business logic, and being described in the method. In other words, the classes that can be modified for each repair location are different.

Correspondence

Normally, I think it's best to create a validation class and implement validation there. However, it is difficult to make a fundamental correction because the man-hours are determined by the number of correction steps and the creation of a small-scale project. So, this time, I mainly modified the search conditions.

How to respond

Regarding the search conditions, since we have set predetermined constants as conditions, we added them as an array and added as a new condition whether they match the conditions in the array.

if(CONST.JOKENS.contain(joken)){
 kensakuResult = ...
}

It's like that. Originally I wanted to put it all together, but doing so would require a huge regression test. (It is necessary to test that it works normally under the conditions so far) For simple quantity problems, integration testing is difficult, but unit testing alone can be done with JUnit. (If you have a JUnit test case that meets your existing requirements)

Unfortunately, I didn't even have a local environment without JUnit, so I gave up on the regression test. So, this time, only the newly added conditions are listed and supported.

if(joken == CONST.JOKEN ||
   ...
   CONST.JOKENS.contain(joken)){
  ...
}

It is like that.

Root cause

In the first place, I think the cause is that I neglected the detailed design at the time of startup. Only the basic design was clearly documented, and each person implemented it so that the screen display would meet the specifications. As a result, the behavior of the class was not specified, and the specifications for each class became different. I think that it is possible to create a detailed design based on the implementation for the time of renovation, but such man-hours do not go down. That's why I had no choice but to deal with these small corrections.

Summary

Let's set the design and coding standards properly. There is no man-hours for repairs such as adding classes to the operation / maintenance phase.

Recommended Posts

Fix problems with existing code in addition to minor modifications
How to implement UICollectionView in Swift with code only
Try to implement n-ary addition in Java
[Swift] How to fix Label in UIPickerView
How to fix system date in JUnit
[Java Bronze] 5 problems to keep in mind