it-swarm-vi.com

Những thứ ngay lập tức rung chuông báo động khi nhìn vào mã?

Tôi đã tham dự một sự kiện thủ công phần mềm vài tuần trước và một trong những ý kiến ​​được đưa ra là "Tôi chắc chắn tất cả chúng ta đều nhận ra mã xấu khi nhìn thấy nó" và mọi người gật đầu lúng túng mà không cần thảo luận thêm.

Điều này luôn làm tôi lo lắng vì có sự thật rằng mọi người đều nghĩ rằng họ là một tài xế trên trung bình. Mặc dù tôi nghĩ rằng tôi có thể nhận ra mã xấu, tôi thích tìm hiểu thêm về những gì người khác cho là mã có mùi vì nó hiếm khi được thảo luận chi tiết trên blog của mọi người và chỉ trong một số ít sách. Đặc biệt tôi nghĩ sẽ rất thú vị khi nghe về bất cứ thứ gì có mùi mã trong một ngôn ngữ chứ không phải ngôn ngữ khác.

Tôi sẽ bắt đầu với một điều dễ dàng:

Mã trong kiểm soát nguồn có tỷ lệ mã nhận xét cao - tại sao nó lại ở đó? nó có nghĩa là bị xóa? nó là một nửa hoàn thành công việc? có lẽ nó không nên được bình luận và chỉ được thực hiện khi ai đó đang thử nghiệm cái gì đó? Cá nhân tôi thấy loại điều này thực sự gây phiền nhiễu ngay cả khi nó chỉ là dòng lẻ ở đây và đó, nhưng khi bạn thấy các khối lớn xen kẽ với phần còn lại của mã thì hoàn toàn không thể chấp nhận được. Nó cũng thường là một dấu hiệu cho thấy phần còn lại của mã có khả năng cũng có chất lượng đáng ngờ.

98
FinnNk
/* Fuck this error */

Thường được tìm thấy bên trong một điều vô nghĩa try..catch chặn, nó có xu hướng thu hút sự chú ý của tôi. Cũng gần như /* Not sure what this does, but removing it breaks the build */.

Một vài điều nữa:

  • Nhiều câu lệnh if lồng nhau
  • Các khối thử bắt được sử dụng để xác định luồng logic một cách thường xuyên
  • Các hàm có tên chung process, data, change, rework, modify
  • Sáu hoặc bảy kiểu giằng khác nhau trong 100 dòng

Một cái tôi vừa tìm thấy:

/* Stupid database */
$conn = null;
while(!$conn) {
    $conn = mysql_connect("localhost", "root", "[pass removed]");
}
/* Finally! */
echo("Connected successfully.");

Đúng vậy, bởi vì phải vũ phu buộc các kết nối MySQL của bạn là cách làm đúng đắn. Hóa ra cơ sở dữ liệu đã có vấn đề với số lượng kết nối để họ liên tục hết thời gian. Thay vì gỡ lỗi này, họ chỉ cần cố gắng lặp đi lặp lại cho đến khi nó hoạt động.

128
Josh K

Cờ đỏ chính đối với tôi là các khối mã trùng lặp, bởi vì nó cho thấy rằng người đó không hiểu các nguyên tắc lập trình cơ bản hoặc quá sợ hãi để thực hiện các thay đổi phù hợp cho cơ sở mã hiện có.

Tôi cũng từng coi việc thiếu bình luận là một lá cờ đỏ, nhưng gần đây đã làm việc với rất nhiều mã rất tốt mà không có bình luận nào tôi đã giảm bớt về điều đó.

104
Ben Hughes

Mã cố gắng thể hiện mức độ thông minh của lập trình viên, mặc dù thực tế là nó không có giá trị thực sự:

x ^= y ^= x ^= y;
74
Rei Miyasaka
  • 20.000 (cường điệu) chức năng dòng. Bất kỳ chức năng nào có nhiều hơn một vài màn hình đều cần bao thanh toán lại.

  • Dọc theo cùng một dòng, các tập tin lớp dường như đi mãi mãi. Có lẽ có một vài khái niệm có thể được trừu tượng hóa thành các lớp sẽ làm rõ mục đích và chức năng của lớp gốc và có lẽ nó đang được sử dụng, trừ khi chúng là tất cả các phương thức bên trong.

  • các biến không mô tả, không tầm thường hoặc quá nhiều biến không mô tả tầm thường. Những điều này làm cho suy luận những gì đang thực sự xảy ra một câu đố.

62
Dominique McDonnell
{ Let it Leak, people have good enough computers to cope these days }

Điều tồi tệ hơn là từ một thư viện thương mại!

61
Reallyethical

Nhận xét dài dòng đến mức nếu có trình biên dịch tiếng Anh, nó sẽ biên dịch và chạy hoàn hảo, nhưng không mô tả bất cứ điều gì mà mã không có.

//Copy the value of y to temp.
temp = y;
//Copy the value of x to y.
y = x;
//Copy the value of temp to x.
x = temp;

Ngoài ra, các nhận xét về mã có thể đã được loại bỏ với mã đã tuân thủ một số nguyên tắc cơ bản:

//Set the age of the user to 13.
a = 13;
53
Rei Miyasaka

Mã tạo cảnh báo khi biên dịch.

42
Rei Miyasaka

Hàm có số trong tên thay vì có tên mô tả, như:

void doSomething()
{
}

void doSomething2()
{
}

Xin vui lòng, làm cho tên hàm có ý nghĩa gì đó! Nếu doS Something và doS Something2 làm những việc tương tự, hãy sử dụng tên hàm để phân biệt sự khác biệt. Nếu doS Something2 là một sự đột phá về chức năng từ doS Something, hãy đặt tên cho chức năng của nó.

36
Wonko the Sane

Số ma thuật hoặc Chuỗi ma thuật.

   if (accountBalance>200) { sendInvoice(...); }

   salesPrice *= 0.9;   //apply discount    

   if (invoiceStatus=="Overdue") { reportToCreditAgency(); }
36
JohnFx
  • Có thể không phải là tồi tệ nhất nhưng rõ ràng cho thấy mức độ người thực hiện:

    if(something == true) 
    
  • Nếu một ngôn ngữ có cấu trúc vòng lặp for hoặc iterator, thì việc sử dụng vòng lặp while cũng thể hiện mức độ hiểu biết về ngôn ngữ của người triển khai:

    count = 0; 
    while(count < items.size()){
       do stuff
       count ++; 
    }
    
    for(i = 0; i < items.size(); i++){
      do stuff 
    }
    //Sure this is not terrible but use the language the way it was meant to be used.
    
  • Chính tả/ngữ pháp kém trong tài liệu/nhận xét ăn vào của tôi gần như bằng chính mã. Lý do cho điều này là vì mã có nghĩa là cho con người đọc và máy móc để chạy. Đây là lý do tại sao chúng tôi sử dụng các ngôn ngữ cấp cao, nếu tài liệu của bạn khó vượt qua nó khiến tôi hình thành ý kiến ​​tiêu cực về codebase mà không cần nhìn vào nó.

36
Chris

Cái tôi nhận thấy ngay lập tức là tần số khối mã được lồng sâ (nếu là, trong khi, v.v.). Nếu mã là thường xuyên đi sâu hơn hai hoặc ba cấp, đó là dấu hiệu của vấn đề thiết kế/logic. Và nếu nó đi sâu như 8 tổ, tốt hơn hết là một lý do chính đáng để nó không bị phá vỡ.

29
GrandmasterB

Khi chấm điểm chương trình của học sinh, đôi khi tôi có thể nói trong một khoảnh khắc kiểu "chớp mắt". Đây là những manh mối tức thì:

  • Định dạng kém hoặc không nhất quán
  • Nhiều hơn hai dòng trống liên tiếp
  • Quy ước đặt tên không chuẩn hoặc không nhất quán
  • Mã lặp lại, lặp lại nguyên văn càng nhiều, cảnh báo càng mạnh
  • Điều gì phải là một đoạn mã đơn giản là quá phức tạp (ví dụ: kiểm tra các đối số được truyền đến chính theo cách phức tạp)

Hiếm khi những ấn tượng đầu tiên của tôi không chính xác và những tiếng chuông cảnh báo này đúng về 95% thời gian. Đối với một ngoại lệ, một sinh viên mới sử dụng ngôn ngữ này đã sử dụng một phong cách từ một ngôn ngữ lập trình khác. Đi sâu vào và đọc lại phong cách của họ trong thành ngữ của ngôn ngữ khác đã gỡ bỏ tiếng chuông báo thức cho tôi, và sau đó sinh viên đã nhận được tín dụng đầy đủ. Nhưng ngoại lệ như vậy là rất hiếm.

Khi xem xét mã nâng cao hơn, đây là những cảnh báo khác của tôi:

  • Sự hiện diện của many Java các lớp chỉ "cấu trúc" để giữ dữ liệu. Không thành vấn đề nếu các trường là công khai hoặc riêng tư và sử dụng getters/setters, nó vẫn không phải là một phần của một thiết kế chu đáo.
  • Các lớp có tên kém, chẳng hạn như chỉ là một không gian tên và không có sự gắn kết thực sự trong mã
  • Tham chiếu đến các mẫu thiết kế thậm chí không được sử dụng trong mã
  • Xử lý ngoại lệ trống mà không giải thích
  • Khi tôi kéo mã lên trong Eclipse, hàng trăm "cảnh báo" màu vàng sẽ mã dòng, chủ yếu là do nhập hoặc các biến không được sử dụng

Về kiểu dáng, tôi thường không muốn thấy:

  • Nhận xét Javadoc chỉ lặp lại mã

Đây là chỉ manh mối với mã xấu. Đôi khi những gì có vẻ như mã xấu thực sự không phải là vì bạn không biết ý định của lập trình viên. Ví dụ, có thể có một lý do chính đáng rằng một cái gì đó có vẻ quá phức tạp - có thể có một sự xem xét khác khi chơi.

28
Macneil

Yêu thích cá nhân/peeve thú cưng: IDE đã tạo tên được nhận. Nếu TextBox1 là biến chính và quan trọng trong hệ thống của bạn, bạn sẽ có một điều nữa đến khi xem xét mã.

25
Wyatt Barnett

Các biến hoàn toàn không được sử dụng, đặc biệt khi biến có tên tương tự với tên biến được sử dụng.

25
C. Ross

Rất nhiều người đã đề cập:

//TODO: [something not implemented]

Trong khi tôi ước điều đó được thực hiện, ít nhất họ đã ghi chú lại. Những gì tôi nghĩ là tồi tệ hơn là:

//TODO: [something that is already implemented]

Todo là vô giá trị và khó hiểu nếu bạn không bao giờ bận tâm để loại bỏ chúng!

21
Morgan Herlocker

Các kết hợp trong tên phương thức:

public void addEmployeeAndUpdatePayrate(...) 


public int getSalaryOrHourlyPay(int Employee) ....

Làm rõ: Lý do chuông này báo động là vì nó chỉ ra phương pháp có khả năng vi phạm nguyên tắc trách nhiệm duy nhất .

20
JohnFx

Một phương pháp đòi hỏi tôi phải cuộn xuống để đọc tất cả.

20
BradB

Liên kết rõ ràng mã nguồn GPL'd thành một chương trình nguồn đóng, thương mại.

Nó không chỉ tạo ra một vấn đề pháp lý ngay lập tức, mà theo kinh nghiệm của tôi, nó thường chỉ ra sự bất cẩn hoặc không quan tâm được phản ánh ở nơi khác trong mã.

13
Bob Murphy

Ngôn ngữ bất khả tri :

  • TODO: not implemented
  • int function(...) { return -1; } (giống như "không được thực hiện")
  • Ném một ngoại lệ cho một lý do không đặc biệt.
  • Việc sử dụng sai hoặc sử dụng không nhất quán 0, -1 Hoặc null làm giá trị trả về đặc biệt.
  • Khẳng định mà không có một bình luận thuyết phục nói tại sao nó không bao giờ nên thất bại.

Ngôn ngữ cụ thể (C++) :

  • Macro C++ viết thường.
  • Biến C++ tĩnh hoặc toàn cầu.
  • Các biến không được khởi tạo hoặc không sử dụng.
  • Bất kỳ array new Dường như không an toàn RAII.
  • Bất kỳ việc sử dụng mảng hoặc con trỏ rõ ràng là không an toàn. Điều này bao gồm printf.
  • Bất kỳ việc sử dụng phần chưa khởi tạo của một mảng.

Microsoft C++ cụ thể :

  • Bất kỳ tên định danh nào va chạm với macro đã được xác định bởi bất kỳ tệp tiêu đề SDK nào của Microsoft.
  • Nói chung, bất kỳ việc sử dụng API Win32 là một nguồn chuông báo động lớn. Luôn mở MSDN và tra cứu các định nghĩa giá trị đối số/trả về bất cứ khi nào nghi ngờ. (Đã chỉnh sửa)

C++/OOP cụ thể :

  • Kế thừa thực hiện (lớp cụ thể) trong đó lớp cha có cả phương thức ảo và không ảo, không có sự phân biệt khái niệm rõ ràng rõ ràng giữa những gì nên/không nên là ảo.
9
rwong

Sử dụng nhiều khối văn bản thay vì enum hoặc các biến được xác định toàn cầu.

Không tốt:

if (itemType == "Student") { ... }

Tốt hơn:

private enum itemTypeEnum {
  Student,
  Teacher
}
if (itemType == itemTypeEnum.Student.ToString()) { ... }

Tốt:

private itemTypeEnum itemType;
...
if (itemType == itemTypeEnum.Student) { ... }
8
Yaakov Ellis

Kiểu thụt đầu dòng kỳ lạ.

Có một vài phong cách rất phổ biến và mọi người sẽ tranh luận về Grave. Nhưng đôi khi tôi thấy ai đó sử dụng một kiểu thụt đầu dòng thực sự hiếm, hoặc thậm chí là trồng tại nhà. Đây là một dấu hiệu cho thấy họ có thể đã không được mã hóa với bất kỳ ai khác ngoài chính họ.

8
Ken

Nhập tham số yếu hoặc trả về giá trị trên các phương thức.

public DataTable GetEmployees() {...}

public DateTime getHireDate(DataTable EmployeeInfo) {...}

public void FireEmployee(Object EmployeeObjectOrEmployeeID) {...}
8
JohnFx

Mùi mã: không tuân theo các thực tiễn tốt nhất

Điều này luôn làm tôi lo lắng vì có sự thật rằng mọi người đều nghĩ rằng họ là một tài xế trên trung bình.

Đây là một tin tức chớp nhoáng cho bạn: 50% dân số thế giới dưới mức thông minh trung bình. Ok, vì vậy một số người sẽ có trí thông minh chính xác trung bình, nhưng chúng ta đừng kén chọn. Ngoài ra, một trong những tác động phụ của sự ngu ngốc là bạn không thể nhận ra sự ngu ngốc của chính mình! Mọi thứ sẽ không tốt nếu bạn kết hợp những tuyên bố này.

Những thứ ngay lập tức rung chuông báo động khi nhìn vào mã?

Nhiều điều tốt đã được đề cập, và nói chung có vẻ như không tuân theo các thực tiễn tốt nhất là một mùi mã.

Thực hành tốt nhất thường không được phát minh ngẫu nhiên, và thường có lý do. Nhiều khi nó có thể chủ quan, nhưng theo kinh nghiệm của tôi thì hầu hết là hợp lý. Thực hiện theo các thực tiễn tốt nhất không phải là một vấn đề và nếu bạn đang tự hỏi tại sao chúng lại như vậy, hãy nghiên cứu nó thay vì bỏ qua và/hoặc phàn nàn về nó - có thể nó hợp lý, có thể không.

Một ví dụ về cách thực hành tốt nhất có thể là sử dụng curlies với mọi khối if, ngay cả khi nó chỉ chứa một dòng:

if (something) {
    // whatever
}

Bạn có thể không nghĩ rằng nó cần thiết, nhưng tôi gần đây đọc rằng đó là một nguồn chính của lỗi. Luôn luôn sử dụng dấu ngoặc cũng đã được thảo luận về Stack Overflow và kiểm tra xem các câu lệnh if có dấu ngoặc cũng là quy tắc trong PMD , một bộ phân tích mã tĩnh cho Java.

Hãy nhớ rằng: "Bởi vì đó là cách thực hành tốt nhất" không bao giờ là câu trả lời chấp nhận được cho câu hỏi "tại sao bạn lại làm điều này?" Nếu bạn không thể nói rõ tại sao một cái gì đó là một thực tiễn tốt nhất, thì đó không phải là một thực tiễn tốt nhất, đó là một sự mê tín.

7
Vetle
  • Nhiều toán tử ternary nối lại với nhau, vì vậy thay vì giống như một khối if...else, Nó trở thành một khối if...else if...[...]...else
  • Tên biến dài không có dấu gạch dưới hoặc camelCasing. Ví dụ từ một số mã tôi đã kéo lên: $lesseeloginaccountservice
  • Hàng trăm dòng mã trong một tệp, có rất ít hoặc không có nhận xét và mã này rất không rõ ràng
  • Các câu lệnh if quá phức tạp. Ví dụ từ mã: if (!($lessee_obj instanceof Lessee && $lessee_obj != NULL)) mà tôi đã nhai xuống if ($lessee_obj == null)
7
Tarka

Bắt ngoại lệ chung :

try {

 ...

} catch {
}

hoặc là

try {

 ...

} catch (Exception ex) {
}

Vùng lạm dụng

Thông thường, sử dụng quá nhiều vùng cho tôi biết rằng các lớp học của bạn quá lớn. Đó là một lá cờ cảnh báo báo hiệu rằng tôi nên điều tra thêm về đoạn mã đó.

6
Erik van Brakel

Bất cứ ai cũng có thể nghĩ về một ví dụ trong đó mã nên tham chiếu hợp pháp một tệp theo đường dẫn tuyệt đối?

6
Rei Miyasaka

Những bình luận nói rằng "điều này là do thiết kế của hệ thống con froz hoàn toàn bị bó buộc."

Điều đó diễn ra trên toàn bộ một đoạn.

Họ giải thích rằng các cấu trúc lại sau đây cần phải xảy ra.

Nhưng đã không làm điều đó.

Bây giờ, họ có thể đã được thông báo rằng họ không thể thay đổi nó bởi ông chủ của họ vào thời điểm đó, vì vấn đề thời gian hoặc năng lực, nhưng có lẽ đó là do mọi người nhỏ mọn.

Nếu một giám sát viên nghĩ rằng j.random. lập trình viên không thể thực hiện tái cấu trúc, sau đó người giám sát nên thực hiện.

Dù sao, điều này xảy ra, tôi biết mã được viết bởi một nhóm bị chia rẽ, với chính trị quyền lực có thể, và họ đã không tái cấu trúc các thiết kế hệ thống con.

Câu chuyện có thật. Nó có thể xảy ra với bạn.

6
Tim Williscroft
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...

Tất nhiên không có bất kỳ loại tài liệu nào và thỉnh thoảng được lồng #define

5
Sven

Mã C++ với các câu lệnh xóa rõ ràng (trừ khi tôi nhìn vào phần bên trong của việc thực hiện con trỏ thông minh). 'xóa' là 'goto' của quản lý bộ nhớ IMHO .

Liên quan chặt chẽ đến điều này, mã như:

// Caller must take ownership
Thing* Foo::Bar()

(Và tính mình may mắn nếu có bình luận!). Nó không giống như con trỏ thông minh là khoa học tên lửa. std::auto_ptr được tạo cho loại điều này (ghi lại thực thi ý định thể hiện trong nhận xét) và là một phần của tiêu chuẩn = cho lứa tuổi bây giờ.

Cùng nhau hét lên mã di sản không được yêu thích, hoặc những người duy trì với một suy nghĩ bị mắc kẹt ở đâu đó vào đầu những năm 90.

5
timday

Các chức năng thực hiện lại chức năng cơ bản của ngôn ngữ. Ví dụ: nếu bạn từng thấy một phương thức "getStringL wavel ()" trong JavaScript thay vì gọi đến thuộc tính ".length" của một chuỗi, bạn biết bạn đang gặp rắc rối.

5
Ant

Các quy ước đặt tên lớp thể hiện sự hiểu biết kém về sự trừu tượng mà họ đang cố gắng tạo ra. Hoặc điều đó không định nghĩa một sự trừu tượng nào cả.

Một ví dụ cực kỳ xuất hiện trong lớp a VB tôi đã thấy một lần có tiêu đề Data và dài hơn 30.000 dòng ... trong đầu tiên tệp. Đó là một lớp một phần được chia thành ít nhất nửa tá các tệp khác. Hầu hết các phương thức là các hàm bao quanh các procs được lưu trữ với các tên như FindXByYWithZ().

Ngay cả với những ví dụ ít kịch tính hơn, tôi chắc chắn rằng tất cả chúng ta chỉ 'đổ' logic vào một lớp học được hình thành kém, đặt cho nó một tiêu đề hoàn toàn chung chung và hối hận về sau.

5
Bryan M.
ON ERROR RESUME NEXT

Phải duy trì Classic ASP đáng buồn là cần thiết cho hầu hết các nhà phát triển ASP.NET, nhưng việc mở một tệp bao gồm chung và thấy rằng trên dòng đầu tiên là phá hủy linh hồn.

4
richeym

Khi không có ý kiến ​​hoặc tài liệu nào về việc mã sẽ làm gì hoặc phải làm gì (nghĩa là "mã là tài liệu").

Các phương thức hoặc biến có số là hậu tố (ví dụ: Đăng nhập2 ()).

4
leson
try
{
//do something
}
catch{}
3
Tom Squires

Mã không bao giờ có thể, nhập một cách hợp lý vào đường dẫn thực thi.

var product = repository.FindProduct(id);

log.Info("Found product " + product.Name);

if (product != null)
{
    // This will always happen
}
else
{
    // **This won't ever happen**
}

hoặc là

if (messages.Count > 0)
{
    // Do stuff with messages
}
else if (messages.Count == 1)
{
    // **I hope this code isn't important...**
}
3
rmac
  • Đặt mọi biến cục bộ trong các dòng đầu tiên của khối phương thức. Đặc biệt là kết hợp với các phương pháp dài.
  • Sử dụng các biến boolean để thoát ra khỏi các vòng lặp/bỏ qua các lần lặp thay vì chỉ sử dụng break/continue
3
Oliver Weiler

Từ quan điểm trung tâm Java của tôi:

  • Phong cách mã hóa không chuẩn.
  • Biến không riêng tư.
  • Thiếu final trên các trường.
  • Vô nghĩa hoặc lạm dụng thừa kế.
  • Các lớp lớn hoặc khối mã.
  • Quá nhiều bình luận (có lẽ họ chỉ đang mơ tưởng thôi).
  • Khai thác gỗ không có cấu trúc.
  • Getters và setters (giao diện nên về hành vi).
  • Dữ liệu trùng lặp.
  • Phụ thuộc lạ.
  • Thống kê, bao gồm cả chủ đề toàn cầu.
  • Đối với mã đa luồng, các phần của cùng một lớp dự kiến ​​sẽ được chạy trong các luồng khác nhau (đáng chú ý là mã GUI).
  • Mã chết.
  • Thao tác chuỗi trộn lẫn với mã khác.
  • Nói chung, trộn các lớp (công cụ cấp cao hơn, kết hợp với lặp qua một mảng nguyên thủy hoặc xử lý luồng, nói).
  • Bất kỳ việc sử dụng sự phản ánh.
  • catch khối không có mã hữu ích (xấu: bình luận, printf, đăng nhập hoặc chỉ trống).
3
Tom Hawtin - tackline

Sử dụng một đối tượng ẩn trong giao diện người dùng (ví dụ: hộp văn bản) để lưu trữ một giá trị thay vì xác định một biến có phạm vi và phạm vi chính xác.

2
MartW

Bất cứ lúc nào tôi đọc như sau:

//Don't put in negative numbers or it will crash the program.

Hoặc một cái gì đó tương tự. Nếu đó là trường hợp sau đó đưa vào một khẳng định! Hãy để trình gỡ lỗi biết rằng trong thời gian chạy, bạn không muốn các giá trị đó và đảm bảo mã giải thích hợp đồng với người gọi.

2
wheaties

Loại mã này:

        if (pflayerdef.DefinitionExpression == "NM_FIELD = '  '" || One.Two.nmEA == "" || 
            One.Two.nmEA == " " || One.Two.nmEA == null ||
            One.Two.nmEA == "  ")
        {                
            MessageBox.Show("BAD CODE");
            return;
        }

Đây là từ một cơ sở sản xuất trực tiếp thực sự!

2
George Silva

Đối với số ma thuật: chúng rất tệ nếu chúng được sử dụng ở những nơi khác nhau và việc thay đổi nó đòi hỏi bạn phải đồng bộ hóa nó ở một vài nơi. Nhưng một số ở một nơi không tệ hơn là có một hằng số để biểu thị một số vẫn được sử dụng ở một nơi.

Hơn nữa, hằng số có thể không có nhiều vị trí trong ứng dụng của bạn. Trong nhiều ứng dụng cơ sở dữ liệu, những thứ đó nên được lưu trữ trong cơ sở dữ liệu theo ứng dụng hoặc theo cài đặt của người dùng. Và để hoàn thành việc triển khai, họ liên quan đến cài đặt này và một vị trí trong ui và một ghi chú trong tài liệu người dùng cuối ... tất cả đều tốt - một sự áp đảo và lãng phí tài nguyên nếu mọi người hoàn toàn hài lòng khi con số là 5 ( và 5 nó là.)

Tôi nghĩ bạn có thể để số và chuỗi tại chỗ cho đến khi có nhu cầu sử dụng số này bên ngoài địa điểm đó. Sau đó là thời gian để cấu trúc lại mọi thứ để thiết kế linh hoạt hơn.

Đối với các chuỗi ... Tôi biết các đối số, nhưng đây là một nơi nữa không có điểm nào trong việc thực hiện chuyển đổi một chuỗi thành một liên tục. Đặc biệt là nếu các chuỗi tại chỗ đến từ việc triển khai liên tục dù sao (ví dụ: nhập được tạo từ ứng dụng bên ngoài và có chuỗi trạng thái ngắn và dễ nhận biết, giống như 'Quá hạn'.) nhiều điểm trong việc chuyển đổi 'Quá hạn' thành STATUS_OVERDUE khi nó chỉ được sử dụng ở một nơi duy nhất.

Tôi rất muốn không thêm sự phức tạp nếu nó không thực sự tạo ra lợi ích cần thiết về tính linh hoạt, tái sử dụng hoặc kiểm tra lỗi. Khi bạn cần sự linh hoạt, mã hóa nó đúng (điều chỉnh cấu trúc lại). Nhưng nếu không cần thiết ...

2
Inca

Mã kết hợp chặt chẽ. Đặc biệt là khi bạn thấy nhiều thứ được mã hóa cứng (tên của máy in mạng, địa chỉ IP, v.v.) ở giữa mã. Chúng phải nằm trong một tệp cấu hình hoặc thậm chí là các hằng số, nhưng sau đây sẽ gây ra vấn đề:

if (Host_ip == '192.168.1.5'){
   printer = '192.168.1.123';
} else
  printer = 'prntrBob';

Một ngày nào đó, Bob sẽ nghỉ việc và máy in của anh sẽ được đổi tên. Một ngày nào đó máy in sẽ nhận được một địa chỉ IP mới. Một ngày nào đó 192.168.1.5 sẽ muốn in trên máy in của Bob.

câu thần chú yêu thích của tôi: luôn viết mã như một kẻ tâm thần giết người, người biết nơi bạn sống sẽ phải duy trì nó!

2
davidhaskins

Mã cho thấy rằng lập trình viên không bao giờ thích nghi, nhiều năm trước, thành Java 5:

  • Sử dụng các Iterators thay vì trên mạng cho mỗi phạm vi
  • Không sử dụng tổng quát trong các bộ sưu tập và chuyển các đối tượng đã truy xuất sang loại dự kiến
  • Sử dụng các lớp cổ như Vector và Hashtable

Không biết những cách đa luồng hiện đại.

2
Dave Briccetti

Đối với SQL:

Các chỉ số lớn đầu tiên là việc sử dụng các phép nối ngụ ý.

Tiếp theo là việc sử dụng nối trái trên bảngB kết hợp với mệnh đề WHERE như:

WHERE TableB.myField = 'test'

Nếu bạn không biết rằng sẽ cho kết quả không chính xác thì tôi không thể tin rằng bất kỳ truy vấn nào bạn viết sẽ cho kết quả chính xác.

2
HLGEM

Mã VB6 kế thừa của chúng tôi, bạn có thể mở bất kỳ trang Mô-đun hoặc mã mẫu nào và tìm một màn hình đầy đủ Công khai hoặc Toàn cầu # & @ ! Các biến được khai báo ở trên cùng, được tham chiếu từ khắp nơi trên @ &!! (*! # chương trình. ARGH !!!!

(Whew, tôi phải lấy nó ra :-))

2
HardCode

Một cái gì đó như thế này

x.y.z.a[b].c

Điều này có mùi giống như nguy hiểm sinh học. Tham chiếu nhiều thành viên này không bao giờ là một dấu hiệu tốt. Và vâng, đây là một biểu thức điển hình trong mã tôi đang làm việc.

2
Gaurav

bất cứ điều gì với cái gì đó như thế này

// TODO: anything could be here!

Chỉnh sửa: Tôi nên đủ điều kiện rằng tôi có ý nghĩa này trong mã sản xuất. Nhưng ngay cả trong mã cam kết kiểm soát nguồn này vẫn không tốt. Nhưng, đó có thể là một điều cá nhân ở chỗ tôi muốn kết thúc ngày nghỉ khi buộc chặt tất cả các kết thúc lỏng lẻo của mình :)

Chỉnh sửa 2: Tôi cần đủ điều kiện hơn nữa khi tôi thấy điều này trong mã được thiết lập. Giống như một cái gì đó đã có một số năm tuổi và tôi đang sửa một lỗi. Tôi thấy một TODO và đó là khi chuông báo thức bắt đầu reo. TODO (với tôi) ngụ ý rằng mã không bao giờ được hoàn thành vì một số lý do.

2
Antony Scott

Việc sử dụng từ khóa synchronized trong Java.

Không có gì sai khi sử dụng synchronized khi nó được sử dụng đúng. Nhưng trong mã tôi làm việc cùng, dường như mỗi khi ai đó cố gắng viết mã luồng an toàn, họ lại hiểu sai. Vì vậy, tôi biết rằng nếu tôi thấy từ khóa này, tôi phải hết sức cẩn thận về phần còn lại của mã ...

1
Guillaume

Tối ưu hóa lổ nhìn trộm trên mã có thể được tối ưu hóa với cấu trúc tốt hơn, ví dụ: tìm kiếm tuyến tính được triển khai trong hội đồng nội tuyến khi tìm kiếm nhị phân trong C/C++/Java/C # đơn giản sẽ phù hợp (và nhanh hơn).

Hoặc là người đó đang thiếu một số khái niệm cơ bản, hoặc không có ý nghĩa về các ưu tiên. Sau này là đáng lo ngại hơn nhiều.

1
Rei Miyasaka

@FinnNk, tôi đồng ý với bạn về mã nhận xét. Điều thực sự làm tôi bực mình là những thứ như thế này:

for (var i = 0; i < num; i++) {
    //alert(i);
}

hoặc bất cứ điều gì đã có để thử nghiệm hoặc gỡ lỗi, và đã được nhận xét và sau đó cam kết. Nếu nó chỉ là thỉnh thoảng, nó không phải là một vấn đề lớn, nhưng nếu nó ở khắp mọi nơi, nó làm lộn xộn mã và làm cho khó thấy những gì đang xảy ra.

1
Elias Zamaria
  • $ data - Giống như quảng cáo "Các đối tượng vật lý, hiện ở mức thấp vô lý 100 trên 5!"
  • Các mục TODO trong mã - Sử dụng trình theo dõi lỗi/vé/vấn đề để mọi người biết những gì cần thiết ở cấp độ sản phẩm thay vì cấp độ tệp.
  • Mã đăng nhập công việc - Đó là kiểm soát phiên bản dành cho.
1
l0b0

Bất cứ điều gì vi phạm các nguyên tắc quan trọng. Chẳng hạn, một vài kiểu chống mẫu đã được đề xuất (số ma thuật - xem http://en.wikipedia.org/wiki/Anti-potype ). Các mô hình chống được gọi là bởi vì chúng gây ra vấn đề (cũng đã được đề cập - mong manh, ác mộng bảo trì, v.v.). Ngoài ra, coi chừng vi phạm các nguyên tắc SOLID - xem http://en.wikipedia.org/wiki/Solid_ (object-direction_design) Ngoài ra, Mã không 'xem xét việc tách các tầng (những thứ truy cập dữ liệu bên trong UI, v.v.). Có các tiêu chuẩn mã hóa và đánh giá mã giúp chống lại điều này.

1
Tim Claason

Hầu hết trong số này là từ Java kinh nghiệm:

  • Gõ chuỗi. Không.
  • Kiểu chữ thường sẽ chỉ ra mùi mã trong Java hiện đại.
  • Mô hình chống Pokemon Exception (khi bạn phải bắt hết chúng).
  • Nỗ lực vận chuyển hàng hóa tại chương trình chức năng khi không phù hợp.
  • Không sử dụng cấu trúc giống như chức năng (Có thể gọi được, Chức năng, v.v.) nếu phù hợp.
  • Thất bại trong việc tận dụng tính đa hình.
1
Ben Hardy

Khi mã trông giống như một mớ hỗn độn: Nhận xét với "việc cần làm" và "ghi chú cho bản thân" và những câu chuyện cười khập khiễng. Mã rõ ràng được sử dụng tại một số điểm chỉ nhằm mục đích gỡ lỗi, nhưng sau đó không bị xóa. Các biến hoặc hàm có tên gợi ý rằng người viết không xem xét rằng bất kỳ ai sẽ đọc nó sau này. Thường thì những cái tên này sẽ rất dài và khó sử dụng.

Ngoài ra: Nếu mã thiếu nhịp điệu. Chức năng của chiều dài khác nhau hoang dã. Các chức năng không tuân thủ các sơ đồ đặt tên giống nhau.

Hơi liên quan: Nó luôn khiến tôi lo lắng nếu một lập trình viên có chữ viết tay slobby. Hoặc nếu họ là nhà văn xấu hoặc người giao tiếp xấu.

1
KaptajnKold

Tôi đã từng làm việc trong một dự án mà nhà thầu đã nhập từng kiểu dữ liệu tiêu chuẩn từ int đến chuỗi bao gồm các con trỏ đến các tên tối nghĩa. Cách tiếp cận của ông làm cho việc hiểu mã thực sự khó khăn. Một phong cách khác cảnh báo tôi là sự linh hoạt sớm; một sản phẩm tôi từng làm việc có mã hoàn chỉnh trong các DLL được tải theo thứ tự không dự đoán được. Tất cả điều này để phù hợp với sự tiến hóa trong tương lai. Một vài DLL sử dụng trình bao bọc luồng cho tính di động. Đó là một cơn ác mộng để gỡ lỗi chương trình hoặc dự đoán luồng bằng cách đọc mã nguồn. Các định nghĩa được phân tán trên các tệp tiêu đề. Không có gì ngạc nhiên khi mã không tồn tại ngoài phiên bản thứ hai.

1
VKs

Đôi khi tôi thấy các phần của một phương thức cung cấp tất cả các đầu vào có thể, nó vẫn KHÔNG BAO GIỜ chạy, vì vậy nó không nên ở đó khiến mọi người bối rối ngay từ đầu. Một ví dụ sẽ là ...
[.___.] Nếu phương thức chỉ có thể được gọi trong ngữ cảnh của siêu người dùng Quản trị viên và tôi thấy có gì đó kiểm tra xem người dùng yêu cầu không phải là siêu người dùng Quản trị viên ...: /

1
chiurox

Những bình luận bất bình thể hiện sự thiếu kiềm chế:

//I CAN'T GET THIS F^#*&^ING PIECE OF S$^* TO COMPILE ON M$VC

Họ là người nóng tính hoặc họ không đủ kinh nghiệm để biết rằng thất bại là không thể tránh khỏi trong lập trình.

Tôi không muốn làm việc với những người như vậy.

1
Rei Miyasaka

Đây là một triệu chứng hơi nhỏ so với những thứ mà người khác đã liệt kê, nhưng tôi là một Python lập trình viên và tôi thường thấy điều này trong cơ sở mã của chúng tôi:

try:
    do_something()
except:
    pass

Điều này thường báo hiệu cho tôi rằng lập trình viên không thực sự biết (hoặc quan tâm) tại sao do_something có thể thất bại (hoặc những gì nó làm) - anh ta chỉ tiếp tục "nghịch ngợm" cho đến khi mã không bị lỗi nữa.


Đối với những người đến từ một nền tảng Java-esque hơn, khối đó là Python tương đương với

try {
    doSomething();
} catch (Exception e) {
    // Don't do anything. Don't even log the error.
}

Vấn đề là, Python sử dụng ngoại lệ cho mã "không đặc biệt", chẳng hạn như ngắt bàn phím hoặc thoát ra khỏi vòng lặp for.

1
mipadi

getters khắp nơi làm cho tôi hoảng sợ.

và một điều đặc biệt: getters ủy thác cho các getters khác.

điều này là xấu bởi vì nó có nghĩa là người viết mà không hiểu cơ bản của hướng đối tượng, đó là đóng gói, có nghĩa là dữ liệu ở đâu, phương thức hành động trên dữ liệu đó phải là gì.

ủy thác là cho một phương pháp không phải tất cả các getters. nguyên tắc là "nói, đừng hỏi"; nói một điều với một đối tượng để làm, đừng yêu cầu nó cho một nghìn điều và sau đó tự làm điều đó.

getters freak tôi ra, vì nó có nghĩa là các nguyên tắc oop khác sẽ bị vi phạm lõi cứng.

0
Belun

Thiếu thông tin loại.

Có một cái nhìn vào các chữ ký phương pháp:

  1. public List<Invoice> GetInvoices(Customer c, Date d1, Date d2)

  2. public GetInvoices(c, d1, d2)

Trong (1) có sự rõ ràng. Bạn biết chính xác những tham số bạn cần để gọi hàm với và rõ ràng hàm này trả về cái gì.

Trong (2) chỉ có sự không chắc chắn. Bạn không biết nên sử dụng tham số nào và bạn không biết hàm trả về cái gì nếu có gì đó. Bạn bị buộc phải sử dụng một cách tiếp cận thử nghiệm và lỗi không hiệu quả để lập trình.

0
ThomasX