インデントのレベルを最小限に抑えるためにこのメソッドをリファクタリングする方法があるのだろうかと思います。クリーンなコードを読んだ後、ボブ叔父さんは、メソッドが2つのレベルを過ぎると、何が起こっているのかを理解することが難しくなります。このメソッドはあまり複雑ではないと思いますが、クリーンアップしてストーリーのように見えると思います。こここれらの各ブロックを入れ子にする必要がありますか?
は、だから私の方法
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
で、イムは、XMLを横断し、対応するメソッドに保存します。各調査には多くの質問がある多数のセクションがあり、多くの質問には多くの選択肢があります。
私はそれぞれのループをメソッドに抽出することができますが、それでも私は4レベルのインデントを持つことができます。
私のような何かをしたい...
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
はこのような何かを行うか、このコードを読みやすくするための任意のアイデアはありますか?それをリファクタリングする必要があるのでしょうか?どんなアイディアも大歓迎です。私はあなたが使用されている既存の
sync_question_group
をどのように定義したかわからないので、私は公平なビットを省略さ
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
:ここ
は完全なクラスは、私の最初の刺し傷のようなものだろう
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
'sync_survey。(xml_document、status_id)'その構文は何ですか? –
これは呼び出しメソッドの略語です。したがって、sync_surveyは、サーベイをfind_または初期化して保存する呼び出しメソッドを持つオブジェクトです。 'sync_survey.call(xml_document、status_id' @EricDuminil – bigelow42
)' sync_survey.sync!(xml_document、status_id) 'などのリファクタリングをする方が良いかもしれないと思います。これらのオブジェクトをどのように作成し、どのようにあなたの例に合うようにするのですか? –