减少方法中的返回语句数

Question

我有一个java代码,其中单个方法中有多个return语句.但是出于代码清理的目的,每个方法只能有一个return语句.可以做些什么来克服这一点.

这是我的代码中的方法: -

public ActionForward login(ActionMapping mapping, ActionForm form, HttpServletRequest request, HttpServletResponse response) throws Exception {

        // Kill any old sessions
        //request.getSession().invalidate();
        DynaValidatorForm dynaform = (DynaValidatorForm)form;

        // validate the form
        ActionErrors errors = form.validate(mapping, request);
        if(!errors.isEmpty()) {
            this.saveErrors(request, errors);
            return mapping.getInputForward();
        }

        // first check if token is set
        if(!isTokenValid(request, true)) {
            String errmsg="There was a problem with your login. Please close your browser then reopen it and try again. Make sure to click the Login button only ONCE.";
            request.setAttribute("errormessage", errmsg);
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
        }

        // check the form for input errors
        String errmsg = checkInput(form);
        if (errmsg != null) {
            request.setAttribute("errormessage", errmsg);
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
        }

        // no input errors detected
        String resumekey = null;
        // check for valid login
        ObjectFactory objFactory = ObjectFactory.getInstance();
        DataAccessor dataAccessor = objFactory.getDataAccessor();

        request.setCharacterEncoding("UTF-8");
        String testcode = dynaform.getString("testcode").trim();
        String studentname =  dynaform.getString("yourname").trim();


        String password = dynaform.getString("password").trim();

        // 4/3/07 - passwords going forward are ALL lower case
        if (!CaslsUtils.isEmpty(password)) {
            password = password.toLowerCase();
        }

        try{
               resumekey = new String(studentname.getBytes("ISO-8859-1"),"UTF-8");

            } catch (Exception e) {
                log_.error("Error converting item content data to UTF-8 encoding. ",e);
            }

        String hashWord = CaslsUtils.encryptString(password);
        // Make sure this is short enough to fit in the db
        if (hashWord.length() > ConstantLibrary.MAX_PASSWORD_LENGTH) {
            hashWord = hashWord.substring(0, ConstantLibrary.MAX_PASSWORD_LENGTH);
        }

        Login login = dataAccessor.getLogin(testcode, hashWord, false);

        if (login == null || !login.getUsertype().equals(Login.USERTYPE_SUBJECT)) {
            request.setAttribute("errormessage", "Incorrect test code or password.");
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
        }

        // Check if the login has expired
        if (login.getLoginexpires() != null && login.getLoginexpires().before(new Date())) {
            request.setAttribute("errormessage", "Your login has expired.");
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
        }

        // Check if the password has expired
        if (login.getPasswordexpires() != null && login.getPasswordexpires().before(new Date())) {
            request.setAttribute("errormessage", "Your login password has expired.");
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
        }

        HttpSession session = request.getSession();
        session.setAttribute(ConstantLibrary.SESSION_LOGIN, login);
        session.setAttribute(ConstantLibrary.SESSION_STUDENTNAME, studentname);
        List<Testtaker> testtakers = null;
        try {
            //invalidate the old session if the incoming user is already logged in.
            synchronized(this){
            invalidateExistingSessionOfCurrentUser(request, studentname, testcode); 
            testtakers = dataAccessor.getTesttakersByResumeKey(studentname, login);// Adding this code to call getTesttakersByResumeKey instead of getTesttakers to improve the performance of the application during student login
            }
        } catch (Exception e) {
            log.error("Exception when calling getTesttakers");
            CaslsUtils.outputLoggingData(log_, request);
            throw e;
        }
        session = request.getSession();
        if(testtakers!=null)
        {
        if(testtakers.size() == 0) {
            // new student -> start fresh
            log_.debug("starting a fresh test");

            // if this is a demo test, skip the consent pages and dump them directly to the select test page
            if (login.getTestengine().equals(Itemmaster.TESTENGINE_DEMO)) {
                return mapping.findForward("continue-panel");
            }
        }
            // send them to fill out the profile

            // check for custom profiles
            String[] surveynames = new String[4];
            List<Logingroup> logingroups = dataAccessor.getLoginGroupsByLogin(login.getLoginid());
            for(Logingroup logingroup : logingroups) {
                Groupmaster group = logingroup.getGroupmaster();
                log_.debug(String.format("group: {groupid: %d, grouptype: %s, groupname: %s}", new Object[] {group.getGroupid(), group.getGrouptype(), group.getName()}));
                Set<Groupsurvey> surveys = group.getGroupsurveys();
                if(surveys.size() > 0) {
                    // grab the first (and only) one
                    Groupsurvey survey = surveys.toArray(new Groupsurvey[0])[0];
                    if(group.getGrouptype().equalsIgnoreCase(Groupmaster.GROUPTYPE_CLASS)) {
                        surveynames[0] = survey.getSurveyname();
                    } else if (group.getGrouptype().equalsIgnoreCase(Groupmaster.GROUPTYPE_SCHOOL)){
                        surveynames[1] = survey.getSurveyname();
                    } else if (group.getGrouptype().equalsIgnoreCase(Groupmaster.GROUPTYPE_DISTRICT)){
                        surveynames[2] = survey.getSurveyname();
                    } else if (group.getGrouptype().equalsIgnoreCase(Groupmaster.GROUPTYPE_STATE)){
                        surveynames[3] = survey.getSurveyname();
                    }
                }
            }

            // match the most grandular survey
            for(int i=0; i < surveynames.length; ++i) {
                if(surveynames[i] != null) {
                    saveToken(request);
                    return mapping.findForward("student-profile-"+surveynames[i]);
                }
            }
            // no custom profile, send them to the default
            saveToken(request);
            return mapping.findForward("student-profile");
        }

        // get the set of availible panels
        Set<Panel> availiblePanels = dataAccessor.getAvailiblePanels(login, studentname);
        if(availiblePanels.size() == 0) {
            // no panels availible.  send to all done!
            log_.debug(String.format("No panels availible for Login:%s with resumekey:%s", login.toString(), studentname));
            session.setAttribute("logoutpage", true);
            resetToken(request);
            return mapping.findForward("continue-alldone");
        }
        //Eventum #427 - Prevent test takers from retaking a finished test.
        TestSubjectResult testSubjecResult=dataAccessor.getTestSubjectResult(login, resumekey);
        if(testSubjecResult != null){
            if(testSubjecResult.getRdscore() != null && testSubjecResult.getWrscore() != null && testSubjecResult.getLsscore() != null && testSubjecResult.getOlscore() != null){
                if(testSubjecResult.getRdscore().getFinishtime() != null && testSubjecResult.getWrscore().getFinishtime() != null && testSubjecResult.getLsscore().getFinishtime() != null && testSubjecResult.getOlscore().getFinishtime() != null){
                    log_.debug(String.format("Already completed all the Skill Tests.", login.toString(), studentname));
                    session.setAttribute("logoutpage", true);
                    resetToken(request);
                    return mapping.findForward("continue-alldone");
                }
            }
        }
        // get a list of resumeable testtakers
        List<Testtaker> resumeableTesttakers = new ArrayList<Testtaker>();
        for(Testtaker testtaker : testtakers) {
            if(testtaker.getPhase().equals(ConstantLibrary.PHASE_GOODBYE)) {
                // testtaker is done with test.  skip.
                continue;
            }
            if(testtaker.getCurrentpanelid() == null) {
                // testtaker is the profile testtaker
                continue;
            }
            resumeableTesttakers.add(testtaker);
        }
        // sort them from least recent to latest
        Collections.sort(resumeableTesttakers, new Comparator<Testtaker>() {
            @Override
            public int compare(Testtaker o1, Testtaker o2) {
                // TODO Auto-generated method stub
                //return 0;
                return new CompareToBuilder()
                    .append(o1.getLasttouched(), o2.getLasttouched())
                    .toComparison();
            }
        });

        if(resumeableTesttakers.size() == 0 && availiblePanels.size() > 0) {
            // nobody is resumeable but there are panels left to take
            // send them to the panel choice
            // TODO: This is probably a misuse of Struts.
            log_.info("No resumeable testtakers. Sending to panel select");
            saveToken(request);
            ActionForward myForward = (new ActionForward("/do/capstartpanel?capStartPanelAction=retest&lasttesttakerid="
                    + testtakers.get(0).getTesttakerid(), true));
            return myForward;// mapping.findForward(ConstantLibrary.FWD_CONTINUE + "-panel");
        } else {
            // grab the one most recently created and take their test
            log_.info(String.format("Resuming with choice of %d testtakers", resumeableTesttakers.size()));
            // we're forwarding to resume at this point, so we should do the some of the initialization
            // that would have happened if we were still using getTesttaker() instead of getTesttakers() above.

            session.setAttribute(ConstantLibrary.SESSION_LOGIN, login);
            session.setAttribute(ConstantLibrary.SESSION_TESTTAKER, resumeableTesttakers.get(resumeableTesttakers.size()-1));
            saveToken(request);
            return mapping.findForward(ConstantLibrary.FWD_RESUME);
        }

    }

Answer 1

将每个方法的多个返回值更改为单个return语句并不值得.实际上,这将不必要地增加将结果存储在局部变量中然后最终返回的负担,

ActionForward result = null;
//scenario 1 
result = ... 
//scenario 2
result = ...
//scenario 3
result = ...
//finally
return result;

希望这会有所帮助,但是,这对我来说没有多大意义

Answer 2

正如其他人所指出的那样,使用单个return语句并不一定能使代码更清晰.但是,在这种情况下,将方法拆分成较小的部分可能会使代码更具可读性.

例如,这部分:

    // first check if token is set
    if(!isTokenValid(request, true)) {
        String errmsg="There was a problem with your login. Please close your browser then reopen it and try again. Make sure to click the Login button only ONCE.";
        request.setAttribute("errormessage", errmsg);
        saveToken(request);
        return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
    }

    // check the form for input errors
    String errmsg = checkInput(form);
    if (errmsg != null) {
        request.setAttribute("errormessage", errmsg);
        saveToken(request);
        return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
    }

可以通过引入两种方法并使用它们来代替:

If(tokenNotSet() || formHasErrors()){
    return mapping.findForward(ConstantLibrary.FWD_CONTINUE);
}

通过在多个位置执行此操作,算法的结构变得更加清晰,可能会让您更深入地了解如何重构此代码以符合您的编码指南.

Answer 3

我会在方法的开头设置一个操作转发变量。

ActionForward actionForwardToReturn = null;

然后替换这两行

return mapping.getInputForward();

return mapping.findForward(ConstantLibrary.FWD_CONTINUE);

用这两行：

actionForwardToReturn = mapping.getInputForward()

actionForwardToReturn = mapping.findForward(ConstantLibrary.FWD_CONTINUE);

最后返回变量。

return actionForwardToReturn;

这应该不会太难:)

附带说明...（实际上是问题的原始答案）：

多个 return 语句会使代码调试变得困难。

我个人只会在方法结束时返回一个操作对象。这样做的好处是，我可以在 return 语句上放置一个断点，并准确查看该对象是什么。

我稍后想要添加的任何日志记录或其他横切问题只需在某一时刻完成。否则，我将不得不在您返回的每一行添加一条日志语句。