Convert zoneId String in RepoConfiguration and SummaryJson to ZoneId object

[yhtMinceraft1010X]

What feature(s) would you like to see in RepoSense?

I noticed that although zoneId is stored as a ZoneId object in CliArguments.java, it is then converted to a String when stored in RepoConfiguration.java, subsequently, when zoneId needs to be used in places like FileInfoAnalyzer.java, it has to do the conversion back to ZoneId.

Is the feature request related to a problem?

This conversion from ZoneId to String to ZoneId is redundant. In my opinion, it is also quite strange to have zoneId be a String, when the variables that zoneId is commonly used with, sinceDate and untilDate, are LocalDateTime objects.

If possible, describe the solution

Since SummaryJson objects for repos are written into Json files during tests, changing zoneId to be ZoneId instead of String may cause the Json output to be different from expected Json files, potentially breaking the tests.

• If the above scenario happens, it is okay to keep the zoneId variable as a String in SummaryJson.
• However, the conversion from String to ZoneId in SummaryJson should be attempted and tested first before reverting back if tests are broken.
• Recommended approach:
  1. Follow step 2 of the Learning the Basics guide for backend. Throughout this step, take note of how the zoneId variable is used in the code.
  2. Convert zoneId variable from String to ZoneId in RepoConfiguration.java and propagate the change to any classes and methods (inclusive of test code) that break after this change.
  3. Repeat the above steps for SummaryJson.java.
  4. Run tests and checkstyle. Commit these changes if successful.

Additional context

This should be a good first-timer issue for future contributors such as CP3108A/B students. The tracing of the zoneId variable and the subsequent refactoring should provide enough exposure to the backend code.

[SkyBlaise99]

Hi, I would like to ask if it is ok for me to work on this issue?

[yhtMinceraft1010X]

Hi, I would like to ask if it is ok for me to work on this issue?

Sure.

[yhtMinceraft1010X]

I've edited the recommended approach. It is best to make the zoneId type changes across all files and all relevant changes needed to pass CI within the same commit.

[SkyBlaise99]

Ok thank you 😄

[SkyBlaise99]

Should I keep the setters to allow String inputs? Basically there will be 2 setters for ZoneId.

public void setZoneId(ZoneId zoneId) {
    this.zoneId = zoneId;
}

public void setZoneId(String zoneIdString) {
    this.zoneId = ZoneId.of(zoneIdString);
}

If not, then in some of the test cases there will be

config.setZoneId(ZoneId.of("UTC-0530"));

which is a bit longer than the original

config.setZoneId("UTC-0530");