Sollte ich diese verschachtelten Blöcke umgestalten?

Question

Ich frage mich, ob es eine Möglichkeit gibt, diese Methode umzuformatieren, um die Ebenen der Einrückung zu minimieren. Nach dem Lesen von sauberem Code sagt Onkel Bob, sobald eine Methode über zwei Ebenen hinausgeht, wird es schwieriger zu verstehen, was vor sich geht. Ich denke nicht, dass diese Methode übermäßig komplex ist, aber ich denke, sie könnte aufgeräumt werden und eher wie eine Geschichte aussehen.

hier ist meine Methode

def sync!(xml_document, language_id, status_id, file) 
    survey = sync_survey.(xml_document, status_id) 
    sync_survey_label.(xml_document, survey.id, language_id, file) 

    xml_document.xpath('//page').each do |xml_section| 
    section = sync_section.(xml_section, survey.id) 
    sync_section_label.(xml_section, section.id, language_id) 

    xml_section.xpath('.//question_group').each do |xml_question_group| 
     question_group = sync_question_group.(xml_question_group, section.id) 
     sync_question_group_label.(xml_question_group, question_group.id, language_id) 

     xml_question_group.xpath('.//question').each do |xml_question| 
     question = sync_question.(xml_question, question_group.id) 
     sync_question_label.(xml_question, question.id, language_id) 

     xml_question.xpath('.//answerchoice').each do |xml_choice| 
      choice = sync_choice.(xml_choice, question.id) 
      sync_choice_label.(xml_choice, choice.id, language_id) 
     end 
     end 
    end 
    end 

    survey 
end 

So, Im eine XML durchqueren und auf die entsprechenden Methoden zu speichern. Jede Umfrage hat viele Abschnitte, die viele Fragengruppen hat, die viele Fragen hat, die viele Möglichkeiten hat.

Ich weiß, ich könnte jede Schleife in eine Methode extrahieren, aber dann hätte ich noch 4 Ebenen der Einrückung.

ich so etwas tun wollen ...

def sync!(xml_document, language_id, status_id, file) 
    survey = sync_survey.(xml_document, status_id) 
    sync_survey_label.(xml_document, survey.id, language_id, file) 
    sync_sections.(xml_document) 
    sync_section_labels.(xml_document, language_id) 
    sync_question_groups.(xml_document) 
    sync_question_group_labels.(xml_document, language_id) 
    sync_questions.(xml_document) 
    sync_question_labels.(xml_document, language_id) 
    sync_choices.(xml_document) 
    sync_choice_labels.(xml_document, language_id) 

    survey 
end 

Gibt es irgendwelche Ideen so etwas wie dieses oder machen diesen Code besser lesbar zu machen? Ist es sogar notwendig, es zu refaktorisieren? Irgendwelche Ideen sind willkommen.

Hier ist die vollständige Klasse

class SurveyBuilder 
    attr_reader :xml_parser, :sync_survey, :sync_survey_label, :sync_section, :sync_section_label, :sync_question, :sync_question_label, :sync_question_group, :sync_question_group_label, :sync_choice, :sync_choice_label 

    def initialize 
    @sync_survey = SurveyBuilder::Sync::Survey.new 
    @sync_survey_label = SurveyBuilder::Sync::SurveyLabel.new 
    @sync_section = SurveyBuilder::Sync::Section.new 
    @sync_section_label = SurveyBuilder::Sync::SectionLabel.new 
    @sync_question = SurveyBuilder::Sync::Question.new 
    @sync_question_label = SurveyBuilder::Sync::QuestionLabel.new 
    @sync_question_group = SurveyBuilder::Sync::QuestionGroup.new 
    @sync_question_group_label = SurveyBuilder::Sync::QuestionGroupLabel.new 
    @sync_choice = SurveyBuilder::Sync::Choice.new 
    @sync_choice_label = SurveyBuilder::Sync::ChoiceLabel.new 
    @xml_parser = SurveyBuilder::XMLParser.new 
    end 

    def call(file, status_id) 
    xml_document = xml_parser.(file) 
    language_id = Language.find_by(code: xml_document.xpath('//lang_code').children.first.to_s).id 

    survey = sync!(xml_document, language_id, status_id, file) 

    survey 
    end 

private 
    def sync!(xml_document, language_id, status_id, file) 
    survey = sync_survey.(xml_document, status_id) 
    sync_survey_label.(xml_document, survey.id, language_id, file) 

    xml_document.xpath('//page').each do |xml_section| 
     section = sync_section.(xml_section, survey.id) 
     sync_section_label.(xml_section, section.id, language_id) 

     xml_section.xpath('.//question_group').each do |xml_question_group| 
     question_group = sync_question_group.(xml_question_group, section.id) 
     sync_question_group_label.(xml_question_group, question_group.id, language_id) 

     xml_question_group.xpath('.//question').each do |xml_question| 
      question = sync_question.(xml_question, question_group.id) 
      sync_question_label.(xml_question, question.id, language_id) 

      xml_question.xpath('.//answerchoice').each do |xml_choice| 
      choice = sync_choice.(xml_choice, question.id) 
      sync_choice_label.(xml_choice, choice.id, language_id) 
      end 
     end 
     end 
    end 
    survey 
    end 

end 

Answer 1

Mein erster Stich wäre so etwas wie:

def sync!(xml_document, language_id, status_id, file) 
    survey = sync_survey.(xml_document, status_id) 
    sync_survey_label.(xml_document, survey.id, language_id, file) 

    xml_document.xpath('//page').each do |xml_section| 
    sync_section(xml_section, ...) 
    end 

    survey 
end 

def sync_section(xml_section, ...) 
    ... 
    xml_section.xpath('.//question_group').each do |xml_question_group| 
    sync_question_group(xml_question_group, ...) 
    end 
end 

def sync_question_group(xml_question_group, ...) 
    ... 
    xml_question_group.xpath('.//question').each do |xml_question| 
    sync_question(xml_question, ...) 
    end 
end 

def sync_question(xml_question, ...) 
    ... 
    xml_question.xpath('.//answerchoice').each do |xml_choice| 
    sync_choice(xml_choice, ...) 
    end 
end 

def sync_choice(xml_choice, ...) 
    ... 
end 

ich ein gutes Stück elided da ich nicht sicher bin, wie Sie die vorhandenen sync_question_group definiert, die verwendet wird, auf verwirrende Weise mit der Punkt-Syntax.