2009-07-20 38 views
5

Chúng tôi đang tái cấu trúc một phương pháp dài; nó chứa một vòng lặp dài for với nhiều câu lệnh continue. Tôi chỉ muốn sử dụng phương pháp trích xuất Phương pháp trích xuất, nhưng cách tự động hóa của Eclipse không biết cách xử lý phân nhánh có điều kiện. Tôi cũng vậy.Phương pháp trích xuất với tiếp tục

chiến lược hiện tại của chúng tôi là giới thiệu một keepGoing cờ (một biến Ví dụ vì chúng ta sẽ muốn phương pháp chiết xuất), đặt nó là sai ở đầu vòng lặp, và thay thế tất cả tiếp tục thiết lập các cờ thành true, sau đó gói tất cả các nội dung sau (ở các mức lồng nhau khác nhau) trong mệnh đề if (keepGoing). Sau đó thực hiện các trích xuất khác nhau, sau đó thay thế các nhiệm vụ keepGoing với lợi nhuận ban đầu từ các phương thức được trích xuất, sau đó loại bỏ cờ.

Có cách nào tốt hơn không?

Cập nhật: Đáp lại bình luận - Tôi không thể chia sẻ mã, nhưng đây là một đoạn trích nặc danh:

private static void foo(C1 a, C2 b, C3 c, List<C2> list, boolean flag1) throws Exception { 
    for (int i = 0; i < 1; i++) { 
     C4 d = null; 
     Integer e = null; 
     boolean flag2 = false; 
     boolean flag3 = findFlag3(a, c); 
     blahblahblah(); 
     if (e == null) { 
      if (flag1) { 
       if (test1(c)) { 
        if (test2(a, c)) { 
         Integer f = getF1(b, c); 
         if (f != null) 
          e = getE1(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
        } else { 
         Integer f = getF2(b, c); 
         if (f != null) 
          e = getE2(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
         flag2 = true; 
        } 
       } else { 
        if (test3(a, c)) { 
         Integer f = getF2(b, c); 
         if (f != null) 
          e = getE2(a, f); 
         if (e == null) { 
          if (d == null) { 
           list.add(b); 
           continue; 
          } 
          e = findE(d); 
         } 
         flag2 = true; 
        } else { 
         if (d == null) { 
          list.add(b); 
          continue; 
         } 
         e = findE(d); 
         flag2 = true; 
        } 
       } 
      } 
      if (!flag1) { 
       if (d == null) { 
        list.add(b); 
        continue; 
       } 
       e = findE(d); 
      } 
     } 
     if (e == null) { 
      list.add(b); 
      continue; 
     } 
     List<C2> list2 = blahblahblah(b, list, flag1); 
     if (list2.size() != 0 && flag1) { 
      blahblahblah(); 
      if (!otherTest()) { 
       if (yetAnotherTest()) { 
        list.add(b); 
        continue; 
       } 
       blahblahblah(); 
      } 
     } 
    } 
} 
+0

có thể đăng mã không? –

+0

bạn có thể cung cấp ví dụ viết tắt không? – akf

+3

Wow ... Tôi chắc chắn có thể thấy lý do tại sao bạn muốn tái cấu trúc nó. –

Trả lời

8

Đây là một trong những điều thú vị mà không có mẫu đơn nào sẽ đưa bạn đến đó.

Tôi sẽ làm việc theo cách lặp lại.

Trước tiên, tôi sẽ cố gắng xem liệu tôi có thể sử dụng sớm để tiếp tục xóa một trong các cấp if đó hay không. Đó là mã rõ ràng hơn nhiều để kiểm tra một điều kiện và trở lại sớm (hoặc trong trường hợp của bạn tiếp tục) hơn là có lồng nhau ifs sâu.

Tiếp theo, tôi nghĩ mình sẽ lấy một số phần bên trong và xem chúng có thể được tách ra thành một phương pháp riêng biệt hay không. Có vẻ như hai khối lớn đầu tiên (trong "if (test2 (a, c)) {" và câu lệnh khác của nó) rất giống nhau. Có logic cắt và dán phải giống nhau.

Cuối cùng sau khi nội dung đó được xóa, bạn có thể bắt đầu xem xét vấn đề thực tế của mình - bạn cần thêm lớp học. Toàn bộ câu lệnh này có lẽ là một phương pháp đa hình ba dòng trong 3-5 lớp anh chị em.

Rất gần với việc vứt bỏ và viết lại mã, khi bạn xác định các lớp thực tế của mình, toàn bộ phương pháp này sẽ biến mất và được thay thế bằng một cái gì đó rất đơn giản. Chỉ cần thực tế rằng nó là một phương pháp tiện ích tĩnh nên nói với bạn điều gì đó - bạn không muốn một trong những người trong loại mã này.

Chỉnh sửa (Sau khi tìm kiếm thêm một chút): Có quá nhiều thứ ở đây, thật thú vị khi trải qua. Hãy nhớ rằng khi bạn làm xong bạn không muốn sao chép mã - và tôi khá chắc chắn rằng toàn bộ điều này có thể được viết mà không có một đơn nếu tôi nghĩ tất cả các trường hợp của bạn là trường hợp có thể/dễ dàng được xử lý bởi đa hình.

Ồ, và như một câu trả lời cho câu hỏi của bạn về nhật thực không muốn làm điều đó - thậm chí không TRY refactoring tự động với điều này, chỉ cần làm điều đó bằng tay. Những thứ bên trong đầu tiên nếu() cần phải được kéo ra thành một phương thức vì nó hầu như giống với mệnh đề trong mệnh đề khác của nó()!

Khi tôi làm một cái gì đó như thế này, tôi thường tạo một phương pháp mới, di chuyển mã từ nếu vào phương thức mới (chỉ để gọi một phương thức mới bên trong if), sau đó chạy kiểm tra và đảm bảo bạn không phá vỡ bất cứ điều gì.

sau đó đi từng dòng và kiểm tra để đảm bảo không có sự khác biệt giữa mã if và mã khác. Nếu có, bù đắp cho nó bằng cách chuyển sự khác biệt dưới dạng biến mới cho phương thức. Sau khi bạn chắc chắn mọi thứ đều giống nhau, hãy thay thế mệnh đề khác bằng một cuộc gọi. Kiểm tra lần nữa. Rất có thể là vào thời điểm này một vài tối ưu hóa bổ sung sẽ trở nên rõ ràng, bạn sẽ rất có thể mất toàn bộ nếu bằng cách kết hợp logic của nó với biến mà bạn truyền để phân biệt hai cuộc gọi.

Chỉ tiếp tục thực hiện những việc như thế và lặp lại. Bí quyết với tái cấu trúc là sử dụng các bước rất nhỏ và kiểm tra giữa từng bước để đảm bảo không có gì thay đổi.

+2

Vâng, đây là loại mã khó cho những người ngẫu nhiên trên Internet làm bất cứ điều gì. Nó thực sự đòi hỏi nhiều kiến ​​thức về vấn đề, và có nhiều thời gian và kiên nhẫn hơn tôi sẵn sàng đưa ra. –

+0

Có kiểm tra tốt tại chỗ là phải trước khi chạm vào mã như thế! Sau đó, bạn có thể làm việc với nó từng bước, và hoàn tác bước cuối cùng của bạn nếu nó phá vỡ các bài kiểm tra. –

+0

Tôi phải đồng ý, nếu tôi hiểu chính xác mã hiện tại về cơ bản là phương pháp DoItAll với vô số đối số. Có lẽ bạn nên làm một bước trở lại mục tiêu mà phương thức này thực hiện. Xác định đầu vào có thể và đầu ra dự kiến ​​(có thể viết một thử nghiệm trên nó nếu nó phức tạp). Sau đó, quyết định xem một, hai hoặc nhiều phương thức được yêu cầu và nếu tất cả các chức năng cần được chứa trong lớp cụ thể này. – Zyphrax

4

continue về cơ bản là một chất tương tự của một sự trở lại sớm, đúng không?

for (...) { 
    doSomething(...); 
} 

private void doSomething(...) { 
    ... 
    if (...) 
     return; // was "continue;" 
    ... 
    if (!doSomethingElse(...)) 
     return; 
    ... 
} 

private boolean doSomethingElse(...) { 
    ... 
    if (...) 
     return false; // was a continue from a nested operation 
    ... 
    return true; 
} 

Bây giờ tôi phải thừa nhận rằng tôi không thực hiện theo chiến lược hiện tại của bạn, vì vậy tôi có thể lặp lại những gì bạn đã nói. Nếu vậy, thì câu trả lời của tôi là tôi không thể nghĩ ra một cách tốt hơn.

+0

Tôi nghĩ rằng việc sử dụng các câu lệnh trả về trong các phương thức trích xuất sẽ hoạt động giống như cách sử dụng tiếp tục trong vòng lặp chính. Gần đây tôi phải đối mặt với cùng một vấn đề trong một dự án của tôi, nơi tôi đã tái cấu trúc mã. Tôi có thể đảm bảo với bạn rằng nó không phức tạp như thế này. –

2

Nếu tôi gặp phải tình huống của bạn, tôi sẽ xem xét sử dụng các kỹ thuật tái cấu trúc khác như "thay thế điều kiện có đa hình". Điều đó nói rằng bạn nên luôn luôn làm một việc tại một thời điểm, vì vậy nếu bạn lần đầu tiên muốn trích xuất phương pháp bạn có hai lựa chọn:

  1. Thêm "keepGoing" cờ
  2. Ném một ngoại lệ từ phương pháp này

Trong hai tùy chọn này, tôi nghĩ cờ keepGoing là tốt hơn. Tôi sẽ không ngừng tái cấu trúc sau khi bạn giải nén phương thức. Tôi chắc chắn một khi bạn có một phương pháp nhỏ hơn, bạn sẽ tìm thấy một cách để loại bỏ lá cờ này và có logic sạch hơn.

0

Tôi sẽ tóm tắt các câu trả lời ở đây, trong khi chấp nhận câu trả lời của Bill K là câu trả lời đầy đủ nhất. Nhưng mọi người đều có thứ gì đó tốt để cung cấp, và tôi có thể sử dụng bất kỳ phương pháp nào trong số những cách tiếp cận này vào lần tới khi tôi gặp phải tình huống này.


mmyers: Cắt thân vòng lặp, dán nó vào một phương pháp mới và thay thế tất cả các continue s với return s. Điều này làm việc rất độc đáo, mặc dù nó sẽ gặp rắc rối nếu có các câu lệnh dòng điều khiển khác, như phá vỡ và trả về, bên trong vòng lặp.


Bill K: Tease nó ngoài lặp đi lặp lại; tìm kiếm trùng lặp và loại bỏ nó. Tận dụng các lớp đa hình để thay thế hành vi có điều kiện. Sử dụng các bước rất nhỏ. Có; đây là tất cả lời khuyên tốt, với khả năng ứng dụng rộng hơn so với trường hợp cụ thể này.


Aaron: Hoặc sử dụng keepGoing cờ để thay thế continue hoặc ném một ngoại lệ. Tôi đã không thử điều này, nhưng tôi nghĩ rằng tùy chọn ngoại lệ là một lựa chọn rất tốt đẹp, và một tôi đã không xem xét.

Các vấn đề liên quan