Link to home
Start Free TrialLog in
Avatar of srikotesh
srikotesh

asked on

how to write the below code in an effective way in java

Hi Experts,

Can some one suggest me how can i write below code in an effective way.

whenever i call this method i will get any of the status it may be 1 or 2 or 3....8
public Integer getContentStatus(int ContentId) {
        int status = 0;
        String success = "";
        Object ContentStatus = ContentDao.getContentByStatus(ContentId);
        if (ContentStatus != null) {
            Object[] row = (Object[]) ContentStatus;
            String taskStatus = (String) row[1];
            if (row[2] == null) {
                success = null;
            }
            else {
                success = String.valueOf((Integer) row[2]);
            }
            String actionStatus = (String) row[3];
            Date scheduleTime = (Date) row[4];
            String actionSubStatus = actionStatus.substring(9);
            String taskResult = taskStatus.substring(5);
            if ("COMP".equals(taskResult)) {
                return status = 1;
            }
            else if ("NOTCOMP".equals(taskResult) && MWatchUtil.isNullOrEmpty(String.valueOf(success))) {
                return status = 8;
            }
            else if ("NOTCOMP".equals(taskResult) && success == "0") {
                return status = 2;
            }
            else if ("IDLE".equals(taskResult)) {
                if ("ABO".equals(actionSubStatus)) {
                    return status = 3;
                }
                else if ("RUN".equals(actionSubStatus)) {
                    return status = 4;
                }
                else if ("INIT".equals(actionSubStatus)) {
                    return status = 5;
                }
                else if ("SCH".equals(actionSubStatus)) {
                    return status = 6;
                }
                else if ("IDLE".equals(actionSubStatus)) {
                    if (IsScheduleTimeLessThanCurrentTime(scheduleTime)) {
                        return status = 8;
                    }
                }
                else {
                    return status = 4;
                }
            }
            else {
                return status = 7;
            }
        }
        return null;
    }


From this method i will display  counts status to the user.
success count ---8
failure count ---3 like that i am going return map with count

From this method i am going to call the above method to get the status id

public Map<String, Integer> getContentStatusCount(List<Integer> customerids) {
        int success, failureByLogic, aborted, running, initiated, scheduled, yetToStart, failureBySystem;
        success = failureByLogic = aborted = running = initiated = scheduled = yetToStart = failureBySystem = 0;
        List<Object> ContentStatusInfo = ContentDao.getContentByStatusInfo(customerids);
        if (!ContentStatusInfo.isEmpty()) {
            for (Object object : ContentStatusInfo) {
                Integer ContentIds = (Integer) object;
                if (ContentServiceHelper.getContentStatus(ContentIds) != null) {
                    int statusvalue = ContentServiceHelper.getContentStatus(ContentIds);
                    if (statusvalue == 1) {
                        success++;
                    }
                    else if (statusvalue == 2) {
                        failureByLogic++;
                    }
                    else if (statusvalue == 3) {
                        aborted++;
                    }
                    else if (statusvalue == 4) {
                        running++;
                    }
                    else if (statusvalue == 5) {
                        initiated++;
                    }
                    else if (statusvalue == 6) {
                        scheduled++;
                    }
                    else if (statusvalue == 7) {
                        yetToStart++;
                    }
                    else if (statusvalue == 8) {
                        failureBySystem++;
                    }
                }
            }
        }
        Map<String, Integer> ContentStatusMap = new HashMap<>();
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(1), success);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(2), failureByLogic);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(3), aborted);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(4), running);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(5), initiated);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(6), scheduled);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(7), yetToStart);
        ContentStatusMap.put(ContentApprovalStatusEnum.getById(8), failureBySystem);
        LOGGER.debug("ContentStatusMap is {}", ContentStatusMap);
        return ContentStatusMap;
    }

Open in new window


Thanks,
Avatar of dpearson
dpearson

I'd suggest adding an enum.  I'm not going to rewrite the whole thing for you, but here's the bones of the structure I'd recommend - adding an enum like this:

public enum Status {
	kABO(3, "IDLE", "ABO"),
	kRun(4, "IDLE", "RUN"),
	kNotComp(8,"NOTCOMP", null)
	;

	private final int statusValue;
	private final String statusString ;
	private final String substatusString ;

	Status(int statusValue, String statusString, String substatusString) {
		this.statusValue = statusValue;
		this.statusString = statusString;
		this.substatusString = substatusString;
	}

	public int getStatusValue() {
		return statusValue;
	}

	public static Status fromString(String statusString, String substatusString) {
		for (Status status : values()) {
			if (status.statusString.equals(statusString) &&
			    (status.substatusString == null || status.substatusString.equals(substatusString))) {
				return status;
			}
		}

		return null ; // Or some default
	}
}

Open in new window


Then you can call Status.fromStatus(taskResult, actionSubStatus) to handle all of the initial mapping from status string and sub status string to a status code.

At that point you have a Status object (not an int) with the actual value of your status which you can then pass around and when you need to map that back to an int you can call getStatusValue() on it.

Hope that gets you started down the right track,

Doug
You can replace the if else condition with switch case.
Avatar of srikotesh

ASKER

Hi dpearson,
that enum class will not applicable for all statuses right.because checking with different conditions.
what about getContentStatusCount() method is there any i can write it in a better way.
ASKER CERTIFIED SOLUTION
Avatar of dpearson
dpearson

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Excellent