JS面向对象编程疑问：构造函数写法规范与代码重构合理性咨询

可以，且这是更符合职责分离原则的写法。构造函数仅负责初始化基础属性，将rgb计算逻辑单独封装为generateRgb()实例方法，逻辑更加内聚，后续需要调整rgb生成规则时无需修改构造函数代码，仅调整对应方法即可。你当前写法中只声明this.rgb不赋值是完全合法的，JS会默认给未赋值的实例属性赋值为undefined，后续调用方法再赋值不会有任何问题。

确实是不合理的写法，属于典型的职责越界。newButton的核心功能是生成按钮元素，不应该承担Player实例属性的计算逻辑，会导致后续维护时很难定位rgb值的生成入口，如果其他业务逻辑也需要用到rgb值，还会产生冗余的重复代码。另外你当前newButton中的r、g、b变量没有加声明关键字，会默认挂载到window对象上造成全局污染，属于需要修复的明显bug。

不需要这么写。更规范的做法是将方法挂载到Player.prototype上，而非每个实例都单独存储一份函数引用，既节省内存空间，也符合JS原型继承的设计逻辑。如果使用ES6 class语法实现会更加简洁易读：

class Player {
  constructor(playerNumber, timeout, sec) {
    this.playerNumber = playerNumber;
    this.rgb = undefined;
    this.timeout = timeout;
    this.sec = sec;
    this.div = document.createElement("div");
  }

  newButton() {
    const r = Math.floor(Math.random() * 100 + 100);
    const g = Math.floor(Math.random() * 100 + 100);
    const b = Math.floor(Math.random() * 100 + 100);
    this.rgb = `rgb(${r},${g},${b})`;
    return `<button onclick="playerTimer(${this.playerNumber})" class="btn" style="background-color:${this.rgb}"><h3>Player ${this.playerNumber+1} </h3> <br><br> Time left:<h2><div id="playerTimerDiv${this.playerNumber}"> 30 </div></h2></button>`;
  }
}

个人小型项目中用字符串拼接DOM的写法问题不大，但是在规模化、长期维护或者团队协作的项目中，这种写法有几个明显缺陷：

• 存在XSS注入风险，如果拼接的字符串中包含未做转义的用户输入内容，直接渲染会产生安全漏洞
• 无法便捷绑定非全局的JS事件监听，你当前用的onclick绑定的是全局函数，上下文容易丢失，也不方便后续销毁事件、回收内存
• 可读性差，复杂DOM结构的字符串嵌套层级多了之后非常难维护，很容易写错标签
• 无法做静态语法检查，HTML字符串中的语法错误IDE很难识别提示
  如果只是个人练手小项目追求开发效率可以继续这么写，如果是生产环境项目更建议用DOM API或者前端框架的模板能力实现。

内容的提问来源于stack exchange，提问作者djspatule