Summing two time periods using JodaTime

I'm building a Java method to sum arithmetically two time periods, using the JodaTime library. My code works fine, but I think that it's possible optimize to reduce the time execution.

```
import org.joda.time.Period;
import org.joda.time.PeriodType;
import org.joda.time.format.PeriodFormatter;
import org.joda.time.format.PeriodFormatterBuilder;

public class Test {

public static void main(String[] args)
{
String time1 = "08:00";
String time2 = "08:00";

System.out.println(Operation_Sum(time1,time2));
}

private static String Operation_Sum(String time1, String time2)
{
String output;
long start = System.currentTimeMillis();

if (time1.equals("00:00") && time2.equals("00:00"))
{
output = "00:00";
}
else if (time1.equals("00:00"))
{
output = time2;
}
else if (time2.equals("00:00"))
{
output = time1;
}
else
{
boolean sign_time1 = false;
boolean sign_time2 = false;
String[] time1_out, time2_out;

boolean negative = false;
String output_split[];

if (time1.contains("-"))
{
time1_out = time1.split(":");
time1_out[0] = time1_out[0].replace("-", "");
time1_out[1] = time1_out[1].replace("-", "");
time1 = time1_out[0] + ':' + time1_out[1];
sign_time1 = true;
}

if (time2.contains("-"))
{
time2_out = time2.split(":");
time2_out[0] = time2_out[0].replace("-", "");
time2_out[1] = time2_out[1].replace("-", "");
time2 = time2_out[0] + ':' + time2_out[1];
sign_time2 = true;
}

PeriodFormatterBuilder builder = new PeriodFormatterBuilder();
builder.minimumPrintedDigits(2);
build

You're really overcomplicating it. The great thing about Joda Time is that it can do all the hard work for you. This simplified method will give the exact same output as your original Operation_Sum:

static String sumTimes(String time1, String time2) {
    PeriodFormatter formatter = new PeriodFormatterBuilder()
            .minimumPrintedDigits(2)
            .printZeroAlways()
            .appendHours()
            .appendLiteral(":")
            .appendMinutes()
            .toFormatter();

    Period period1 = formatter.parsePeriod(time1);
    Period period2 = formatter.parsePeriod(time2);
    return formatter.print(period1.plus(period2));
}

Test cases I used on both your method and this new one:

@Test
public void testSum8Plus8() {
    String time1 = "08:00";
    String time2 = "08:00";
    assertEquals("16:00", JodaDemo.sumTimes(time1, time2));
}

@Test
public void testSum8Plus18() {
    String time1 = "08:00";
    String time2 = "18:00";
    assertEquals("26:00", JodaDemo.sumTimes(time1, time2));
}

@Test
public void testSum8Minus18() {
    String time1 = "08:00";
    String time2 = "-18:00";
    assertEquals("-10:00", JodaDemo.sumTimes(time1, time2));
}

In addition to the simplification, some other improvements:

• Variables should be named camelCase (don't start with capital letter, don't use underscore)

• Typical *Builder classes support a fluent interface, so instead of builder.someMethod(); builder.anotherMethod();, you can just chain the calls as builder.someMethod().anotherMethod();

• Unit tests are good to let you make changes and confirm that everything is still working. It's also easier for code reviewers, so they can easily test refactored version of the code. Try to include more tests to cover all corner cases.

There were glaring logical mistakes in your original implementation. For example in this code:

if (time1.contains("-"))
{
    time1_out = time1.split(":");
    time1_out[0] = time1_out[0].replace("-", "");
    time1_out[1] = time1_out[1].replace("-", "");
    time1 = time1_out[0] + ':' + time1_out[1];
    sign_time1 = true;
}

This is (pretty much) equivalent to the simpler and better:

if (time1.contains("-")) {
    time1 = time1.replaceAll("-", "");
    sign_time1 = true;
}

That is, here, and in many other places in your code, you were doing a lot of operations that can be easily simplified to fewer operations.

StackExchange Code Review Q#62022, answer score: 11