Complete make over of DecorSettingsView inner workings

* Cleaned up header (why declare some methods protected?!)
 * Refactoring by applying basic "write only once" rule
 * DecorInfoUtility is now cached as member
 * This also fixes multiple instances of it being leaked.
 * Return code of SetDecorator() is now checked, so it should be hard
   to go out of sync with actually visible Decorator.
 * fSavedDecor and fCurrentDecor were pointers to the memory of
   temporary BStrings, this never worked!
 * IsDefaultable() always returned true, despite it being pretty easy
   to implement properly.
 * Revert() didn't do anything, probably because fSavedDecor was never
   initialized anywhere.
 * The UI was not updated in all situations to decorator changes.

Alexander, please have a close look at these changes.

src/preferences/appearance/DecorSettingsView.cpp

@@ -1,9 +1,10 @@
 /*
- *  Copyright 2010-2011 Haiku, Inc. All rights reserved.
+ *  Copyright 2010-2012 Haiku, Inc. All rights reserved.
  *  Distributed under the terms of the MIT license.
  *
  *	Authors:
  *		Alexander von Gluck, [email protected]
+ *		Stephan Aßmus <[email protected]>
  */
 
 
@@ -14,6 +15,7 @@
 
 #include <Alert.h>
 #include <Box.h>
+#include <Button.h>
 #include <Catalog.h>
 #include <GridLayoutBuilder.h>
 #include <GroupLayoutBuilder.h>
@@ -23,7 +25,6 @@
 #include <PopUpMenu.h>
 #include <Slider.h>
 #include <SpaceLayoutItem.h>
-#include <String.h>
 #include <TextView.h>
 
 #include "APRWindow.h"
@@ -92,39 +93,15 @@ DecorSettingsView::MessageReceived(BMessage *msg)
 		case kMsgSetDecor:
 		{
 			BString newDecor;
-			if (msg->FindString("decor", &newDecor) != B_OK)
-				break;
-
-			DecorInfoUtility* decorUtility
-				= new(std::nothrow) DecorInfoUtility();
-
-			if (decorUtility == NULL)
-				return;
-
-			DecorInfo* decor = decorUtility->FindDecorator(newDecor);
-			if (decor == NULL)
-				return;
-
-			fSavedDecor = fCurrentDecor;
-			fCurrentDecor = (char*)decor->Name().String();
-
-			decorUtility->SetDecorator(decor);
-
-			Window()->PostMessage(kMsgUpdate);
+			if (msg->FindString("decor", &newDecor) == B_OK)
+				_SetDecor(newDecor);
 			break;
 		}
 		case kMsgDecorInfo:
 		{
-			DecorInfoUtility* decorUtility
-				= new(std::nothrow) DecorInfoUtility();
-
-			if (decorUtility == NULL)
-				return;
-
-			BString decoratorName(fCurrentDecor);
-			DecorInfo* decor = decorUtility->FindDecorator(decoratorName);
+			DecorInfo* decor = fDecorUtility.FindDecorator(fCurrentDecor);
 			if (decor == NULL)
-				return;
+				break;
 
 			BString authorsText(decor->Authors().String());
 			authorsText.ReplaceAll(", ", "\n    ");
@@ -146,11 +123,12 @@ DecorSettingsView::MessageReceived(BMessage *msg)
 			infoAlert->SetShortcut(0, B_ESCAPE); 
 			infoAlert->Go(); 
 
-			Window()->PostMessage(kMsgUpdate);
 			break;
 		}
+
 		default:
 			BView::MessageReceived(msg);
+			break;
 	}
 }
 
@@ -159,18 +137,11 @@ void
 DecorSettingsView::_BuildDecorMenu()
 {
 	fDecorMenu = new BPopUpMenu(B_TRANSLATE("Choose Decorator"));
-	DecorInfo* decorator = NULL;
 
 	// collect the current system decor settings
-	DecorInfoUtility* decorUtility = new(std::nothrow) DecorInfoUtility();
-
-	if (decorUtility == NULL) {
-		return;
-	}
-
-	int32 count = decorUtility->CountDecorators();
+	int32 count = fDecorUtility.CountDecorators();
 	for (int32 i = 0; i < count; ++i) {
-		decorator = decorUtility->DecoratorAt(i);
+		DecorInfo* decorator = fDecorUtility.DecoratorAt(i);
 		if (decorator == NULL) {
 			fprintf(stderr, "Decorator : error NULL entry @ %li / %li\n",
 				i, count);
@@ -186,18 +157,41 @@ DecorSettingsView::_BuildDecorMenu()
 
 		fDecorMenu->AddItem(item);
 	}
-	fCurrentDecor = (char*)decorUtility->CurrentDecorator()->Name().String();
-	delete decorUtility;
 
-	_SetCurrentDecor();
+	_AdoptToCurrentDecor();
 }
 
 
 void
-DecorSettingsView::_SetCurrentDecor()
+DecorSettingsView::_SetDecor(const BString& name)
 {
-	BMenuItem *item = fDecorMenu->FindItem(fCurrentDecor);
-	BString currDecor = fCurrentDecor;
+	_SetDecor(fDecorUtility.FindDecorator(name));
+}	
+
+
+void
+DecorSettingsView::_SetDecor(DecorInfo* decorInfo)
+{
+	if (fDecorUtility.SetDecorator(decorInfo) == B_OK) {
+		_AdoptToCurrentDecor();
+		Window()->PostMessage(kMsgUpdate);
+	}
+}	
+
+
+void
+DecorSettingsView::_AdoptToCurrentDecor()
+{
+	fCurrentDecor = fDecorUtility.CurrentDecorator()->Name();
+	if (fSavedDecor.Length() == 0)
+		fSavedDecor = fCurrentDecor;
+	_AdoptInterfaceToCurrentDecor();
+}
+
+void
+DecorSettingsView::_AdoptInterfaceToCurrentDecor()
+{
+	BMenuItem* item = fDecorMenu->FindItem(fCurrentDecor);
 	if (item != NULL)
 		item->SetMarked(true);
 }
@@ -206,23 +200,14 @@ DecorSettingsView::_SetCurrentDecor()
 void
 DecorSettingsView::SetDefaults()
 {
-	DecorInfoUtility* decorUtility
-		= new(std::nothrow) DecorInfoUtility();
-
-	if (decorUtility == NULL)
-		return;
-	DecorInfo* defaultDecorator(decorUtility->DefaultDecorator());
-	decorUtility->SetDecorator(defaultDecorator);
-	_BuildDecorMenu();
-
-	delete decorUtility;
+	_SetDecor(fDecorUtility.DefaultDecorator());
 }
 
 
 bool
 DecorSettingsView::IsDefaultable()
 {
-	return true;
+	return fCurrentDecor != fDecorUtility.DefaultDecorator()->Name();
 }
 
 
@@ -236,7 +221,5 @@ DecorSettingsView::IsRevertable()
 void
 DecorSettingsView::Revert()
 {
-	if (!IsRevertable())
-		return;
+	_SetDecor(fSavedDecor);
 }
-

src/preferences/appearance/DecorSettingsView.h

@@ -1,54 +1,55 @@
 /*
- *  Copyright 2010-2011 Haiku, Inc. All rights reserved.
+ *  Copyright 2010-2012 Haiku, Inc. All rights reserved.
  *  Distributed under the terms of the MIT license.
  *
  *  Authors:
  *      Alexander von Gluck, [email protected]
+ *		Stephan Aßmus <[email protected]>
  */
 #ifndef DECOR_SETTINGS_VIEW_H
 #define DECOR_SETTINGS_VIEW_H
 
 
-#include <Button.h>
-#include <GroupView.h>
-#include <InterfaceDefs.h>
+#include <DecorInfo.h>
+#include <String.h>
 #include <View.h>
 
-#include <DecorInfo.h>
 
-
-class BBox;
+class BButton;
 class BMenuField;
 class BPopUpMenu;
 
 
 class DecorSettingsView : public BView {
 public:
-							DecorSettingsView(const char* name);
-	virtual					~DecorSettingsView();
+								DecorSettingsView(const char* name);
+	virtual						~DecorSettingsView();
 
-	virtual	void			AttachedToWindow();
-	virtual	void			MessageReceived(BMessage* message);
+	virtual	void				AttachedToWindow();
+	virtual	void				MessageReceived(BMessage* message);
 
-			void			SetDefaults();
-			void			Revert();
-			bool			IsDefaultable();
-			bool			IsRevertable();
+			void				SetDefaults();
+			void				Revert();
+			bool				IsDefaultable();
+			bool				IsRevertable();
 
 private:
-			void			_BuildDecorMenu();
-			void			_SetCurrentDecor();
+			void				_SetDecor(const BString& name);
+			void				_SetDecor(BPrivate::DecorInfo* decorInfo);
 
-			BButton*		fDecorInfoButton;	
+			void				_BuildDecorMenu();
+			void				_AdoptToCurrentDecor();
+			void				_AdoptInterfaceToCurrentDecor();
 
-protected:
-			float			fDivider;
+private:
+			DecorInfoUtility	fDecorUtility;
 
-			BMenuField*		fDecorMenuField;
-			BPopUpMenu*		fDecorMenu;
+			BButton*			fDecorInfoButton;
+			BMenuField*			fDecorMenuField;
+			BPopUpMenu*			fDecorMenu;
 
-			char*			fSavedDecor;
-			char*			fCurrentDecor;
+			BString				fSavedDecor;
+			BString				fCurrentDecor;
 };
 
 #endif // DECOR_SETTINGS_VIEW_H