Not sure how/whether to refactor this.

Started by
0 comments, last by Verg 18 years, 8 months ago
Long source box ahead. I'm cleaning up a lot of my source code for my IM client. Attached is the header file for the contact list window. Normally I do pretty well to constrain a class to one responsibility, but my eyebrows are raised when the implementation file for this particular class is over 1,700 lines long. Does this make it a candidate for refactoring? It is pretty unwieldy code, as it stands. I'm not sure if its even possible to split up, however. I think the main problem is the fact that it both presents the contact list, as well as having a number of bindings to the main app event system. Can anyone suggest a way to break this up into more manageable pieces?

	class ContactListWindow :
		public CFrameWindowImpl<ContactListWindow, CWindow, CFrameWinTraits>
	{
	public:
		ContactListWindow(ContactListPrefs& prefs);
		~ContactListWindow();

		DECLARE_FRAME_WND_CLASS(L"ContactListWindow", 0)

		BEGIN_MSG_MAP(ContactListWindow)
			COMMAND_ID_HANDLER_EX(ID_OPENIM_ACCOUNTS, OnAccounts)
			COMMAND_ID_HANDLER_EX(ID_OPENIM_OPTIONS, OnOptions)
			COMMAND_ID_HANDLER_EX(ID_OPENIM_EXIT, OnExit)
			COMMAND_ID_HANDLER_EX(ID_VIEW_SHOWOFFLINECONTACTS, OnShowOfflineContacts)
			COMMAND_ID_HANDLER_EX(ID_HELP_FEEDBACK, OnFeedback)
			COMMAND_ID_HANDLER_EX(ID_HELP_ABOUT, OnAbout)
			MSG_WM_CLOSE(OnClose)
			MSG_WM_CONTEXTMENU(OnContextMenu)
			MSG_WM_CREATE(OnCreate)
			MESSAGE_HANDLER(WM_DEFERREDUPDATE, OnDeferredUpdate)
			//MSG_WM_DISPLAYCHANGE(OnDisplayChange)
			MESSAGE_HANDLER(WM_EDITITEM, OnEditItem)
			MESSAGE_HANDLER(WM_SHOWSTATUSMENU, OnShowStatusMenu)
			MSG_WM_LBUTTONUP(OnLButtonUp)
			MSG_WM_RBUTTONDOWN(OnRButtonDown)
			MSG_WM_PARENTNOTIFY(OnParentNotify)
			MSG_WM_MOUSEMOVE(OnMouseMove)
			MSG_WM_MENUSELECT(OnMenuSelect)
			MSG_WM_SIZE(OnSize)
			MESSAGE_HANDLER(WM_STARTCONVERSATION, OnStartConversation)
			MESSAGE_HANDLER(WM_TIMER, OnTimer)
			NOTIFY_CODE_HANDLER(NM_CUSTOMDRAW, OnCustomDraw)
			NOTIFY_CODE_HANDLER(NM_DBLCLK, OnDoubleClick)
			NOTIFY_CODE_HANDLER(TTN_GETDISPINFO, OnToolTipGetDispInfo);
			NOTIFY_CODE_HANDLER(TVN_KEYDOWN, OnKeyDown)
			NOTIFY_CODE_HANDLER(TVN_BEGINDRAG, OnBeginDrag)
			NOTIFY_CODE_HANDLER(TVN_BEGINLABELEDIT, OnBeginLabelEdit)
			NOTIFY_CODE_HANDLER(TVN_ENDLABELEDIT, OnEndLabelEdit)
			CHAIN_MSG_MAP(CFrameWindowImpl<ContactListWindow>)
		END_MSG_MAP()
	
		void			ApplyPreferences();
		void			EnforceConstraints();
		void			OnEnumWindows(HWND wnd);

	private:
		enum NodeType
		{
			AccountNodeType,
			ContactNodeType,
			GroupNodeType
		};

		template<typename T>
		struct Node
		{
			NodeType	Type;
			T*			Data;
			CTreeItem	Item;
			HTREEITEM	Parent;
			int			Position;
		};

		struct AccountNode :
			public Node<Account>
		{
			UINT_PTR	SignOnTimer;
			bool		SigningOn;

		};

		typedef std::deque<Node<Contact>*>	ContactNodes;

		bool			CanMove(CTreeItem movingItem, CTreeItem targetItem);
		void			Expand(CTreeItem root);
		Account*		GetAccountFromNode(Node<void>*);
		int				GetIconIndex(Account* account);
		int				GetIconIndex(Contact* contact);
		String			GetLabel(Account*);
		String			GetLabel(Contact*);
		String			GetLabel(Group*);
		HTREEITEM		GetInsertPosition(HTREEITEM parent, int position);
		void			OnAbout(UINT, int, HWND);
		void			OnAccounts(UINT, int, HWND);
		void			OnAccountConnect(AccountConnectEvent&);
		void			OnAccountConnectError(AccountConnectErrorEvent&);
		void			OnAccountCreate(AccountCreateEvent&);
		void			OnAccountDestroy(AccountDestroyEvent&);
		void			OnAccountDisconnect(AccountDisconnectEvent&);
		void			OnAccountError(AccountErrorEvent&);
		void			OnAccountStatusUpdate(AccountStatusUpdateEvent&);
		void			OnAccountUpdate(AccountUpdateEvent&);
		void			OnAppGlobalSetStatus(AppGlobalSetStatusEvent&);
		LRESULT			OnBeginDrag(WPARAM, NMHDR* nmhdr, BOOL&);
		LRESULT			OnBeginLabelEdit(WPARAM, NMHDR* nmhdr, BOOL&);
		void			OnClose();
		void			OnContactCreate(ContactCreateEvent&);
		void			OnContactDelete(Node<Contact>* node);
		void			OnContactDestroy(ContactDestroyEvent&);
		void			OnContactUpdate(ContactUpdateEvent&);
		void			OnContextMenu(HWND, CPoint);
		void			OnContextMenuAccount(AccountNode* node, CPoint pt);
		void			OnContextMenuContact(Node<Contact>* node, CPoint pt);
		void			OnContextMenuGroup(Node<Group>* node, CPoint pt);
		LRESULT			OnCreate(CREATESTRUCT*);
		LRESULT			OnCustomDraw(WPARAM, NMHDR* nmhdr, BOOL&);
		LRESULT			OnDeferredUpdate(UINT, WPARAM, LPARAM, BOOL&);
		void			OnDisplayChange(UINT imageDepth, CSize resolution);
		LRESULT			OnDoubleClick(WPARAM, NMHDR* nmhdr, BOOL&);
		LRESULT			OnEditItem(UINT, WPARAM, LPARAM, BOOL&);
		LRESULT			OnEndLabelEdit(WPARAM, NMHDR* nmhdr, BOOL&);
		void			OnExit(UINT, int, HWND);
		void			OnFeedback(UINT, int, HWND);
		void			OnFinalMessage(HWND);
		void			OnGroupCreate(GroupCreateEvent&);
		void			OnGroupDelete(Node<Group>* node);
		void			OnGroupDestroy(GroupDestroyEvent&);
		void			OnGroupUpdate(GroupUpdateEvent&);
		LRESULT			OnKeyDown(WPARAM, NMHDR* nmhdr, BOOL&);
		void			OnLButtonUp(UINT nFlags, CPoint point);
		void			OnRButtonDown(UINT nFlags, CPoint point);
		void			OnMenuSelect(UINT, UINT, HMENU ) {}
		void			OnMouseMove(UINT nFlags, CPoint point);
		void			OnOptions(UINT, int, HWND);
		void			OnParentNotify(UINT message, UINT /*msg2*/, LPARAM lParam);
		void			OnPreferencesUpdated(PreferencesUpdatedEvent&);
		void			OnSize(UINT, CSize);
		void			OnShowOfflineContacts(UINT, int, HWND);
		LRESULT			OnShowStatusMenu(UINT, WPARAM, LPARAM, BOOL&);
		void			OnShutdown(ShutdownEvent&);
		LRESULT			OnStartConversation(UINT, WPARAM, LPARAM, BOOL&);
		LRESULT			OnTimer(UINT, WPARAM, LPARAM, BOOL&);
		void			OnTimerEvent(TimerEvent&);
		LRESULT			OnToolTipGetDispInfo(WPARAM, NMHDR* nmhdr, BOOL&);
		void			Refresh(HTREEITEM item = TVI_ROOT);
		int				ShowContextMenu(CMenu& menu, CPoint where);
		void			Update(Account* account);
		void			Update(Contact* contact);
		void			Update(Group* group);
		void			Update(HTREEITEM item);

	private:
		enum Transaction
		{
			None,
			AddContact,
			AddGroup,
			RenameContact,
			RenameGroup
		};

		typedef std::map<String, AccountNode* >		AccountTable;
		typedef std::map<String, ContactNodes* >	ContactTable;	
		typedef std::map<String, Node<Group>* >		GroupTable;
	
		CTreeViewCtrlEx			m_tree;
		CMultiPaneStatusBarCtrl	m_statusBar;
		ImageMap				m_imageMap;
		CToolTipCtrl			m_toolTip;
		ContactListPrefs&		m_prefs;
		PreferencesPane			m_pane;
		Transaction				m_transaction;

		AccountTable			m_accounts;
		ContactTable			m_contacts;
		GroupTable				m_groups;

		CMenu					m_accountContextMenu;
		CMenu					m_contactContextMenu;
		CMenu					m_groupContextMenu;
		CMenu					m_treeContextMenu;		
		CMenu					m_statusChangeMenu;

		bool					m_dragging;
		CTreeItem				m_draggedItem;
		bool					m_disableAlwaysOnTop;
	};

--God has paid us the intolerable compliment of loving us, in the deepest, most tragic, most inexorable sense.- C.S. Lewis
Advertisement
Whoa! That is pretty big...

Seems like most of it is dedicated to event handling, which is a fairly major undertaking in itself...

Have you tried using a std::multimap to map the events to functors, or objects?

MFC is at least partly to blame for the bloat, though, no?

What I do in cases like this is start pooling function names and data into smaller sets of responsibilities, and then seeing which ones are used in multiple "responsibility sets". Once I've limited each one to a responsibility set, I'm ready to break the classes up...

Usually a few delegation classes end up aggregating a bunch of smaller worker classes in the end (but I'm sure you know how to refactor)... just my approach, anyway...

But, as to your question... yes, I think it's possible to refactor this beast-- there is more than one responsibility in there.

[smile]
my_life:          nop          jmp my_life
[ Keep track of your TDD cycle using "The Death Star" ] [ Verge Video Editor Support Forums ] [ Principles of Verg-o-nomics ] [ "t00t-orials" ]

This topic is closed to new replies.

Advertisement