patternjavaMinor

Simple Calculator RESTfull API on Java

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

simplerestfulljavaapicalculator

Problem

I have written a small calculator REST API and I need someone to look the structure and design of what I did. I want to know how I can make it better. Here is an example with multiplication. Other operations are similar.

Task was to write RESTfull API that will take request like this /multiply/a/b/c with double type parameters a, b ,c and return abc in JSON.

Structure:

me.leqada.rest
├── Application.java
├── logic
│   ├── Cache.java
│   └── Logic.java
├── model
│   ├── ActInfo.java
│   ├── Error.java
│   └── Output.java
└── service
    └── Calculator.java

Application class (Just to not use web.xml):

import me.leqada.rest.service.Calculator;
import org.apache.log4j.Logger;
import javax.ws.rs.ApplicationPath;
import java.util.HashSet;
import java.util.Set;

@ApplicationPath("/")
public class Application extends javax.ws.rs.core.Application {
    private final static Logger logger = Logger.getLogger(Application.class);

    public Set> getClasses() {
        final Set> resources = new HashSet<>();

        try {
            resources.add(Calculator.class);
        }
        catch(Exception e) {
            logger.error(e.getStackTrace());
        }
        return resources;
    }
}

Calculator class (request handlers):

```
import me.leqada.rest.logic.Cache;
import me.leqada.rest.logic.Logic;
import me.leqada.rest.model.ActInfo;
import me.leqada.rest.model.Output;
import org.apache.log4j.Logger;
import me.leqada.rest.model.Error;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.core.Response;

@Path("/")
@Produces("application/json")
public class Calculator {
private final static Logger logger = Logger.getLogger(Calculator.class);
private Output output;
private ActInfo resultInfo = new ActInfo();
private final Error error = new Error();
private final Logic logic = new Logic();

@GET
@Path("multiply/{a}/{b}/{c}")
public Response m

Solution

In my opinion your classes names are justified by what they are doing, but I want to review the code for some classes: My review is based on the above code only,

Error class: This class should be immutable, If once you will get any error, then that error will have its specific code and error-text and which will not be changed, if you get some error again then it will be a new error with some diff. error code and message. You can make it immutable as the Output class.

public class Error {
    private final String errorText;
    private final int errorCode;

    public Error(int errorCode, String errorText) {
        this.errorText = errorText;
        this.errorCode = errorCode;
    }

    public String getErrorText() {
        return errorText;
    }
    public int getErrorCode() {
        return errorCode;
    }
}

ActInfo class: You should have Error class reference instead of errorCode and errorText fields, And that is the purpose of having Error class. You should provide getters and setters accordingly for error object. I don't think you need constructor also, because the default values are already NULL for Error and 0 for result:

public class ActInfo implements Serializable {
    private Error error;
    private double result;

    public Error getError() {
        return error;
    }

    public void setError(Error error) {
        this.error = error;
    }

    public double getResult() {
        return result;
    }

    public void setResult(double result) {
        this.result = result;
    }
}

Logic class: Due to above changes in Error class and ActInfo class, some code you have to refactor as below:

public class Logic{
...................
................
    case "multiplication": {
            resultInfo.setResult(mult(params));
            if (Double.isInfinite(resultInfo.getResult())) {
                logger.warn("Result number is too big. Returning 402 error");
                resultInfo.setError(new Error(402, "Too Big number"));
                resultInfo.setResult(0);
            }
            break;
        }
................
...............
    // Try to make the code generic whenever possible
    private double mult(double[] arr) {
        double result = 1;
        for(double e : arr) {
            result *= e;
        }
        return result;
    }

}

Cache class: In this class get() method is really very lengthy, its little bit hard to understand, Your method should be small enough to understand, It should do only one thing, We can make core more clean if we create the methods small by making them responsible for doing one thing and methods should be justified by their names and what they are actual doing, Here we can refactor it and extract the functionality of calculating the multiplication result, if not exists in cache, in different method, so that we can use that method somewhere else also if needed, because multiplication calculation logic is separate now. May be we can refactor like below:

```
public static ActInfo get (String[] paramsText, Logic logic) {
String action = paramsText[0];
int key = Arrays.deepHashCode(paramsText);

logger.info("Trying to get information from cache ...");
logger.info(action + "|" + paramsText[1] + "|" + paramsText[2] + "|" + paramsText[3]);
logger.info("Generated key is: " + key);

try {
JCS cache = JCS.getInstance("default");
ActInfo cachedData = (ActInfo)cache.get(key);

if (cachedData != null) {
logger.info("Value found in cache. Returning: " + cachedData.getResult());
// The cachedData is valid and can be used
return cachedData;
}
else{
ActInfo resultInfo = calculateMultiplicationResult(paramsText, logic);
logger.info("Final result calculated. Adding to cache and returning: " +
resultInfo.getResult());
cache.put(key, resultInfo);
return resultInfo;

}
} catch (CacheException e) {
logger.error(e.getMessage());
ActInfo resultInfo = new ActInfo();
resultInfo.setError(new Error(500, "Server error"));
return resultInfo;
}
}

private static ActInfo calculateMultiplicationResult(String[] paramsText, Logic logic) {
double[] params;
ActInfo resultInfo = new ActInfo();
logger.info("Value not found in cache. Trying to parse doubles from input ...");
params = logic.parseToNum(paramsText);

if (params == null) {
logger.warn("Parameter problem. Returning 400 code.");
resultInfo.setError(new Error(400, "Invalid parameter"));

} else {
for (double param : params) {
// If passed parameter is not fit to double
if (Double.isInfinite(param)){
logger.info("Result number is

Code Snippets

public class Error {
    private final String errorText;
    private final int errorCode;

    public Error(int errorCode, String errorText) {
        this.errorText = errorText;
        this.errorCode = errorCode;
    }

    public String getErrorText() {
        return errorText;
    }
    public int getErrorCode() {
        return errorCode;
    }
}

public class ActInfo implements Serializable {
    private Error error;
    private double result;

    public Error getError() {
        return error;
    }

    public void setError(Error error) {
        this.error = error;
    }

    public double getResult() {
        return result;
    }

    public void setResult(double result) {
        this.result = result;
    }
}

public class Logic{
...................
................
    case "multiplication": {
            resultInfo.setResult(mult(params));
            if (Double.isInfinite(resultInfo.getResult())) {
                logger.warn("Result number is too big. Returning 402 error");
                resultInfo.setError(new Error(402, "Too Big number"));
                resultInfo.setResult(0);
            }
            break;
        }
................
...............
    // Try to make the code generic whenever possible
    private double mult(double[] arr) {
        double result = 1;
        for(double e : arr) {
            result *= e;
        }
        return result;
    }

}

public static ActInfo get (String[] paramsText, Logic logic) {
        String action = paramsText[0];
        int key =  Arrays.deepHashCode(paramsText);

        logger.info("Trying to get information from cache ...");
        logger.info(action + "|" + paramsText[1] + "|" + paramsText[2] + "|" + paramsText[3]);
        logger.info("Generated key is: " + key);

        try {
            JCS cache = JCS.getInstance("default");
            ActInfo cachedData = (ActInfo)cache.get(key);

            if (cachedData != null) {
                logger.info("Value found in cache. Returning: " + cachedData.getResult());
                // The cachedData is valid and can be used
                return cachedData;
            }
            else{
                ActInfo resultInfo = calculateMultiplicationResult(paramsText, logic);
                logger.info("Final result calculated. Adding to cache and returning: " +
                        resultInfo.getResult());
                cache.put(key, resultInfo);
                return resultInfo;

            }
        } catch (CacheException e) {
            logger.error(e.getMessage());
            ActInfo resultInfo = new ActInfo();
            resultInfo.setError(new Error(500, "Server error"));
            return resultInfo;
        }
    }

    private static ActInfo calculateMultiplicationResult(String[] paramsText, Logic logic) {
        double[] params;
        ActInfo resultInfo = new ActInfo();
        logger.info("Value not found in cache. Trying to parse doubles from input ...");
        params = logic.parseToNum(paramsText);

        if (params == null) {
            logger.warn("Parameter problem. Returning 400 code.");
            resultInfo.setError(new Error(400, "Invalid parameter"));

        } else {
            for (double param : params) {
                // If passed parameter is not fit to double
                if (Double.isInfinite(param)){
                    logger.info("Result number is too big. Returning 402 error");
                    resultInfo.setError(new Error(402, "Too big number"));
                    resultInfo.setResult(0);
                    return resultInfo;
                }
            }
            logger.info("Calling act method to calculate final result ...");
            resultInfo = logic.act(action, params);
        }
        return resultInfo;
    }

Context

StackExchange Code Review Q#131650, answer score: 2

Revisions (0)

No revisions yet.