Destruktor i Try .. Catch
Ostatnio zmodyfikowano 2026-04-06 23:21
tBane Temat założony przez niniejszego użytkownika |
Destruktor i Try .. Catch » 2026-04-04 18:59:42 Cześć. Chcę napisać dobrze program tak jak zalecił mi jeden z użytkowników tego forum pekfos i postanowiłem dodać try .. catch do destruktora. Czy dobrze jest napisany poniższy desktruktor? Struktura klasyclass Dialog_Brightness_Contrast : public Dialog { public: BrightnessContrastState _state; std::shared_ptr < SliderWithButtons > _brightness_slider; std::shared_ptr < SliderWithButtons > _contrast_slider; std::shared_ptr < ColoredButtonWithText > _reset; std::shared_ptr < ColoredButtonWithText > _confirm; std::vector < std::shared_ptr < Layer >> _original_layers; std::vector < std::shared_ptr < Layer >> _edited_layers; Dialog_Brightness_Contrast( std::vector < std::shared_ptr < Layer >> layers ); ~Dialog_Brightness_Contrast(); void saveOriginalLayers( std::vector < std::shared_ptr < Layer >> layers ); void setPosition( sf::Vector2i position ); void setTheFilter(); void cursorHover(); void handleEvent( const sf::Event & event ); void update(); void draw(); };
DestruktorDialog_Brightness_Contrast::~Dialog_Brightness_Contrast() { try { if( Dialog_Brightness_Contrast::_state == BrightnessContrastState::Idle ) { _brightness_slider->setValue( 0 ); _contrast_slider->setValue( 0 ); getCurrentAnimation()->getCurrentFrame()->_layers.clear(); getCurrentAnimation()->getCurrentFrame()->_layers = _edited_layers; layers_panel->loadLayersFromCurrentFrame(); } else { if( selection->_state == SelectionState::Selected ) { sf::Image original_image = getCurrentAnimation()->getCurrentLayer()->_image; pasteImageWithMask( getCurrentAnimation()->getCurrentLayer()->_image, * selection->_resizedImage, selection->_resizedRect.position.x, selection->_resizedRect.position.y, * selection->_resizedMaskImage,( toolbar->_option_transparency->_checkbox->_value == 0 ) ? sf::Color::Transparent : toolbar->_second_color->_color ); history->saveStep(); canvas->_isEdited = true; selection->normalize( selection->_resizedRect ); getCurrentAnimation()->getCurrentLayer()->_image = original_image; } else { history->saveStep(); } } } catch( const std::exception & e ) { std::cerr << "Exception in Dialog_Brightness_Contrast destructor: " << e.what() << std::endl; } catch( ... ) { std::cerr << "Unknown exception in Dialog_Brightness_Contrast destructor" << std::endl; } }
|
|
pekfos |
» 2026-04-06 20:55:17 Ignorujesz wyjątki więc jest ok, ale powinieneś się zastanowić dlaczego w ogóle destruktor alokuje zasoby. To raczej podejrzana decyzja projektowa. |
|
tBane Temat założony przez niniejszego użytkownika |
» 2026-04-06 21:28:00 Wygodniej jest tak napisać niż tworzyć kolejna funkcje np. onClose() i dopiero po niej kasować obiekt. |
|
pekfos |
» 2026-04-06 21:53:20 Wygodniej - może. Pamiętaj że nie możesz sensownie zwrócić błędu z destruktora. Nie ma wartości zwracanej, pole w klasie przestaje istnieć, a wyjątku nie możesz bezpiecznie rzucić. W sytuacji w której jednak chcesz coś zrobić w przypadku błędu, metoda onClose ma więcej sensu.
Co do podejrzanej decyzji projektowej, czemu efekt aplikuje się przy zamykaniu okna dialogowego? Czemu nie przyciski OK/cancel? Zamknięcie okna intuicyjnie anuluje operację, a nie ją wykonuje. |
|
tBane Temat założony przez niniejszego użytkownika |
» 2026-04-06 21:59:12 Ponieważ zapisuje stan aplikacji i pracuje na oryginalnych warstwach dzięki czemu mam podgląd efektu. Gdy okno jest zamykane to przywraca warstwy czyli cofam efekt. |
|
pekfos |
» 2026-04-06 22:19:13 Głównym problemem tutaj może być brak pamięci gdy pracujesz na obrazach o dużej rozdzielczości (zakładam że główne użycie pamięci bierze się z sf::Image i reszta ma pomijalny wpływ). Jeżeli operacja się nie uda z tego powodu, to są spore szanse że nie uda się także odwołanie operacji, jeżeli też wymaga podobnych zasobów. Zawsze łatwiej zagwarantować poprawny stan po błędzie jeżeli pracujesz na kopii, bo wystarczy odrzucić kopię. |
|
tBane Temat założony przez niniejszego użytkownika |
» 2026-04-06 22:33:48 if( Dialog_Brightness_Contrast::_state == BrightnessContrastState::Idle ) { _brightness_slider->setValue( 0 ); _contrast_slider->setValue( 0 ); getCurrentAnimation()->getCurrentFrame()->_layers.clear(); getCurrentAnimation()->getCurrentFrame()->_layers = _edited_layers; layers_panel->loadLayersFromCurrentFrame(); }
Faktycznie powinno być _orginal_layers :-/ Jutro to poprawię. Ale najlepsze jest to, że działa poprawnie ten kod :D |
|
pekfos |
» 2026-04-06 22:44:44 Za słabo testujesz skoro błędny kod działa poprawnie. |
|
| « 1 » 2 |