From f4183b0913b43f984b04330d5ea35fa9a59a9012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 26 Feb 2012 10:19:04 +0100 Subject: [PATCH] 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. --- .../appearance/DecorSettingsView.cpp | 103 ++++++++---------- .../appearance/DecorSettingsView.h | 49 +++++---- 2 files changed, 68 insertions(+), 84 deletions(-) diff --git a/src/preferences/appearance/DecorSettingsView.cpp b/src/preferences/appearance/DecorSettingsView.cpp index c149c5ce78..d5afffd4ca 100644 --- a/src/preferences/appearance/DecorSettingsView.cpp +++ b/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, kallisti5@unixzen.com + * Stephan Aßmus */ @@ -14,6 +15,7 @@ #include #include +#include #include #include #include @@ -23,7 +25,6 @@ #include #include #include -#include #include #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); } - diff --git a/src/preferences/appearance/DecorSettingsView.h b/src/preferences/appearance/DecorSettingsView.h index 637f964aa1..520bde281d 100644 --- a/src/preferences/appearance/DecorSettingsView.h +++ b/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, kallisti5@unixzen.com + * Stephan Aßmus */ #ifndef DECOR_SETTINGS_VIEW_H #define DECOR_SETTINGS_VIEW_H -#include -#include -#include +#include +#include #include -#include - -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