216 lines
7.3 KiB
Markdown
216 lines
7.3 KiB
Markdown
# Recipe 模块代码审阅报告
|
||
|
||
## 一、架构概览
|
||
|
||
配方模块采用 MVVM 架构，分为四层配方体系：Carrier → Substrate → Wafer → Process，形成级联关系。使用 Stylet 框架 + JSON 文件持久化。
|
||
|
||
```mermaid
|
||
graph TD
|
||
A[CarrierRecipe 载具配方] --> B[SubstrateRecipe 基板配方]
|
||
B --> C[WaferRecipe 芯片配方]
|
||
C --> D[ProcessRecipe 工艺配方]
|
||
|
||
E[RecipeManager] --> A
|
||
E --> B
|
||
E --> C
|
||
E --> D
|
||
|
||
F[RecipeWrapManager] --> G[RecipeWrap 列表]
|
||
|
||
H[RecipeViewModel] --> I[CarrierRecipeViewModel]
|
||
H --> J[SubstrateRecipeViewModel]
|
||
H --> K[WaferRecipeViewModel]
|
||
H --> L[ProcessRecipeViewModel]
|
||
```
|
||
|
||
---
|
||
|
||
## 二、发现的问题
|
||
|
||
### 问题 1：ViewModel 层存在严重的代码重复（高优先级）
|
||
|
||
`CarrierRecipeViewModel`、`SubstrateRecipeViewModel`、`WaferRecipeViewModel`、`ProcessRecipeViewModel` 四个类中，以下逻辑几乎完全相同：
|
||
|
||
- `SelectedRecipeWrap` 属性的 setter 逻辑（离开检查 + 加载配方 + 快照）
|
||
- `RegisterButtonGroupEvents()` 方法（18 行事件注册代码完全一致）
|
||
- `TryLeaveCurrentRecipeContext()` 方法
|
||
- `SaveCurrentRecipe()` / `CaptureSavedSnapshot()` / `IsRecipeDirty()` / `SelectRecipeWrapInternal()` 方法
|
||
- `OnCreateNewRecipe` / `OnDeleteRecipe` / `OnOpenRecipe` / `OnSaveRecipe` / `OnClearRecipe` 等事件处理
|
||
|
||
**改进建议**：提取一个泛型基类 `RecipeViewModelBase<TRecipe> where TRecipe : RecipeBase`，将公共逻辑下沉，子类只需覆写差异部分（如加载配方的具体类型、级联逻辑等）。
|
||
|
||
---
|
||
|
||
### 问题 2：RecipeButtonGroupViewModel 在 ViewModel 中直接创建 View（中优先级）
|
||
|
||
`RecipeButtonGroupViewModel.ShowRecipeWindow<TWindow, TViewModel>()` 方法直接 `new TWindow()`，违反了 MVVM 的核心原则——ViewModel 不应直接创建和管理 View。
|
||
|
||
**改进建议**：使用 Stylet 的 `IWindowManager.ShowDialog()` 来显示对话框窗口，或引入一个 `IDialogService` 接口来解耦。
|
||
|
||
---
|
||
|
||
### 问题 3：RecipeNameWindowModel 与 RecipeReNameWindowModel 代码重复（中优先级）
|
||
|
||
这两个类的验证逻辑、`OkCommand`/`CancelCommand`、`UpdateValidationMessage()` 方法几乎完全一致，`RecipeReNameWindowModel` 仅多了一个 `OldRecipeName` 属性。
|
||
|
||
**改进建议**：让 `RecipeReNameWindowModel` 继承 `RecipeNameWindowModel`，或提取公共基类。
|
||
|
||
---
|
||
|
||
### 问题 4：RecipeBase 同时实现 INotifyPropertyChanged 和继承 JsonFileWritableBase（中优先级）
|
||
|
||
`RecipeBase` 手动实现了 `INotifyPropertyChanged`（包含 `SetProperty<T>` 辅助方法），但其子类的子属性模型（如 `SubstrateInfo`、`MarkData` 等）继承的是 Stylet 的 `PropertyChangedBase`，使用 `SetAndNotify`。这导致项目中存在两套属性变更通知机制。
|
||
|
||
**改进建议**：统一通知机制。如果 `JsonFileWritableBase` 不能继承 `PropertyChangedBase`，考虑用组合代替继承，或让 `RecipeBase` 也使用 Stylet 的通知基础设施。
|
||
|
||
---
|
||
|
||
### 问题 5：CameraBaseViewModel 放在 Models 目录下（低优先级）
|
||
|
||
`CameraBaseViewModel` 是一个 ViewModel 基类，但放在了 `Recipe/Models/` 目录下，违反了项目的分层约定。
|
||
|
||
**改进建议**：移动到 `Recipe/ViewModel/` 目录。
|
||
|
||
---
|
||
|
||
### 问题 6：命名拼写错误（低优先级）
|
||
|
||
- `SubtrateMarkPars` → 应为 `SubstrateMarkPars`（缺少 `s`）
|
||
- `SubtrateMarkParameterInfo` → 应为 `SubstrateMarkParameterInfo`
|
||
- `ThickNess` → 应为 `Thickness`（大小写不一致）
|
||
- `PIDOperater` → 应为 `PIDOperator`
|
||
|
||
**改进建议**：统一修正拼写，使用 IDE 的重命名功能确保所有引用同步更新。
|
||
|
||
---
|
||
|
||
### 问题 7：PIDOperater 使用反射遍历属性，且通过 IoC.Get 静态调用获取硬件（中优先级）
|
||
|
||
`PIDOperater.SendPIDParameters()` 和 `ReadPIDParametersFromDevice()` 使用反射遍历所有属性来读写 PID 参数，性能不佳且不易维护。同时 `GetControler()` 通过 `IoC.Get<HardwareManager>()` 静态调用获取依赖，违反了依赖注入原则。
|
||
|
||
**改进建议**：
|
||
- 将 `IMotionController` 或 `HardwareManager` 通过构造函数注入
|
||
- 考虑用显式的参数列表或字典代替反射遍历
|
||
|
||
---
|
||
|
||
### 问题 8：RecipeManager 的删除操作缺少异常处理（中优先级）
|
||
|
||
`DeleteCarrierRecipe` / `DeleteSubstrateRecipe` / `DeleteWaferRecipe` / `DeleteProcessRecipe` 直接调用 `Directory.Delete(path, true)` 而没有任何异常处理。如果目录不存在或被占用，会直接抛出异常。
|
||
|
||
**改进建议**：添加 try-catch 和存在性检查，返回操作结果或错误信息。
|
||
|
||
---
|
||
|
||
### 问题 9：OnCopyRecipe / OnImportRecipe / OnExportRecipe 为空实现（低优先级）
|
||
|
||
多个 ViewModel 中的 `OnCopyRecipe`、`OnImportRecipe`、`OnExportRecipe` 方法体为空，功能未实现。
|
||
|
||
**改进建议**：要么实现这些功能，要么在 UI 上禁用对应按钮（通过 `CanExecute` 返回 false），避免用户点击后无反应。
|
||
|
||
---
|
||
|
||
### 问题 10：CarrierRecipeViewModel.OnViewLoaded 中直接访问 RecipeWraps[0] 可能越界（高优先级）
|
||
|
||
```csharp
|
||
// CarrierRecipeViewModel.cs:248
|
||
RecipeWraps[0].IsInUse = true;
|
||
```
|
||
|
||
如果 `RecipeWraps` 为空集合，这行代码会抛出 `ArgumentOutOfRangeException`。
|
||
|
||
**改进建议**：添加空集合检查。
|
||
|
||
---
|
||
|
||
### 问题 11：SubstrateRecipeViewModel.OnViewLoaded 中 SelectedRecipeWrap 可能为 null（高优先级）
|
||
|
||
```csharp
|
||
// SubstrateRecipeViewModel.cs:498-499
|
||
SelectedRecipeWrap = RecipeWraps.Where(...).FirstOrDefault();
|
||
SelectedRecipeWrap.IsInUse = true; // 可能 NullReferenceException
|
||
```
|
||
|
||
`FirstOrDefault()` 可能返回 null，直接访问 `.IsInUse` 会崩溃。
|
||
|
||
**改进建议**：添加 null 检查。
|
||
|
||
---
|
||
|
||
### 问题 12：RecipeWrapManager.Handle 方法中复用同一个 CarrierRecipe 实例（中优先级）
|
||
|
||
```csharp
|
||
// RecipeWrapManager.cs:74
|
||
CarrierRecipe carrierRecipe = new CarrierRecipe();
|
||
foreach (var carrierWrap in CarrierRecipeWraps)
|
||
{
|
||
carrierRecipe.RecipeName = carrierWrap.RecipeName;
|
||
carrierRecipe.Read();
|
||
...
|
||
}
|
||
```
|
||
|
||
在循环中复用同一个 `CarrierRecipe` 实例，每次 `Read()` 会覆盖上一次的数据。虽然功能上可能没问题，但语义不清晰，且如果 `Read()` 失败可能导致残留旧数据。
|
||
|
||
**改进建议**：每次循环创建新实例。
|
||
|
||
---
|
||
|
||
### 问题 13：MarkTeachViewModel 直接持有 Window 引用（中优先级）
|
||
|
||
```csharp
|
||
private MarkCoordinatePreviewWindow _coordinatePreviewWindow;
|
||
```
|
||
|
||
ViewModel 直接持有 `Window` 实例引用，违反 MVVM 原则。
|
||
|
||
**改进建议**：使用 `IWindowManager` 管理窗口生命周期，或通过 Messenger/EventAggregator 通知 View 层打开窗口。
|
||
|
||
---
|
||
|
||
### 问题 14：事件参数类散落在不同位置（低优先级）
|
||
|
||
- `RecipeEventArgs` / `RecipeRenameEventArgs` 在 `Recipe/Models/RecipeEventArgs.cs` 中
|
||
- `SubstrateNameChangedEventArgs` 等在 `EventArgsFolder/RecipeNameChangedEventArgs.cs` 中，且命名空间为根命名空间 `MainShell`
|
||
|
||
**改进建议**：统一事件参数的命名空间和存放位置，建议放在 `Recipe/Models/` 或专门的 `Recipe/Events/` 目录下。
|
||
|
||
---
|
||
|
||
### 问题 15：SubstrateHeightMeasureSetting 和 SubstrateHeightMeasurePoint 未实现 IParameterItem（低优先级）
|
||
|
||
其他模型类（如 `SubstrateInfo`、`CarrierInfo`）都实现了 `IParameterItem` 接口以支持 `Clone()`，但 `SubstrateHeightMeasureSetting` 和 `SubstrateHeightMeasurePoint` 没有实现，可能导致序列化/克隆时行为不一致。
|
||
|
||
**改进建议**：统一实现 `IParameterItem` 接口。
|
||
|
||
---
|
||
|
||
### 问题 16：ActiveRecipeService 未被使用（低优先级）
|
||
|
||
`IActiveRecipeService<T>` 和 `ActiveRecipeService<T>` 定义了但在 Recipe 模块中未见使用。
|
||
|
||
**改进建议**：确认是否为未来预留，如果不需要则移除以减少代码噪音。
|
||
|
||
---
|
||
|
||
## 三、改进优先级汇总
|
||
|
||
| 优先级 | 问题编号 | 简述 |
|
||
|--------|---------|------|
|
||
| 高 | 1 | 四个 RecipeViewModel 严重代码重复，提取泛型基类 |
|
||
| 高 | 10 | CarrierRecipeViewModel.OnViewLoaded 数组越界风险 |
|
||
| 高 | 11 | SubstrateRecipeViewModel.OnViewLoaded 空引用风险 |
|
||
| 中 | 2 | ButtonGroupViewModel 直接创建 View |
|
||
| 中 | 3 | 两个命名窗口 Model 代码重复 |
|
||
| 中 | 4 | 两套属性变更通知机制并存 |
|
||
| 中 | 7 | PIDOperater 反射 + 静态 IoC 调用 |
|
||
| 中 | 8 | 删除操作缺少异常处理 |
|
||
| 中 | 12 | RecipeWrapManager 循环中复用实例 |
|
||
| 中 | 13 | MarkTeachViewModel 持有 Window 引用 |
|
||
| 低 | 5 | CameraBaseViewModel 放错目录 |
|
||
| 低 | 6 | 命名拼写错误 |
|
||
| 低 | 9 | 空实现的操作方法 |
|
||
| 低 | 14 | 事件参数类位置分散 |
|
||
| 低 | 15 | 部分模型未实现 IParameterItem |
|
||
| 低 | 16 | ActiveRecipeService 未使用 |
|