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

写的很好，没问题。初学者 这样写很好的，能运行，让别人容易看懂就是好代码

我觉得挺好，能用能看明白，别的都是锦上添花

(⊙ˍ⊙)，Java 被你写成了 JavaScript 的感觉，Java 强类型的优势全无了。

接收请求参数应该创建一个类型实体来，而不是直接在 JSON 对象上获取，响应也是。

userType 判断那块，可以用 Map 来处理。「用户是否存在于白名单中」，你那里 return 了 下面就可以不要 else 了，下面直接写在那个 if 外就可以了。

返回不要用 map 封装 很难维护 最好写一个返回的 vo 然后 userType 可以用枚举封装一下

@Rache1 所以前端看懂了 哈哈

//用户类型:0 无效，1 网格，2 客户经理，3 泛渠道，4 其他 这里可以建个 enum ，后面需要扩充的时候再修改

所以楼主就是那个小白吗

json 解析都进业务层了，数据校验还需要自己 try catch ，楼主不是钓鱼拉血压的吧……
还有这个奇怪的 override ，是用了啥奇怪的框架还是写了一个毫无意义的 service 接口……

1. valetUserInfo.setCreateTime(DateUtils.getNowDate()); valetUserInfo.setUpdateTime(DateUtils.getNowDate());
创建时间和更新时间应该自动加, 手动加容易忘记.
2. 不要注释代码, 哪天想找回来 git 里有记录的.

代码 "臃肿" 并不是问题. 个人看法是 耦合度太高.无法应对需求变动,建议参考其它用户,权限,角色管理系统. 我个人认为存在的问题有这么几个
1. login 方法中的参数,未做比较严格的校验,如果客户端传入了 10 万字符长度的,你这里会不会卡?
2. 是否可以将参数校验单独提取为一个 方法? validator
3. 用户黑白名单与角色的功能是写死的,如果需求变动,是需要改代码的. 如果允许可以使用 apache shiro 这种成熟的权限框架,如果觉得重,也务必将 授权,鉴权 独立为两个模块,方便扩展.

valatUserInfo,userType 可以抽成一个函数
尽量不要用 map
最后的组装可以用 builder 或者 Beanutil 这种东西

当然以上都是吹毛求疵了，我个人会做成这样

private final MemberService memberService;
public ResponseEntity<?> login(@NotNull final String mobile, @NotNull final String passwd) {
String decrptMobile = AesUtil.decrypt(mobile);
Member member = memberService.findByMobile(mobile);
if(BCrypt.check(passwd, member.passwd)) throw new LoginException('登录失败');
String token = TokenUtil.generate(decMobile);
LoginResp resp = LoginResp.builder()
.name(member.name)
.age(member.age)
.token(token)
.build();
ResponseEntity.status(HttpStatus.OK)
.header("token", token)
.body(resp);
}

1. 不要用 Map 做方法参数 /返回值，建单独的 param/vo 类，否则以后维护接口的时候看到这么长一串参数会一脸懵逼
2. 代码里不要写 0 ，1 ，2 ，3 ，4 。。。这种魔法数字，建立常量 /枚举来维护这些值

初学这样写挺好的，后面就是抽离封装方法了。可以重复利用的功能抽成一个方法，比如白名单，用户存在，用户的类型，参数的 put ，用户登录的更新，这些抽成方法，然后去调用。
再进一步就要用到架构类的知识了，现在这样就行

{'0':'无效'}[userType]

过来人的经验：能跑就行！！！

看到了 论坛的大爱。。。。谢谢大家 虽然我看不懂

“又不是不能用”🌸🐔

@Rache1 请问 userType 怎么用 map 处理呢？有点不太理解。

按我们组长的话“能用，不慢就行”

写的很棒，毫无 Java 感（

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

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

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

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

顺便再了解下 JSR303 ！

你这登录就妥了~

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

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

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

上来就报个 NPE 你怕不怕？

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


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 代替 注释

这个有点像我写 Node

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

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

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

槽点太多，哈哈

@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);
}

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

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

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

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