刚入职的小白写了一个登录的接口，但感觉写的很臃肿，请教一下大佬有没有可以优化的地方呢？

[zx9481]

@Override
    public Response login(String params) {
        try {
            JSONObject jsonObject = JSON.parseObject(params);
            // 解密手机号码
            AesNewUtil aes = new AesNewUtil();
            String decMobile = aes.decrypt(jsonObject.getString("mobile"));
            // 获取 openId
            String openId = jsonObject.getString("openId");
            if (StringUtils.isEmpty(openId)) {
                return Response.fail(RespCode.PARAM_NULL);
            }
            logger.info("openId : {}", openId);
            // 查询代客用户表
            ValetUser valetUser = selectUserByMobile(decMobile);
            // 用户是否存在于白名单中
            if (null == valetUser) {
                return Response.fail(RespCode.NO_GET_INFO);
            } else {
                String userId = valetUser.getUserId();
                // 判断用户信息是否存在
                ValetUserInfo valetUserInfo = valetUserInfoMapper.selectByPrimaryKey(userId);
                if (null == valetUserInfo) {
                    // 保存用户信息
                    valetUserInfo = new ValetUserInfo();
                    valetUserInfo.setUserId(userId);
                    valetUserInfo.setOpenId(openId);
                    valetUserInfo.setCreateTime(DateUtils.getNowDate());
                    valetUserInfo.setUpdateTime(DateUtils.getNowDate());
                    valetUserInfoMapper.insertSelective(valetUserInfo);
                }
                // 获取 token
                String token = TokenUtil.generateToken(userId, decMobile, "app");
                cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().set(Constants.USER_AUTH_PRIFIX + userId, token, 7, TimeUnit.DAYS);
//                cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().set(Constants.LOGIN_MOBILE_AUTH_PRIFIX + userId, decMobile, 7, TimeUnit.DAYS);
//                logger.info((String) cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().get(Constants.USER_AUTH_PRIFIX + valetUser.getUserId()));
                Map<String, Object> result = new HashMap<>();
                // 用户类型:0 无效，1 网格，2 客户经理，3 泛渠道，4 其他
                String userType = String.valueOf(valetUser.getUserType());
                if ("0".equals(userType)) {
                    userType = "无效";
                } else if ("1".equals(userType)) {
                    userType = "网格";
                } else if ("2".equals(userType)) {
                    userType = "客户经理";
                } else if ("3".equals(userType)) {
                    userType = "泛渠道";
                } else {
                    userType = "其他";
                }
                result.put("mobile", valetUser.getMobile());
                result.put("userName", valetUser.getUserName());
                result.put("cusName", valetUser.getCusName());
                result.put("userType", userType);
                result.put("operId", valetUser.getOperId());
                result.put("orgId", valetUser.getOrgId());
                result.put("merchantNum", valetUser.getMerchantNum());
                result.put("token", token);
                // 更新用户表登录时间
                valetUserMapper.updateLoginTimeByUserId(DateUtils.getNowDate(), userId);
                logger.info("登录成功 {}", decMobile);
                return Response.success("登录成功", result);
            }
        } catch (ValetappException e) {
            logger.error("登录失败 ： " + e.getMessage());
            return Response.fail(RespCode.FAIL);
        }
    }

[SingeeKing]

写的很棒，毫无 Java 感（

[jalena]

util 类为啥不是 static ！！！还要 new ？

再者入参？ java 难道不支持 entity ？

再说了，放开胆子干啊！！！ JSONObject 啊

[jalena]

要写 java ，了解下 Lombok 。。。。。。声明个类，加个 @data ，分分钟的事情。。要想骚，再加个 @build

[jalena]

顺便再了解下 JSR303 ！

你这登录就妥了~

[jalena]

顺便再夸奖下，比我当年写的好！！

[daimubai]

小白也提出一点建议哈，
1. 首先 Java 是一个面向对象的语言，全程 json 、map 这这这，参数和返回结果自然要用一个对象来处理
2. 既然 return 了，那这里的 else 没有必要，我个人不喜欢把一堆代码包在 else 中
3. 取 userType 这里我会用枚举 switch enum ，并且单独抽出一个方法 getUserType()

[daimubai]

而且使用对象的话，你可以用到很多的插件，比如一键生成属性的 setter 方法啊等等

[Tenlearn]

上来就报个 NPE 你怕不怕？

[season8]

优化是吧？
你这个方法真的很长，层次也很深，做的事也很多，显得很略复杂。


1. 入参用 string 、map 都是增加后期维护成本，复杂方法都不知道具体属性
2. 这个 try catch 粗暴，可以的话在具体业务方法上加
3. 你这个 StringUtils.isEmpty(openId) ， 要是传的 空串咋办
4. 手机号也没做非空校验，当然可能解密会抛异常，但这样不够直观
5. if(xxx){return} else{xxx}这种写法 去掉 else 能减少代码层次，稍微提高可读性
6. 保存用户信息可以提出来
7. userType 转换 也可以提出来，用 枚举、map 或者 直接提出来一个简单的方法都可以
8. 最后返回的 result 几乎就是把 valetUser 属性拿了一遍，能不能用 JSON.toJSON(valetUser)，或者 Response 设计成泛型，直接放入 valetUser
9. 个人觉得好多注释都没必要，大多是一眼就能看出来是干啥的，你可以在关键业务或者复杂上加，或者用 log 代替 注释

[h1104350235]

这个有点像我写 Node

[season8]

@season8 关于第 3 点，我是当你用的 commons-lang3.StringUtils ，是的话用 isNotBlank 代替

[Rache1]

@zx9481 😂 建议使用枚举了，写这个评论的时候忘了枚举这茬儿了。

[9c04C5dO01Sw5DNL]

人生就这样，凑合过吧（狗头

[night98]

槽点太多，哈哈

@Override
public Response login(String params) { // 入参改成 VO
try {
JSONObject jsonObject = JSON.parseObject(params); // 这块干掉
// 解密手机号码
AesNewUtil aes = new AesNewUtil();
String decMobile = aes.decrypt(jsonObject.getString("mobile")); // 改成静态方法
// 获取 openId
String openId = jsonObject.getString("openId");
if (StringUtils.isEmpty(openId)) { // 入参时校验
return Response.fail(RespCode.PARAM_NULL);
}
logger.info("openId : {}", openId);
// 查询代客用户表
ValetUser valetUser = selectUserByMobile(decMobile);
// 用户是否存在于白名单中
if (null == valetUser) { // 入参时校验，了解一下自定义注解验证
return Response.fail(RespCode.NO_GET_INFO);
} else {
String userId = valetUser.getUserId();
// 判断用户信息是否存在
ValetUserInfo valetUserInfo = valetUserInfoMapper.selectByPrimaryKey(userId);
if (null == valetUserInfo) {
// 保存用户信息
valetUserInfo = new ValetUserInfo();
valetUserInfo.setUserId(userId);
valetUserInfo.setOpenId(openId);
valetUserInfo.setCreateTime(DateUtils.getNowDate());
valetUserInfo.setUpdateTime(DateUtils.getNowDate());
valetUserInfoMapper.insertSelective(valetUserInfo);
}
// 获取 token
String token = TokenUtil.generateToken(userId, decMobile, "app");
cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().set(Constants.USER_AUTH_PRIFIX + userId, token, 7, TimeUnit.DAYS);
// cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().set(Constants.LOGIN_MOBILE_AUTH_PRIFIX + userId, decMobile, 7, TimeUnit.DAYS);
// logger.info((String) cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().get(Constants.USER_AUTH_PRIFIX + valetUser.getUserId()));
Map<String, Object> result = new HashMap<>(); //返回改成 dto
// 用户类型:0 无效，1 网格，2 客户经理，3 泛渠道，4 其他
String userType = String.valueOf(valetUser.getUserType()); // 这里改成枚举+find 方法
if ("0".equals(userType)) {
userType = "无效";
} else if ("1".equals(userType)) {
userType = "网格";
} else if ("2".equals(userType)) {
userType = "客户经理";
} else if ("3".equals(userType)) {
userType = "泛渠道";
} else {
userType = "其他";
}
result.put("mobile", valetUser.getMobile()); // 这里用 beanutil 或者 mapstruct 优化，一行完事
result.put("userName", valetUser.getUserName());
result.put("cusName", valetUser.getCusName());
result.put("userType", userType);
result.put("operId", valetUser.getOperId());
result.put("orgId", valetUser.getOrgId());
result.put("merchantNum", valetUser.getMerchantNum());
result.put("token", token);
// 更新用户表登录时间
valetUserMapper.updateLoginTimeByUserId(DateUtils.getNowDate(), userId);
logger.info("登录成功 {}", decMobile);
return Response.success("登录成功", result);
}
} catch (ValetappException e) { //改成全局异常处理
logger.error("登录失败 ： " + e.getMessage());
return Response.fail(RespCode.FAIL);
}
}



改完差不多这样
@Override
public Response login(@Valid UserVO user) {
logger.info("openId : {}", openId);
String decMobile = AesNewUtil.decrypt(user.getMobile());
String userId = valetUser.getUserId();
ValetUser valetUser = selectUserByMobile(decMobile);
// 判断用户信息是否存在
ValetUserInfo valetUserInfo = valetUserInfoMapper.selectByPrimaryKey(userId);
if (Objects.isNull(valetUserInfo)) {
// 保存用户信息
valetUserInfo = ValetUserInfoMapper.convert(user);
valetUserInfoMapper.insertSelective(valetUserInfo);
}
String token = TokenUtil.generateToken(userId, decMobile, "app");
cacheOpt.getOpt(Constants.CACHE_STR).opsForValue().set(Constants.USER_AUTH_PRIFIX + userId, token, 7, TimeUnit.DAYS);

String userType = UserType.find(valetUser.getUserType());
TokenInfo token = tokenmapper.convert(valetUser,userType);
valetUserMapper.updateLoginTimeByUserId(DateUtils.getNowDate(), userId);
logger.info("登录成功 {}", decMobile);
return Response.success("登录成功", token);
}

[Joker123456789]

用户类型可以用枚举，不需要 if

最后的那个 map 是 多余的，直接返回 valetUser 即可

还有一个小建议，手机号不需要加密，实际应用中 配上 https 然后不要用 get 传输 就可以解决问题

[rehoni]

建议下个 alibaba coding guideline 插件，他会提示你一些修改的，比如魔法值，比如方法过长等等，已经很可以了，后边学的越多就写的越好了。