Refaktoryzacja kodu - switch case

0

Cześć wszystkim

Piszę pierwszy swój projekt z GUI w oparciu o GTK2+ (wiem, że jest GTK3, ale za późno się zorientowałem i nie chcę już po prostu zmieniać koncepcji) i przy budowaniu menu natrafiłem na switche. Na stacku przeczytałem, że switche ogólnie się są anty-patternem, ale coś mi mówi, że takie rozwiązanie jeśli chodzi o budowanie każdego menu z osobna jest zbyt rozbudowane. Chciałbym się dowiedzieć czy to podejście ze switchami jest prawidłowe czy jest jakieś inne rozwiązanie, żeby menu zbudować.

Tak wygląda kod jednego z sześciu menu. Oczywiście liczba itemów różni się między każdym z menu, a więc ilość case'ów się zmienia.

void add_items_to_file_menu(struct f_menu *file_menu, GtkAccelGroup *accel_group)
{
	GtkWidget *tmp = NULL;

	for (int i = 0; i < 7; i++) {
		switch (i) {
			case 0:
			{
				tmp = file_menu->new_file = create_item(GTK_STOCK_NEW); 
				gtk_widget_add_accelerator(file_menu->new_file, "activate", accel_group, 
										   GDK_n, GDK_CONTROL_MASK, GTK_ACCEL_VISIBLE);
			} break;
			case 1: 
			{
				tmp = file_menu->open = create_item(GTK_STOCK_OPEN); 
				gtk_widget_add_accelerator(file_menu->open, "activate", accel_group,
										   GDK_o, GDK_CONTROL_MASK, GTK_ACCEL_VISIBLE);
			} break;
			case 2:
			{
				tmp = file_menu->save = create_item(GTK_STOCK_SAVE);
				gtk_widget_add_accelerator(file_menu->save, "activate", accel_group,
										   GDK_s, GDK_CONTROL_MASK, GTK_ACCEL_VISIBLE);
			} break;
			case 3: 
			{
				tmp = file_menu->save_as = create_item( GTK_STOCK_SAVE_AS); 
				gtk_widget_add_accelerator(file_menu->save_as, "activate", accel_group,
										   GDK_s, (GDK_SHIFT_MASK + GDK_CONTROL_MASK), 
										   GTK_ACCEL_VISIBLE);
			} break;
			case 4: tmp = file_menu->export_file = gtk_menu_item_new_with_label("Export..."); break;
			case 5: 
			{
				tmp = file_menu->close = create_item(GTK_STOCK_CLOSE); 
				gtk_widget_add_accelerator(file_menu->close, "activate", accel_group,
										   GDK_w, GDK_CONTROL_MASK, GTK_ACCEL_VISIBLE);
			} break;
			case 6: 
			{
				tmp = file_menu->quit = create_item(GTK_STOCK_QUIT); 
				gtk_widget_add_accelerator(file_menu->quit, "activate", accel_group,
										   GDK_q, GDK_CONTROL_MASK, GTK_ACCEL_VISIBLE);
			} break;
		}
		gtk_menu_shell_append(GTK_MENU_SHELL(file_menu->container), tmp);
	}
}

Switch jest wykorzystywany po to, aby ustalić kolejność ustawienia widgetów w menu. Czy to jest dobre podejście do tego typu problemów pisząc w języku C? Jeśli nie to czy jest jakieś inne rozwiązanie?

2

Po co Ci tam w ogóle pętla i ten switch :P ?
Kolejność widgetów w menu i tak będzie zdefiniowana przez kolejność w kodzie.
Żeby odpalić jedną funkcję dla każdego itema (gtk_menu_shell_append(GTK_MENU_SHELL(file_menu->container), tmp);) dodajesz po 3 linijki w switchu...

Jak już musisz w pętli, to może zrób tablicę struktur z danymi, które się zmieniają dla każdego itema. Będziesz iterował w pętli for po tablicy, podstawiając odpowiednie pola w odpowiednie miejsca.

A jak chcesz zapisać krótko, bez dodatkowych struktur i bez pętli, to powtarzalny kod opakuj w funkcję:

void create_menu_item(param1, param2, param3, ...) {
    param1 = create_item(param2); 
    gtk_widget_add_accelerator(param1, "activate", accel_group, param3, param4, param5);
    gtk_menu_shell_append(GTK_MENU_SHELL(file_menu->container), param1);
}

Gdzieniegdzie zapewne trzeba będzie zastosować odpowiednie operatory do referencji/wskaźników, ale w to się nie wgłębiam.
Jak można zauważyć, pozbyłem się tmp i jestem prawie pewien, że to zadziała ;)

0

To prawda i nawet zapisałem takie rozwiązanie w swoim programie, aczkolwiek przeraża mnie trochę liczba argumentów. Wyglądałoby to mniej więcej tak:

void create_item(struct e_menu *edit_menu, struct e_menu *item,
                 const gchar *stock, GtkAccelGroup *accel_group,
                 guint accel_key, GdkModifierType accel_mods)
{
    item = gtk_image_menu_item_new_from_stock(stock, NULL);
    gtk_widget_add_accelerator(item, "activate", accel_group,
                               accel_key, accel_mods,
                               GTK_ACCEL_VISIBLE);
    gtk_menu_shell_append(GTK_MENU_SHELL(edit_menu->container), item);
}

Wywołanie wyglądałoby mniej więcej tak:

create_item(edit_menu, edit_menu->undo, GTK_STOCK_UNDO, accel_group,
            GDK_z, GDK_CONTROL_MASK);

Ta liczba argumentów jest chyba lekko zbyt duża. To jest rzeczywiście lepsze rozwiązanie od powyższych switchy? Ja zdecydowałem się wcześniej jednak na porzucenie takiego podejścia. Czyli niesłusznie?

2
Shizzer napisał(a):

To jest rzeczywiście lepsze rozwiązanie od powyższych switchy? Ja zdecydowałem się wcześniej jednak na porzucenie takiego podejścia. Czyli niesłusznie?

Każdy for z którego indeks jest użyty potem w switch to totalny bezsens, więc tak nowe rozwiązanie jest dużo lepsze od tego for-switch.
Zwróć uwagę, że jeśli będziesz chciał zmienić jakieś wspólne właściwości elementów menu, to poprawisz tylko tę funkcje i gotowe, nie musisz poprawiać każdego case.

W kolejnym kroku, możesz zastanowić się, czy nie lepiej będzie zamknąć argumenty tej funkcji w jakiejś tablicy i iterować po tej tablicy.
(różnica niewielka, ale ja tak preferuje).

A już idąc zupełnie hiper super pro, możesz próbować wczytywać menu z pliku konfiguracyjnego (np jakiś xml), kótry będzie ładowany zależnie od np ustawień lokalizacji.

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.