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,
LVL 2
srikoteshAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

dpearsonCommented:
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
Laxman PotadarCommented:
You can replace the if else condition with switch case.
srikoteshAuthor Commented:
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.
dpearsonCommented:
that enum class will not applicable for all statuses right.because checking with different conditions.

You can use this to handle the mapping of task result and substatus to a status code.  Then if you want to still do additional checking you can still just use a conditional afterwards, something like:

Status status = Status.fromString( ... ) ;

if (status == kIdle && IsScheduleTimeLessThanCurrentTime(scheduleTime))
   status = kNotIdle ;

This will be far easier to read than what you currently have.

As for getContentStatus() you can get rid of a lot of that too.  You can just use the map to maintain the counts.  That way it will update to handle all of the possible statuses.

Something like this:

	public Map<String, Integer> getContentStatusCount(List<Integer> customerids) {
		List<Object> ContentStatusInfo = ContentDao.getContentByStatusInfo(customerids);
		Map<String, Integer> ContentStatusMap = new HashMap<>();

		if (!ContentStatusInfo.isEmpty()) {
			for (Object object : ContentStatusInfo) {
				Integer ContentIds = (Integer) object;
				if (ContentServiceHelper.getContentStatus(ContentIds) != null) {
					int statusvalue = ContentServiceHelper.getContentStatus(ContentIds);

					ContentApprovalStatusEnum key = ContentApprovalStatusEnum.getById(statusValue);
					
					Integer count = ContentStatusMap.get(key) ;
					
					if (count == null)
						count = 0 ;
					
					count++ ;
					ContentStatusMap.put(key, count) ;
				}
			}
		}

		LOGGER.debug("ContentStatusMap is {}", ContentStatusMap);
		return ContentStatusMap;
}

Open in new window

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
srikoteshAuthor Commented:
Excellent
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java EE

From novice to tech pro — start learning today.