V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
zx9481
V2EX  ›  Java

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

  •  
  •   zx9481 · 111 天前 · 2508 次点击
    这是一个创建于 111 天前的主题,其中的信息可能已经有所发展或是发生改变。
    @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);
            }
        }
    
    第 1 条附言  ·  111 天前
    没想到随便一发有这么多大佬ヾ(≧▽≦*)o 谢谢大家
    36 条回复    2022-04-28 08:34:05 +08:00
    lcy630409
        1
    lcy630409  
       111 天前   ❤️ 1
    写的很好,没问题。初学者 这样写很好的,能运行,让别人容易看懂就是好代码
    SmaliYu
        2
    SmaliYu  
       111 天前
    我觉得挺好,能用能看明白,别的都是锦上添花
    Rache1
        3
    Rache1  
       111 天前   ❤️ 1
    (⊙ˍ⊙),Java 被你写成了 JavaScript 的感觉,Java 强类型的优势全无了。

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

    userType 判断那块,可以用 Map 来处理。「用户是否存在于白名单中」,你那里 return 了 下面就可以不要 else 了,下面直接写在那个 if 外就可以了。
    kosmosr
        4
    kosmosr  
       111 天前
    返回不要用 map 封装 很难维护 最好写一个返回的 vo 然后 userType 可以用枚举封装一下
    356693212
        5
    356693212  
       111 天前
    @Rache1 所以前端看懂了 哈哈
    sinnosong1
        6
    sinnosong1  
       111 天前
    //用户类型:0 无效,1 网格,2 客户经理,3 泛渠道,4 其他 这里可以建个 enum ,后面需要扩充的时候再修改
    Latin
        7
    Latin  
       111 天前
    所以楼主就是那个小白吗
    chendy
        8
    chendy  
       111 天前
    json 解析都进业务层了,数据校验还需要自己 try catch ,楼主不是钓鱼拉血压的吧……
    还有这个奇怪的 override ,是用了啥奇怪的框架还是写了一个毫无意义的 service 接口……
    MoYi123
        9
    MoYi123  
       111 天前
    1. valetUserInfo.setCreateTime(DateUtils.getNowDate()); valetUserInfo.setUpdateTime(DateUtils.getNowDate());
    创建时间和更新时间应该自动加, 手动加容易忘记.
    2. 不要注释代码, 哪天想找回来 git 里有记录的.
    ming159
        10
    ming159  
       111 天前
    代码 "臃肿" 并不是问题. 个人看法是 耦合度太高.无法应对需求变动,建议参考其它用户,权限,角色管理系统. 我个人认为存在的问题有这么几个
    1. login 方法中的参数,未做比较严格的校验,如果客户端传入了 10 万字符长度的,你这里会不会卡?
    2. 是否可以将参数校验单独提取为一个 方法? validator
    3. 用户黑白名单与角色的功能是写死的,如果需求变动,是需要改代码的. 如果允许可以使用 apache shiro 这种成熟的权限框架,如果觉得重,也务必将 授权,鉴权 独立为两个模块,方便扩展.
    hidemyself
        11
    hidemyself  
       111 天前
    valatUserInfo,userType 可以抽成一个函数
    尽量不要用 map
    最后的组装可以用 builder 或者 Beanutil 这种东西

    当然以上都是吹毛求疵了,我个人会做成这样
    ye4tar
        12
    ye4tar  
       111 天前
    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);
    }
    storyxc
        13
    storyxc  
       111 天前
    1. 不要用 Map 做方法参数 /返回值,建单独的 param/vo 类,否则以后维护接口的时候看到这么长一串参数会一脸懵逼
    2. 代码里不要写 0 ,1 ,2 ,3 ,4 。。。这种魔法数字,建立常量 /枚举来维护这些值
    janus77
        14
    janus77  
       111 天前
    初学这样写挺好的,后面就是抽离封装方法了。可以重复利用的功能抽成一个方法,比如白名单,用户存在,用户的类型,参数的 put ,用户登录的更新,这些抽成方法,然后去调用。
    再进一步就要用到架构类的知识了,现在这样就行
    bojue
        15
    bojue  
       111 天前
    {'0':'无效'}[userType]
    pengtdyd
        16
    pengtdyd  
       111 天前
    过来人的经验:能跑就行!!!
    KK7890
        17
    KK7890  
       111 天前
    看到了 论坛的大爱。。。。谢谢大家 虽然我看不懂
    rootx
        18
    rootx  
       111 天前
    “又不是不能用”🌸🐔
    zx9481
        19
    zx9481  
    OP
       111 天前
    @Rache1 请问 userType 怎么用 map 处理呢?有点不太理解。
    siweipancc
        20
    siweipancc  
       111 天前 via iPhone
    按我们组长的话“能用,不慢就行”
    SingeeKing
        21
    SingeeKing  
       111 天前   ❤️ 1
    写的很棒,毫无 Java 感(
    jalena
        22
    jalena  
       111 天前
    util 类为啥不是 static !!!还要 new ?

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

    再说了,放开胆子干啊!!! JSONObject 啊
    jalena
        23
    jalena  
       111 天前
    要写 java ,了解下 Lombok 。。。。。。声明个类,加个 @data ,分分钟的事情。。要想骚,再加个 @build
    jalena
        24
    jalena  
       111 天前
    顺便再了解下 JSR303 !

    你这登录就妥了~
    jalena
        25
    jalena  
       111 天前
    顺便再夸奖下,比我当年写的好!!
    daimubai
        26
    daimubai  
       111 天前
    小白也提出一点建议哈,
    1. 首先 Java 是一个面向对象的语言,全程 json 、map 这这这,参数和返回结果自然要用一个对象来处理
    2. 既然 return 了,那这里的 else 没有必要,我个人不喜欢把一堆代码包在 else 中
    3. 取 userType 这里我会用枚举 switch enum ,并且单独抽出一个方法 getUserType()
    daimubai
        27
    daimubai  
       111 天前
    而且使用对象的话,你可以用到很多的插件,比如一键生成属性的 setter 方法啊等等
    Tenlearn
        28
    Tenlearn  
       111 天前
    上来就报个 NPE 你怕不怕?
    season8
        29
    season8  
       111 天前
    优化是吧?
    你这个方法真的很长,层次也很深,做的事也很多,显得很略复杂。


    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
        30
    h1104350235  
       111 天前
    这个有点像我写 Node
    season8
        31
    season8  
       111 天前
    @season8 关于第 3 点,我是当你用的 commons-lang3.StringUtils ,是的话用 isNotBlank 代替
    Rache1
        32
    Rache1  
       111 天前
    @zx9481 😂 建议使用枚举了,写这个评论的时候忘了枚举这茬儿了。
    giiiiiithub
        33
    giiiiiithub  
       111 天前
    人生就这样,凑合过吧(狗头
    night98
        34
    night98  
       111 天前
    槽点太多,哈哈

    @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
        35
    Joker123456789  
       110 天前
    用户类型可以用枚举,不需要 if

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

    还有一个小建议,手机号不需要加密,实际应用中 配上 https 然后不要用 get 传输 就可以解决问题
    rehoni
        36
    rehoni  
       67 天前
    建议下个 alibaba coding guideline 插件,他会提示你一些修改的,比如魔法值,比如方法过长等等,已经很可以了,后边学的越多就写的越好了。
    关于   ·   帮助文档   ·   API   ·   FAQ   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   2055 人在线   最高记录 5497   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 24ms · UTC 00:23 · PVG 08:23 · LAX 17:23 · JFK 20:23
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.