Refactoring einer 1500 LOC-Methode, die nur die grafische Benutzeroberfläche erstellt [geschlossen]

8

Ich kratzte mir gerade am Kopf, wie man eine Methode umgestaltet, die im Grunde nur die Benutzeroberfläche erstellt.

Die Methode ist mehr als 1500 Codezeilen (LOC) lang - und zählt. Es ist gewachsen, es gab keinen Plan, wie man das angehen sollte. Sie finden das vielleicht vertraut.

Wie auch immer, es ist im Grunde nur eine große Methode, die ungefähr so ​​aussieht:

    .
    .
    .

    # null checks
    null_checks_bx = Box(True)

    null_checks_ck = CheckBox()
    null_checks_ck.set_text('Null checks throwing exceptions of type:')

    if 'doNullChecks' in options:
        null_checks_ck.set_active(options['doNullChecks'])
    else:
        null_checks_ck.set_active(True)

    # dict to sorted list: extract values from dict by list comprehension
    exceptions = sorted([exception.get_full_name() for exception in JavaTypes.exception_types])

    null_checks_se = Selector()
    null_checks_se.add_items(exceptions)
    null_checks_se.set_enabled(null_checks_ck.get_active())

    if 'nullChecksExceptionFullClassName' in options:
        null_checks_se.set_value(options['nullChecksExceptionFullClassName'])
    else:
        # default to first entry
        #(defaulting to doEquals = True and doHashCode = True by using hardcoded Commons Lang implies availability of Commons Lang NullArgumentException)
        null_checks_se.set_value(JavaTypes.exception_types[0].get_full_name())

    # callback
    Callbacks.sync_model_active_status(null_checks_ck, null_checks_se)

    null_checks_ck.add_clicked_callback(lambda: Callbacks.sync_model_active_status(null_checks_ck, null_checks_se))

    null_checks_bx.add(Label(indent), False, True)  # dummy indent label
    null_checks_bx.add(null_checks_ck, False, True)
    null_checks_bx.add(null_checks_se, False, True)

    # relationship entity class constructor calls
    relationship_constructor_calls_bx = Box(True)
    #relationship_constructor_calls_bx.set_spacing(5)
    #relationship_constructor_calls_bx.set_padding(3)

    relationship_constructor_calls_ck = CheckBox()
    relationship_constructor_calls_ck.set_text('Instantiate relationship entities (if all necessary columns present)')

    if 'doRelationshipConCalls' in options:
        relationship_constructor_calls_ck.set_active(options['doRelationshipConCalls'])
    else:
        relationship_constructor_calls_ck.set_active(False)

    relationship_constructor_calls_bx.add(Label(indent), False, True)  # dummy indent label
    relationship_constructor_calls_bx.add(relationship_constructor_calls_ck, False, True)

    # relationship not-null checks: this is an option independent of the null checks!
    relationship_not_null_checks = Box(True)
    #relationship_not_null_checks.set_spacing(5)
    #relationship_not_null_checks.set_padding(3)

    relationship_not_null_checks_ck = CheckBox()
    relationship_not_null_checks_ck.set_text('Not-null checks before relationship instantiation')

    relationship_not_null_checks_ck.set_enabled(relationship_constructor_calls_ck.get_active() and not null_checks_ck.get_active())

    if 'doRelationshipNotNullChecks' in options:
        relationship_not_null_checks_ck.set_active(options['doRelationshipNotNullChecks'])
    else:
        relationship_not_null_checks_ck.set_active(True)

    # callback
    Callbacks.sync_convenience_constructor_options(null_checks_ck, relationship_constructor_calls_ck, relationship_not_null_checks_ck)

    null_checks_ck.add_clicked_callback(lambda: Callbacks.sync_convenience_constructor_options(null_checks_ck, relationship_constructor_calls_ck, relationship_not_null_checks_ck))
    relationship_constructor_calls_ck.add_clicked_callback(lambda: Callbacks.sync_convenience_constructor_options(null_checks_ck, relationship_constructor_calls_ck, relationship_not_null_checks_ck))

    relationship_not_null_checks.add(Label(indent), False, True)  # dummy indent label
    relationship_not_null_checks.add(Label(indent), False, True)  # dummy indent label
    relationship_not_null_checks.add(relationship_not_null_checks_ck, False, True)

    # build final box
    checks_bx = Box(False)
    checks_bx.set_spacing(5)
    #checks_bx.set_padding(3)
    checks_bx.set_homogeneous(True)
    checks_bx.add(omit_auto_increment_columns_bx, False, True)
    checks_bx.add(omit_auto_timestamp_columns_bx, False, True)
    checks_bx.add(null_checks_bx, False, True)
    checks_bx.add(relationship_constructor_calls_bx, False, True)
    checks_bx.add(relationship_not_null_checks, False, True)

    # callback
    Callbacks.sync_model_active_status(convenience_constructors_ck, checks_bx)

    convenience_constructors_ck.add_clicked_callback(lambda: Callbacks.sync_model_active_status(convenience_constructors_ck, checks_bx))

    # need wrapped box for Panel class
    constructors_bx = Box(False)
    constructors_bx.set_spacing(5)
    constructors_bx.set_padding(3)
    #constructors_bx.set_homogeneous(True)
    constructors_bx.add(convenience_constructors_bx, False, True)
    constructors_bx.add(checks_bx, False, True)

    constructors_pn = Panel(TitledGroupPanel)
    constructors_pn.set_title('Constructors')
    constructors_pn.add(constructors_bx)

    # getters and setters
    is_conversion_bx = Box(True)
    #is_conversion_bx.set_spacing(3)
    #is_conversion_bx.set_padding(3)

    is_conversion_ck = CheckBox()
    is_conversion_ck.set_text('Convert respective column names starting with "is_"')
    is_conversion_ck.set_tooltip("Column is_incognito => getter isIncognito() (not getIsIncognito()) and setter setIncognito()")

    if 'doIsConversion' in options:
        is_conversion_ck.set_active(options['doIsConversion'])
    else:
        is_conversion_ck.set_active(True)

    is_conversion_bx.add(is_conversion_ck, False, True)

    # generate code for relationship getters and setters to synchronize with column fields
    sync_relationship_pk_fields_bx = Box(True)
    #sync_relationship_pk_fields_bx.set_spacing(3)
    #sync_relationship_pk_fields_bx.set_padding(3)

    sync_relationship_pk_fields = CheckBox()

    .
    .
    .

Du verstehst, worum es geht.

Was ich nicht sehe, ist, nach welchen Kriterien diese Methode aufgeteilt werden könnte, damit der Code übersichtlicher, verständlicher usw. wird. Pp.

Was sind hier die Best Practices?

Ich könnte einige Methoden erstellen, die wiederkehrende Arten von Panels und Widgets erstellen, aber dies würde eine ziemlich mühsame Anstrengung für das werden, was ich nicht sehe, um den wirklichen Gewinn zu sehen.

Was sind wieder einige nützliche, praktische Ansätze für große, ungeschickte Methoden, die nur die grafische Benutzeroberfläche erstellen?

Vielen Dank

PS: Übrigens, die Benutzeroberfläche ist ein einzelnes Dialogfeld in einer Standardanwendung mit Fenstern (kein Web). Dieses Dialogfeld verfügt über 6 Registerkarten, die hauptsächlich Kontrollkästchen, Optionsfelder und Textfelder enthalten. Es ist ein Plugin zur Codegenerierung, aus dem der Benutzer viele Optionen auswählen kann. Ich könnte ein Bild posten, sobald das Plugin wieder läuft ...

Kawu
quelle
Eine Sache, die ich ändern würde, ist das Trennen des statischen Setups vom dynamischen Zustand.
CodesInChaos
@ Kawu Sie können die Antwort auch akzeptieren, wenn sie Ihr Problem löst und die Frage umfassend beantwortet.
Toniedzwiedz
Ja natürlich. Ich bin kein SO Anfänger. Es ist nur schwer, die akzeptierte Antwort zu übertreffen.
Kawu

Antworten:

27

Ihr Code scheint eine Funktion zu haben, die Sie hier zu Ihrem Vorteil nutzen können: alle diese Gruppierungskommentare. Sie können diese Kommentare als Grundlage für die Aufteilung der Methode in Blöcke verwenden, indem Sie kleinere Methoden extrahieren . Sie ziehen jeden Block in eine eigene Methode und rufen diese Methode dann vom ursprünglichen Speicherort des Codes aus auf.

Wenn Sie anfangen, diese Teile herauszuziehen, werden Sie entweder auf natürliche Weise Duplikate finden oder Sie werden auf natürliche Weise starke Beziehungen zwischen einigen der Methoden finden. Wenn Sie Duplikate finden, extrahieren Sie diese in Hilfsmethoden, um Ihren Code auszutrocknen. Wenn Sie natürliche Beziehungsgrenzen finden, können Sie diese in separate Klassen eng verwandter Funktionen aufteilen.

Im Folgenden habe ich beispielsweise begonnen, die Logik zum Erstellen des Nullprüfungsabschnitts herauszuarbeiten. Ich habe es als Methode extrahiert, einige Teile herausgebrochen, die einen besseren Namen benötigten, und alle Variablen innerhalb der Methode de-namespaced. Es kann noch viel mehr getan werden, aber das bleibt dem Leser als Übung.

def add_indent_label(self, box):
    box.add(Label(indent), False, True)

def get_null_checks(self, options):
    box = Box(True)

    checkbox = CheckBox()
    checkbox.set_text('Null checks throwing exceptions of type:')

    if 'doNullChecks' in options:
        checkbox.set_active(options['doNullChecks'])
    else:
        checkbox.set_active(True)

    exceptions = self.get_exceptions()

    selector = Selector()
    selector.add_items(exceptions)
    selector.set_enabled(checkbox.get_active())

    if 'nullChecksExceptionFullClassName' in options:
        selector.set_value(options['nullChecksExceptionFullClassName'])
    else:
        selector.set_value(self.get_first_exception())

    # callback
    Callbacks.sync_model_active_status(checkbox, selector)
    checkbox.add_clicked_callback(lambda: Callbacks.sync_model_active_status(checkbox, selector))

    self.add_indent_label(box);
    box.add(checkbox, False, True)
    box.add(selector, False, True)

    return box

def get_exceptions(self):
    return sorted([exception.get_full_name() for exception in JavaTypes.exception_types])

# This could probably have a better name
def get_first_exception(self):
    #(defaulting to doEquals = True and doHashCode = True by using hardcoded Commons Lang implies availability of Commons Lang NullArgumentException)
    return JavaTypes.exception_types[0].get_full_name()

Und in Ihrem ursprünglichen Code wird es:

null_checks_bx = self.get_null_checks(options)
cbojar
quelle