Skip to content

Commit 6b69022

Browse files
committed
Refactor Menu constructors to use delegation pattern
Updated the Menu class on macOS to use delegating constructors, consolidating initialization logic into the parameterized constructor. This eliminates code duplication and ensures consistent setup whether creating a new NSMenu or wrapping an existing native menu object. Also updated documentation to illustrate the constructor delegation pattern.
1 parent 4ce2597 commit 6b69022

File tree

2 files changed

+88
-25
lines changed

2 files changed

+88
-25
lines changed

.cursor/rules/pimpl-pattern.mdc

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,82 @@ MyClass::MyClass() : pimpl_(std::make_unique<Impl>()) {}
233233
MyClass::~MyClass() = default; // unique_ptr handles cleanup
234234
```
235235

236+
#### Constructor Delegation Pattern
237+
238+
When a class has multiple constructors, use **delegating constructors** to avoid code duplication. The default constructor delegates to the parameterized constructor:
239+
240+
```cpp
241+
// Header
242+
class TrayIcon {
243+
public:
244+
TrayIcon(); // Default constructor
245+
TrayIcon(void* native_tray); // Wraps existing native object
246+
virtual ~TrayIcon();
247+
248+
private:
249+
class Impl;
250+
std::unique_ptr<Impl> pimpl_;
251+
};
252+
253+
// Implementation - delegate to avoid duplication
254+
TrayIcon::TrayIcon() : TrayIcon(nullptr) {} // Delegate to parameterized constructor
255+
256+
TrayIcon::TrayIcon(void* tray) {
257+
// Handle both cases in one place
258+
NSStatusItem* status_item = nullptr;
259+
260+
if (tray == nullptr) {
261+
// Create new platform object
262+
NSStatusBar* status_bar = [NSStatusBar systemStatusBar];
263+
status_item = [status_bar statusItemWithLength:NSVariableStatusItemLength];
264+
} else {
265+
// Wrap existing platform object
266+
status_item = (__bridge NSStatusItem*)tray;
267+
}
268+
269+
// All initialization logic in one place
270+
pimpl_ = std::make_unique<Impl>(status_item);
271+
272+
// Additional setup that applies to both cases
273+
if (pimpl_->status_item_) {
274+
// Configure the status item...
275+
}
276+
}
277+
278+
TrayIcon::~TrayIcon() = default;
279+
```
280+
281+
**Benefits:**
282+
- ✅ Eliminates duplicate initialization code
283+
- ✅ Single source of truth for object setup
284+
- ✅ Easier to maintain and update
285+
- ✅ Prevents inconsistencies between constructors
286+
287+
**Without Delegation (Don't do this):**
288+
```cpp
289+
// Bad - duplicated initialization logic
290+
TrayIcon::TrayIcon() : pimpl_(std::make_unique<Impl>()) {
291+
NSStatusBar* status_bar = [NSStatusBar systemStatusBar];
292+
NSStatusItem* status_item = [status_bar statusItemWithLength:NSVariableStatusItemLength];
293+
pimpl_->status_item_ = status_item;
294+
295+
// Setup code duplicated...
296+
if (pimpl_->status_item_) {
297+
// Configure...
298+
}
299+
}
300+
301+
TrayIcon::TrayIcon(void* tray) : pimpl_(std::make_unique<Impl>()) {
302+
NSStatusItem* status_item = (__bridge NSStatusItem*)tray;
303+
pimpl_->status_item_ = status_item;
304+
305+
// Same setup code duplicated again!
306+
if (pimpl_->status_item_) {
307+
// Configure...
308+
}
309+
}
310+
```
311+
236312
### 4. Manager Classes with PIMPL
237313

238314
Singleton managers also use PIMPL ([window_manager.h](mdc:src/window_manager.h)):

src/platform/macos/menu_macos.mm

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -589,34 +589,21 @@ - (void)menuDidClose:(NSMenu*)menu {
589589
};
590590

591591
// Menu implementation
592-
Menu::Menu() : id(IdAllocator::Allocate<Menu>()) {
593-
NSMenu* ns_menu = [[NSMenu alloc] init];
594-
pimpl_ = std::make_unique<Impl>(ns_menu);
595-
objc_setAssociatedObject(ns_menu, kMenuIdKey, [NSNumber numberWithUnsignedInt:id],
596-
OBJC_ASSOCIATION_RETAIN_NONATOMIC);
592+
Menu::Menu() : Menu(nullptr) {}
597593

598-
// 设置默认的 Block 处理器,直接发送事件
599-
pimpl_->delegate_.openedBlock = ^(MenuId menu_id) {
600-
try {
601-
Emit<MenuOpenedEvent>(menu_id);
602-
} catch (...) {
603-
// Protect against event emission exceptions
604-
}
605-
};
594+
Menu::Menu(void* native_menu) : id(IdAllocator::Allocate<Menu>()) {
595+
NSMenu* ns_menu = nullptr;
606596

607-
pimpl_->delegate_.closedBlock = ^(MenuId menu_id) {
608-
try {
609-
Emit<MenuClosedEvent>(menu_id);
610-
} catch (...) {
611-
// Protect against event emission exceptions
612-
}
613-
};
614-
}
597+
if (native_menu == nullptr) {
598+
// Create new platform object
599+
ns_menu = [[NSMenu alloc] init];
600+
} else {
601+
// Wrap existing platform object
602+
ns_menu = (__bridge NSMenu*)native_menu;
603+
}
615604

616-
Menu::Menu(void* native_menu)
617-
: id(IdAllocator::Allocate<Menu>()),
618-
pimpl_(std::make_unique<Impl>((__bridge NSMenu*)native_menu)) {
619-
NSMenu* ns_menu = (__bridge NSMenu*)native_menu;
605+
// All initialization logic in one place
606+
pimpl_ = std::make_unique<Impl>(ns_menu);
620607
objc_setAssociatedObject(ns_menu, kMenuIdKey, [NSNumber numberWithUnsignedInt:id],
621608
OBJC_ASSOCIATION_RETAIN_NONATOMIC);
622609

0 commit comments

Comments
 (0)