Refactoring kode pemrosesan pembayaran Bizantium dengan anggaran terbatas [ditutup]

Saya telah mengerjakan aplikasi Ruby on Rails besar selama beberapa tahun. Itu diwarisi dalam kondisi yang buruk, tetapi sebagian besar bug produksi telah diperbaiki dengan waktu. Ada beberapa bagian yang belum tersentuh seperti kode pemrosesan pembayaran. Kode ini berfungsi sebagian besar, kecuali bahwa setiap kali tagihan ditolak oleh prosesor pembayaran, pengguna mendapat 500 kesalahan alih-alih pesan yang bermanfaat. Saya ingin memperbaiki kode agar lebih mudah dipelihara. Saya akan memberikan penjelasan singkat tentang cara kerjanya.

Saya telah menghapus semua kode penanganan kesalahan dari cuplikan berikut.

Labirin dimulai dalam sebuah pengontrol:

  def submit_credit_card
    ...
    @credit_card = CreditCard.new(params[:credit_card].merge(:user => @user))
    @credit_card.save
    ...
    @submission.do_initial_charge(@user)
    ...
  end

Kemudian dalam Submissionmodel:

  def do_initial_charge(user)
    ...
    self.initial_charge = self.charges.create(:charge_type => ChargeType.find(1), :user => user)
    self.initial_charge.process!
    self.initial_charge.settled?
  end

Dalam Chargemodel:

  aasm column: 'state' do
    ...
    event :process do
      transitions :from => [:created, :failed], :to => :settled, :guard => :transaction_successful?
    end
    ...
  end

  def initialize(*params)
    super(*params)
    ...
    self.amount = self.charge_type.amount
  end

  def transaction_successful?
    user.reload
    credit_card = CreditCard.where(user_id: user_id).last
    cct = self.cc_transactions.build(:user => user, :credit_card => credit_card, :cc_last_four => credit_card.num_last_four, :amount => amount, :charge_id => id)
    cct.process!
    if self.last_cc_transaction.success
      self.update_attribute(:processed, Time.now)
      return true
    else
      self.fail!
      return false
    end
  end

Ada banyak bit yang dipertanyakan di atas seperti memuat ulang userdan menemukan yang terakhir CreditCarddaripada melewati yang baru saja disimpan. Juga kode ini tergantung pada ChargeTypedimuat dari database dengan ID yang dikodekan keras.

Di CcTransactionkami melanjutkan jejak:

  def do_process
    response = credit_card.process_transaction(self)
    self.authorization = response.authorization
    self.avs_result    = response.avs_result[:message]
    self.cvv_result    = response.cvv_result[:message]
    self.message       = response.message
    self.params        = response.params.inspect
    self.fraud_review  = response.fraud_review?
    self.success       = response.success?
    self.test          = response.test
    self.response      = response.inspect
    self.save!
    self.success
  end

Semua ini tampaknya dilakukan adalah menyimpan catatan di cc_transactionstabel database. Pemrosesan pembayaran aktual dilakukan dalam CreditCardmodel. Saya tidak akan membuat Anda bosan dengan detail kelas itu. Pekerjaan yang sebenarnya dilakukan oleh ActiveMerchant::Billing::AuthorizeNetCimGateway.

Jadi kita memiliki setidaknya 5 model yang terlibat ( Submission, Charge, ChargeType, CcTransaction, dan CreditCard). Jika saya melakukan ini dari awal, saya hanya akan menggunakan Paymentmodel tunggal . Hanya ada 2 jenis biaya, jadi saya akan mengkodekan nilai-nilai tersebut sebagai variabel kelas. Kami tidak menyimpan detail kartu kredit, sehingga model itu tidak perlu. Info transaksi dapat disimpan dalam paymentstabel. Pembayaran gagal tidak perlu disimpan.

Saya bisa masuk dan melakukan refactoring ini dengan cukup mudah kecuali untuk persyaratan bahwa tidak ada yang salah pada server produksi. Setiap kelas redundan memiliki banyak metode yang dapat dipanggil dari mana saja di basis kode. Ada serangkaian tes integrasi tetapi cakupannya tidak 100%.

Bagaimana saya bisa memperbaiki ini sambil memastikan tidak ada yang rusak? Jika saya melewati 5 kelas pembayaran dan grepmengedit setiap metode untuk mencari tahu di mana mereka dipanggil ada kemungkinan besar saya akan melewatkan sesuatu. Klien sudah terbiasa dengan bagaimana kode saat ini berjalan dan memperkenalkan bug baru tidak dapat diterima. Selain meningkatkan cakupan tes menjadi 100%, apakah ada cara untuk memperbaiki ini dengan pasti bahwa tidak ada yang akan rusak?

Pertimbangkan apakah kode ini benar-benar membutuhkan refactoring . Kode ini mungkin tidak menyenangkan dan estetis bagi Anda sebagai pengembang perangkat lunak, tetapi jika berhasil, Anda mungkin tidak boleh mendesain ulangnya. Dan berdasarkan pertanyaan Anda, sepertinya kode sebagian besar berfungsi.

Ini artikel klasik dari Joel on Software menyoroti semua risiko yang tidak perlu menulis ulang kode. Itu kesalahan yang mahal, tapi sangat menggoda. Semuanya layak dibaca sebelumnya, tetapi bacaan tentang kompleksitas yang tampaknya tidak perlu ini tampaknya sangat sesuai:

Kembali ke fungsi dua halaman itu. Ya, saya tahu, itu hanya fungsi sederhana untuk menampilkan jendela, tetapi telah menumbuhkan sedikit rambut dan hal-hal di atasnya dan tidak ada yang tahu mengapa. Baiklah, saya akan memberi tahu Anda alasannya: itu adalah perbaikan bug. Salah satunya memperbaiki bug yang dimiliki Nancy ketika dia mencoba menginstalnya di komputer yang tidak memiliki Internet Explorer. Lain memperbaiki bug yang terjadi dalam kondisi memori rendah. Yang lain memperbaiki bug yang terjadi ketika file berada di floppy disk dan pengguna menarik keluar disk di tengah. Panggilan LoadLibrary itu jelek tapi itu membuat kode bekerja pada versi lama Windows 95.

Masing-masing bug ini memakan waktu berminggu-minggu penggunaan di dunia nyata sebelum ditemukan. Programmer mungkin menghabiskan beberapa hari mereproduksi bug di lab dan memperbaikinya. Jika itu seperti banyak bug, perbaikannya mungkin satu baris kode, atau bahkan mungkin beberapa karakter, tetapi banyak pekerjaan dan waktu masuk ke dua karakter.

Ya, kodenya tidak perlu rumit. Namun tetap saja, beberapa langkah yang tampaknya tidak perlu mungkin ada karena suatu alasan. Dan jika Anda ingin menulis ulang, Anda harus mempelajari semua pelajaran itu lagi.

Reload pengguna, misalnya, tampaknya tidak ada gunanya: itulah sebabnya Anda harus khawatir tentang mengubahnya. Tidak ada pengembang (bahkan yang buruk) yang akan memperkenalkan langkah seperti itu dalam desain awal mereka. Itu hampir pasti dimasukkan ke sana untuk memperbaiki beberapa masalah. Mungkin itu masalah yang refactoring ke desain yang "benar" akan menghilangkan ... tapi mungkin tidak.

Juga, poin kecil lainnya, saya tidak yakin dengan pendapat Anda bahwa tidak perlu menyimpan pembayaran yang gagal. Saya dapat memikirkan dua alasan kuat untuk melakukannya: mencatat bukti kemungkinan penipuan (misalnya seseorang mencoba berbagai nomor kartu), dan dukungan pelanggan (pelanggan mengklaim mereka terus berusaha membayar, dan itu tidak berfungsi ... Anda ingin bukti dari setiap upaya pembayaran untuk membantu memecahkan masalah ini). Ini membuat saya berpikir bahwa mungkin Anda belum sepenuhnya memikirkan persyaratan sistem, dan mereka tidak sesederhana yang Anda yakini.

Perbaiki masalah individu tanpa mengubah desain secara mendasar. Anda menyebutkan satu masalah: ada kesalahan 500 ketika pembayaran gagal. Masalah ini seharusnya mudah diperbaiki, mungkin hanya dengan menambahkan satu atau dua baris di lokasi yang tepat. Itu bukan alasan yang cukup untuk merobohkan segalanya untuk membuat desain yang "benar".

Mungkin ada masalah lain, tetapi jika kode ini bekerja 99% dari waktu sekarang, tidak mungkin bahwa ini akan menjadi masalah mendasar yang membutuhkan penulisan ulang.

Jika desain saat ini tertanam di seluruh kode, maka bahkan menghabiskan cukup banyak upaya untuk memperbaiki masalah tanpa mengubah desain mungkin diperlukan.

Dalam beberapa kasus mungkin memang perlu dilakukan desain ulang yang lebih besar. Tetapi ini akan membutuhkan pemikiran yang lebih kuat yang telah Anda berikan sejauh ini. Misalnya, jika Anda berencana untuk mengembangkan model pembayaran lebih lanjut dan memperkenalkan fitur-fitur baru yang signifikan, membersihkan kode akan memiliki beberapa manfaat. Atau mungkin desain mengandung cacat keamanan mendasar yang perlu diperbaiki.

Ada alasan untuk memperbaiki sejumlah besar kode. Tetapi jika Anda melakukannya, tingkatkan cakupan tes menjadi 100%. Ini mungkin sesuatu yang akan menghemat waktu Anda secara keseluruhan .