Skip to content

[Quality] 代码复杂度: MolecularRenderer God Class与多处设计问题 #28

Open
Open
[Quality] 代码复杂度: MolecularRenderer God Class与多处设计问题#28

Description

@newtontech
newtontech
opened on Mar 17, 2026

概述

对 src/renderer/MolecularRenderer.ts (commit f79f6b4) 进行代码复杂度审查后，发现多个实际的代码质量问题，包括单一职责原则违反、紧耦合、魔法数字泛滥、资源管理缺陷等。

🔴 问题 1: God Class - 单一职责原则严重违反

文件: src/renderer/MolecularRenderer.ts (lines 1-277)

当前代码

export class MolecularRenderer {
  // 管理Three.js核心对象
  private scene: THREE.Scene;
  private camera: THREE.PerspectiveCamera;
  private renderer: THREE.WebGLRenderer;
  
  // 渲染一个分子需要：
  // 1. 场景管理
  // 2. 相机控制
  // 3. 光照设置
  // 4. 几何体创建
  // 5. 材质管理
  // 6. 动画循环
  // 7. 窗口事件处理
  // 8. 资源清理

问题分析

MolecularRenderer 类承担了8个不同的职责：

职责 所在方法 代码行数
Three.js场景初始化 constructor 30+行
光照系统管理 setupLighting() 15行
分子渲染协调 renderMolecule() 20行
原子几何体创建 createAtomMesh() 18行
化学键几何体创建 createBondMesh() 32行
分子居中与缩放 centerAndScaleMolecule() 25行
窗口大小调整处理 handleResize() 10行
渲染循环管理 startRenderLoop() 12行
资源清理 dispose(), clearMolecule() 30+行

总代码行数: 277行（单个类！）

为什么这是问题

  • 难以测试: 测试任何一个功能都需要构造整个 Three.js 环境
  • 难以维护: 修改渲染逻辑可能影响资源清理
  • 难以复用: 无法单独使用原子创建逻辑或键创建逻辑
  • 认知负荷: 开发者需要理解整个类才能修改任何部分

改进建议

将类拆分为单一职责的组件：

// SceneManager.ts - 管理Three.js场景、相机、渲染器
export class SceneManager {
  constructor(container: HTMLElement, config: SceneConfig) {}
  resize(): void {}
  render(): void {}
  dispose(): void {}
}

// LightingSystem.ts - 管理光照
export class LightingSystem {
  constructor(scene: THREE.Scene) {}
  setupDefaultLighting(): void {}
}

// AtomGeometryFactory.ts - 创建原子几何体
export class AtomGeometryFactory {
  createAtomMesh(atom: Atom): THREE.Mesh {}
}

// BondGeometryFactory.ts - 创建化学键几何体
export class BondGeometryFactory {
  createBondMesh(bond: Bond, atoms: Atom[]): THREE.Mesh {}
}

// MoleculeLayoutEngine.ts - 分子布局计算
export class MoleculeLayoutEngine {
  centerAndScale(group: THREE.Group): void {}
}

// MolecularRenderer.ts - 仅协调以上组件
export class MolecularRenderer {
  private sceneManager: SceneManager;
  private atomFactory: AtomGeometryFactory;
  private bondFactory: BondGeometryFactory;
  private layoutEngine: MoleculeLayoutEngine;
  
  renderMolecule(molecule: Molecule): void {
    // 高层次的协调，而非具体的实现
  }
}

🔴 问题 2: 魔法数字泛滥 (Magic Numbers)

文件: src/renderer/MolecularRenderer.ts

当前代码中的魔法数字

// Line ~45: 硬编码相机位置
this.camera.position.set(0, 0, 20);  // ❓ 为什么是20？

// Line ~48: 硬编码背景色
this.scene.background = new THREE.Color(config.backgroundColor || 0x1a1a2e);

// Line ~65: 硬编码光照参数
const ambientLight = new THREE.AmbientLight(0xffffff, 0.6);  // ❓ 为什么是0.6？
const directionalLight = new THREE.DirectionalLight(0xffffff, 0.8);  // ❓ 0.8？
directionalLight.position.set(10, 10, 10);  // ❓ 为什么是10,10,10？
const directionalLight2 = new THREE.DirectionalLight(0xffffff, 0.4);  // ❓ 0.4？

// Line ~114: 硬编码球体分段数
const geometry = new THREE.SphereGeometry(radius, 32, 32);  // ❓ 为什么是32？

// Line ~135: 硬编码化学键半径
const geometry = new THREE.CylinderGeometry(0.15, 0.15, distance, 12);  // ❓ 0.15？12？

// Line ~140: 硬编码化学键颜色
const material = new THREE.MeshPhongMaterial({ color: 0x666666 });  // ❓ 灰色？

// Line ~172: 硬编码缩放目标尺寸
const targetSize = 10;  // ❓ 为什么是10？

问题分析

统计: 至少 10个 魔法数字，没有任何注释说明其含义或来源。

这些数字的问题：

  1. 可读性差: 新开发者无法理解这些数值的业务含义
  2. 可维护性差: 修改时需要搜索整个文件
  3. 一致性差: 不同地方可能使用相似的数字但含义不同

改进建议

// config/renderer.config.ts
export const RENDERER_CONFIG = {
  // 相机设置
  camera: {
    defaultPosition: new THREE.Vector3(0, 0, 20),
    fov: 45,
    near: 0.1,
    far: 1000,
  },
  
  // 光照设置
  lighting: {
    ambient: { color: 0xffffff, intensity: 0.6 },
    primaryDirectional: { 
      color: 0xffffff, 
      intensity: 0.8,
      position: new THREE.Vector3(10, 10, 10)
    },
    secondaryDirectional: {
      color: 0xffffff,
      intensity: 0.4,
      position: new THREE.Vector3(-10, -10, -10)
    },
  },
  
  // 原子几何设置
  atom: {
    sphereSegments: 32,  // 平衡质量与性能
    shininess: 100,
    specular: 0x444444,
  },
  
  // 化学键几何设置
  bond: {
    radius: 0.15,  // 相对于原子半径的比例
    cylinderSegments: 12,
    color: 0x666666,  // 标准化学键灰色
  },
  
  // 分子布局设置
  layout: {
    targetSize: 10,  // 适应视口的理想尺寸
  },
  
  // 默认颜色
  background: 0x1a1a2e,  // 深蓝色背景，减少眼疲劳
} as const;

// 使用示例
this.camera.position.copy(RENDERER_CONFIG.camera.defaultPosition);
const geometry = new THREE.SphereGeometry(
  radius, 
  RENDERER_CONFIG.atom.sphereSegments, 
  RENDERER_CONFIG.atom.sphereSegments
);

🟡 问题 3: 紧耦合与难以测试

文件: src/renderer/MolecularRenderer.ts

当前代码

export class MolecularRenderer {
  constructor(config: MolecularRendererConfig) {
    // 直接在构造函数中创建Three.js对象
    this.scene = new THREE.Scene();
    this.camera = new THREE.PerspectiveCamera(45, width / height, 0.1, 1000);
    this.renderer = new THREE.WebGLRenderer({ antialias: true });
    
    // 直接操作DOM
    this.container.appendChild(this.renderer.domElement);
    
    // 直接绑定全局事件
    window.addEventListener('resize', this.handleResize);
  }
}

测试代码的复杂性

// tests/renderer/MolecularRenderer.test.ts 中的模拟
jest.mock('three', () => {
  const actualThree = jest.requireActual('three');
  return {
    ...actualThree,
    WebGLRenderer: jest.fn().mockImplementation(() => ({
      setSize: jest.fn(),
      setPixelRatio: jest.fn(),
      render: jest.fn(),
      dispose: jest.fn(),
      domElement: document.createElement('canvas'),
    })),
  };
});

测试需要: 需要 mock 整个 Three.js 库才能进行单元测试！

改进建议 - 依赖注入

export interface RendererDependencies {
  scene: THREE.Scene;
  camera: THREE.PerspectiveCamera;
  renderer: THREE.WebGLRenderer;
  eventTarget: EventTarget;  // 抽象window对象
}

export class MolecularRenderer {
  constructor(
    config: MolecularRendererConfig,
    private deps: RendererDependencies = createDefaultDependencies(config)
  ) {
    // 使用注入的依赖，而非直接创建
    this.scene = deps.scene;
    this.camera = deps.camera;
    this.renderer = deps.renderer;
    
    // 可以通过注入的eventTarget进行事件管理
    this.eventTarget = deps.eventTarget;
  }
}

// 测试时注入mock对象
const mockRenderer = {
  setSize: jest.fn(),
  render: jest.fn(),
  dispose: jest.fn(),
  domElement: mockCanvas,
};

const renderer = new MolecularRenderer(
  config,
  { scene: mockScene, camera: mockCamera, renderer: mockRenderer, eventTarget: mockEventTarget }
);

🟡 问题 4: 复杂的资源清理逻辑

文件: src/renderer/MolecularRenderer.ts (lines 207-227)

当前代码

/**
 * Clear the current molecule from the scene
 */
clearMolecule(): void {
  while (this.moleculeGroup.children.length > 0) {
    const child = this.moleculeGroup.children[0];
    if (child instanceof THREE.Mesh) {
      child.geometry.dispose();
      if (Array.isArray(child.material)) {
        child.material.forEach(m => m.dispose());
      } else {
        child.material.dispose();
      }
    }
    this.moleculeGroup.remove(child);
  }
}

问题分析

  1. 类型检查复杂: Array.isArray(child.material) 表明类型不确定
  2. 手动资源管理: 容易遗漏（如 texture、geometry attributes）
  3. 循环逻辑复杂: while + children[0] 不如 for...of 清晰
  4. 缺乏抽象: 每个需要清理的地方都要重复这段逻辑

改进建议

// utils/resourceManager.ts
export class ResourceManager {
  private disposables: Set<THREE.Disposable> = new Set();
  
  track<T extends THREE.Disposable>(resource: T): T {
    this.disposables.add(resource);
    return resource;
  }
  
  dispose(): void {
    this.disposables.forEach(resource => {
      if ('dispose' in resource) {
        resource.dispose();
      }
    });
    this.disposables.clear();
  }
}

// 使用
class MolecularRenderer {
  private resourceManager = new ResourceManager();
  
  private createAtomMesh(atom: Atom): THREE.Mesh {
    const geometry = this.resourceManager.track(
      new THREE.SphereGeometry(radius, 32, 32)
    );
    const material = this.resourceManager.track(
      new THREE.MeshPhongMaterial({ color })
    );
    return new THREE.Mesh(geometry, material);
  }
  
  clearMolecule(): void {
    this.resourceManager.dispose();
  }
}

🟠 问题 5: 缺乏输入验证与错误处理

文件: src/renderer/MolecularRenderer.ts

当前代码

renderMolecule(molecule: Molecule): void {
  if (this.isDisposed) return;  // 静默失败！
  
  // 直接使用molecule，没有验证
  molecule.atoms.forEach(atom => {
    // 如果atom.x是undefined会怎样？
    const atomMesh = this.createAtomMesh(atom);
    this.moleculeGroup.add(atomMesh);
  });
}

private createBondMesh(bond: Bond, atoms: Atom[]): THREE.Mesh | null {
  const atom1 = atoms.find(a => a.id === bond.atom1Id);
  const atom2 = atoms.find(a => a.id === bond.atom2Id);
  
  if (!atom1 || !atom2) return null;  // 静默返回null！
  
  // ...创建化学键
}

问题分析

场景 当前行为 期望行为
Renderer已dispose 静默返回，无渲染 抛出错误或返回状态
化学键引用了不存在的原子 返回null，化学键不显示 抛出有意义的错误
atom.x/y/z为undefined NaN传播，可能导致渲染错误 输入验证错误
molecule为null/undefined 运行时错误 编译时类型保护

改进建议

// validation/moleculeValidator.ts
export class MoleculeValidationError extends Error {
  constructor(message: string, public invalidElements: string[]) {
    super(message);
    this.name = 'MoleculeValidationError';
  }
}

export function validateMolecule(molecule: Molecule): void {
  const errors: string[] = [];
  
  // 验证原子
  molecule.atoms.forEach((atom, index) => {
    if (typeof atom.x !== 'number') errors.push(`Atom[${index}].x is not a number`);
    if (typeof atom.y !== 'number') errors.push(`Atom[${index}].y is not a number`);
    if (typeof atom.z !== 'number') errors.push(`Atom[${index}].z is not a number`);
    if (!atom.element) errors.push(`Atom[${index}] has no element`);
  });
  
  // 验证化学键
  const atomIds = new Set(molecule.atoms.map(a => a.id));
  molecule.bonds.forEach((bond, index) => {
    if (!atomIds.has(bond.atom1Id)) {
      errors.push(`Bond[${index}] references non-existent atom: ${bond.atom1Id}`);
    }
    if (!atomIds.has(bond.atom2Id)) {
      errors.push(`Bond[${index}] references non-existent atom: ${bond.atom2Id}`);
    }
  });
  
  if (errors.length > 0) {
    throw new MoleculeValidationError('Invalid molecule data', errors);
  }
}

// 在renderMolecule中使用
renderMolecule(molecule: Molecule): void {
  if (this.isDisposed) {
    throw new Error('Cannot render: MolecularRenderer has been disposed');
  }
  
  validateMolecule(molecule);  // 提前验证
  
  // ...渲染逻辑
}

🟠 问题 6: 性能问题 - 无几何体/材质复用

文件: src/renderer/MolecularRenderer.ts (lines 112-127, 131-152)

当前代码

private createAtomMesh(atom: Atom): THREE.Mesh {
  const radius = getAtomRadius(atom.element);
  const color = getCPKColor(atom.element);
  
  // ❌ 每次渲染都创建新的几何体！
  const geometry = new THREE.SphereGeometry(radius, 32, 32);
  
  // ❌ 每次渲染都创建新的材质！
  const material = new THREE.MeshPhongMaterial({
    color: color,
    shininess: 100,
    specular: 0x444444,
  });
  
  return new THREE.Mesh(geometry, material);
}

问题分析

对于1000个原子的分子：

  • 创建 1000 个 SphereGeometry 对象
  • 创建 1000 个 MeshPhongMaterial 对象
  • GPU 内存占用极高
  • 渲染性能下降

Three.js最佳实践: 复用几何体和材质，使用InstancedMesh。

改进建议

// renderer/geometryCache.ts
export class GeometryCache {
  private geometries: Map<string, THREE.SphereGeometry> = new Map();
  private materials: Map<string, THREE.MeshPhongMaterial> = new Map();
  
  getAtomGeometry(radius: number): THREE.SphereGeometry {
    const key = radius.toFixed(2);
    if (!this.geometries.has(key)) {
      this.geometries.set(
        key, 
        new THREE.SphereGeometry(radius, 32, 32)
      );
    }
    return this.geometries.get(key)!;
  }
  
  getAtomMaterial(color: number): THREE.MeshPhongMaterial {
    const key = color.toString(16);
    if (!this.materials.has(key)) {
      this.materials.set(
        key,
        new THREE.MeshPhongMaterial({
          color: color,
          shininess: 100,
          specular: 0x444444,
        })
      );
    }
    return this.materials.get(key)!;
  }
  
  clear(): void {
    this.geometries.forEach(g => g.dispose());
    this.materials.forEach(m => m.dispose());
    this.geometries.clear();
    this.materials.clear();
  }
}

// 使用
class MolecularRenderer {
  private geometryCache = new GeometryCache();
  
  private createAtomMesh(atom: Atom): THREE.Mesh {
    const radius = getAtomRadius(atom.element);
    const color = getCPKColor(atom.element);
    
    // 复用缓存的几何体和材质
    const geometry = this.geometryCache.getAtomGeometry(radius);
    const material = this.geometryCache.getAtomMaterial(color);
    
    return new THREE.Mesh(geometry, material);
  }
}

🟢 问题 7: 未使用的代码

文件: src/types/molecule.ts (line 14)

当前代码

export interface Bond {
  id: string;
  atom1Id: string;
  atom2Id: string;
  order?: number; // 1 = single, 2 = double, 3 = triple
}

问题: order 字段被定义但 从未在代码中使用。

在 createBondMesh 中：

// 没有根据bond.order改变任何渲染属性
const geometry = new THREE.CylinderGeometry(0.15, 0.15, distance, 12);
const material = new THREE.MeshPhongMaterial({ color: 0x666666 });

建议

要么实现根据键级调整渲染的功能：

private createBondMesh(bond: Bond, atoms: Atom[]): THREE.Mesh {
  const order = bond.order || 1;
  const radius = 0.15 * (1 + (order - 1) * 0.3);  // 双键更粗
  const geometry = new THREE.CylinderGeometry(radius, radius, distance, 12);
  // ...
}

要么移除未使用的字段以保持代码简洁。

📊 复杂度总结

指标 当前值 建议值
MolecularRenderer类行数 277 < 100（拆分后）
每个方法平均行数 20 < 15
类职责数量 8 1
魔法数字数量 10+ 0（使用常量）
直接依赖数量 5+ < 3（通过DI）
测试覆盖率 低（需大量mock） 高（易于mock）

🎯 优先级建议

  • 高 - God Class拆分（影响可维护性）
  • 高 - 魔法数字提取（影响可读性）
  • 中 - 依赖注入重构（影响可测试性）
  • 中 - 资源管理优化（影响稳定性）
  • 低 - 键级功能实现或移除（功能完整性）

相关Commit

  • 引入代码的commit: f79f6b4 - feat(renderer): implement basic Three.js molecular renderer
  • 当前HEAD: fe8823b - fix: Update GitHub Actions for Node.js deployment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions