The refactor goal was to make clear the logic for getting an invoice. - THIS IS THE MOST IMPORTANT POINT: Original code tells me there are 2 kinds of invoices so I knew I wanted a "if" that stated very simply and high level the logic for deciding which. So I wrote that and worked all the rest of the code around it. - Moving code into the `get_xxx` methods I saw that fetching the existing invoice was in both methods. - The above had me realize that no matter what, we use an pre-existing invoice if it's there. So I pulled that line out and put it at the top. Now it is obvious what's really going on. - I renamed `invoice_obj` because that sounds like it's an invoice too. But it's not. Its properties are used for fetching an actual invoice. So I thought of `invoice_obj` as `metadata` of a real invoice. - Make both "if" branches the same "level of abstraction". I'm trying to express that there are 2 kinds of invoices. I'm hiding the detail code that actually does it - that's a different level of detail. I want it to say this: if ... get_initial_invoice(metadata) else get_invoice(metadata) and not this: if ... get_initial_invoice(metadata) else Invoice.find_by(metadata[:id]) ------------------- **Refactored Code** return existing_invoice if existing_invoice.present? invoice = nil if initial_invoice?(invoice_metadata) invoice = get_initial_invoice(invoice_metadata) else invoice = get_invoice(invoice_metadata) end return invoice private def get_initial_invoice(invoice_metadata) ac = Account.find_by(stripe_customer_id: invoice_metadata[:customer]) create_initial_invoice(invoice_metadata, ac) end def get_invoice(invoice_metadata) return Invoice.find_by(invoice_metadata[:id]) end def initial_invoice?(invoice_metadata) return false if invoice_metadata[:subscription].nil? sub = Stripe::Subscription.retrieve(invoice_metadata[:subscription]) invoice_metadata[:period_start] == sub[:created] ? true : false end