Java socket: Cải thiện cách xử lý lịch sử chat

Hi mọi người , đây là dự án socket chat thu gọn của em

Em có 2 vấn đề

  1. Khi 1 user load lịch sử mà có tin nhắn mới từ user khác gửi tới sẽ gây chen tin nhắn mới vào chuỗi tin lịch sử
    Em đã xử lý bằng cách chuyển nội dung từ server chuyển về thành JSON và từ JSON đọc type bên trong , input sẽ phân tích type chứa nội dụng là "current hay “history” và chuyển vào queue khác nhau . Từ đây luồng input hoạt động liên tục mà vẫn đảm bảo phân lọc đúng current và history để đẩy lên UI một cách song song cả 2 tin nhắn của 2 thời điểm

  2. Khi 2 user cùng gửi tin nhắn tới nhau tại 1 thời điểm thì việc khởi tạo ID sẽ bị trùng lặp khi vào file lưu
    Em đã xử lý bằng cách gắn sync cho phương thức khởi tạo ID , vì mỗi client sẽ có 1 thread riêng nên thread nào vào trước sẽ phải thread còn lại làm xong

ko biết 2 cách giải quyết 2 vấn đề có được tối ưu ko ? và cần cải thiện gì cho 2 cách trên ko ạ , em cảm ơn mọi người nhiều

The more I read your package, the more I believe that your Java knowledge is patchy.

  1. Threads are expensive and resource-intensive. The more threads are involved, the greater the risk of overwriting each other (poor concurrency or parallelism). Instead of an extra thread (IOClientHandler), why don’t you use the ExecutorService to start the client that has arrived? An ArrayList is not only superfluous, but it makes the separation between “me” and “the others” more difficult.
  2. The extra (IOClient)Thread causes problems with synchronization because you no longer know which thread belongs to whom and thus the risk of overwriting/intermixture becomes omnipresent (see 1).
  3. File IOs are not completely protected from overwriting/intermixture by a synchronized() block. A separate protection procedure must be developed here.
  4. Why can’t the ClientManager be started directly as client, but by an extra app? And that worsens the overview so much that you don’t even know which client is doing what.
  5. BlockingQueue is good for modification (add, remove), but not secure for reading or addAll(). You have to implement a ReentrantLock, for example, so that reading is more secure.

Thank you for your help

1 + 3 + 5 : oke , i will consider what you advise and apply it again to my project

4 : In fact, each client of a user will be a different device and each different device is a separate main, so I separate the classes containing ClientManager as a practical example, right?

2 : from 4 , every time a class containing ClientManager runs

is a starting device of the starting user

from here there will be 2 separate threads for that client (IO), 2 threads belonging to that client

You don’t seem to understand what I’m talking about. The principle of client/server is that each client gets a server worker spawned by the server and both live and work with each other while the worker takes care of communication with other clients for its own partner on the server’s side. Instead of sticking with the easy way, you try to invent a new way where you don’t even know what’s going on inside your app and I’m not going to rewrite your app.

Sorry. I think you should consider whether to continue as an IT developer or choose an easier profession that requires less abstract imagination than IT.

I’ll stop helping you here and wish you all the best in making the right choice for your future,

UPDATE:

In fact, each client of a user will be a different device and each different device is a separate main, so I separate the classes containing ClientManager as a practical example, right?

Different device? Gosh, since when does the server care about the hardware the client is running on and your app uses the same ClientManager just with a string parameter userID. So what’s the difference? JAVA is hardware independent and that’s the main reason why JAVA is so widely used in the developer community.

I will absorb the knowledge about synchronization as you said and rebuild

But what about handling duplicate history and new messages? Do you think it’s okay?

Cậu có chắc cậu chỉ có 2 vấn đề không? :smile:
Tớ có đọc code của cậu, và tớ nghĩ cậu phải có tới hơn 30 vấn đề cơ :smile:

Cậu chạy được ở máy của cậu đúng với những gì cậu muốn đúng không? Vậy thì đây có lẽ không phải vấn đề.
Tớ biết cậu mong cách tối ưu, nhưng với thiết kế app rắc rối hiện tại của cậu, tớ không nghĩ có cách nào tối ưu hơn đâu. Cậu phải cải thiện app của cậu trước khi nói tới chuyện tối ưu.

Tớ có xem cách cậu xử lý, tớ phải nói nó rất buggy. Cậu có một biến static trong 1 class, và để đảm bảo biến đó có thể được thay đổi với từng thread, cậu phải sychronized tất cả các code sửa biến chung này. Điều này dẫn tới 2 hệ quả:

  1. Throughput của app cậu sẽ giảm xuống, do chỉ có 1 thread duy nhất có thể access vào method này tại 1 thời điểm. Nếu method đó tốn 1s để xử lý, through put của cậu sẽ là 1 message/s.
    Đã vậy, method đó cũng khá cồng kềnh, cậu phải sync cả đoạn đọc file để lấy ID, parse nó sang int, tăng thêm 1, rồi parse ngược lại về String.
    Tất nhiên, phần này nâng cao hơn so với một bài tập nho nhỏ như cậu đang làm.
  2. Nếu lúc nào đó, cậu viết một method mới, và quên mất phải synchronize đoạn update generate ID, cậu sẽ gặp lỗi. Đây là lý do chính tớ nói cách xử lý đó khá buggy.

Có thể cậu sẽ chạy được theo ý cậu, nhưng cậu phải học cách xử lý tốt hơn. Cách cậu xử lý lại là thứ cản trở cậu sửa hay phát triển app về sau.


@Joe ở trên đã chỉ cho cậu một vài vấn đề, nhưng tớ nghĩ bạn ấy khá nice, và khá nản khi đọc code của cậu. Tớ sẽ chỉ cho cậu thêm vài vấn đề nữa, mà tớ nghĩ cậu cần sửa nó ngay nếu cậu muốn tiến xa hơn.

Vấn đề về code

Cậu vẫn còn rất yếu ở phần đặt tên biến/method, cũng như cấu trúc source code.

  • Class FileUtil kế thừa Thread là một thảm hoạ. Thường, utility class là class chứa các method tiện ích, không có side effect, giúp cậu xử lý thứ gì đó. TimeUtil chính xác là utility class.
    Tuy nhiên, class đó có nhiệm vụ đọc file, quản lý ID (để làm gì đó tớ không hiểu), load lịch sử chat lưu trong file… Đã vậy, để nó kế thừa Thread không để làm gì cả.
  • Tên package cậu viết hoa (Client, Server, Service). Cậu nên viết thường thì đúng hơn.
  • Access modifier của cậu cũng đặt lung tung. Ví dụ: ChatLogger.jsonMessage cậu để default (nên là private), ChatLogger.saveMessage cậu để public (nên là private).

Đừng nghĩ đây là vấn đề nhỏ nhặt. Nếu cậu làm thợ xây, nếu cậu để đinh, búa, dụng cụ lung tung, dán nhãn hộp đinh là đ, ổ điện là s, cậu sẽ gây tai nạn do người khác không hiểu cậu đang làm cái quái gì. Cậu phải tuân theo practice, biết thứ gì đặt ở đâu, và đảm bảo mọi người đều hiểu điều đó dễ dàng nhất có thể.
Trong code, trừ khi cậu viết code mà không ai phải đọc, cậu cũng phải tuân theo practice. Việc đặt tên biến/method, cách cấu trúc code, folder… là thứ cơ bản nhất mà cậu phải học.
Tớ recommend cậu đọc “The Art of Readable Code: Simple and Practical Techniques for Writing Better Code” để cải thiện việc này.

Vấn đề xử lý

  • Cậu start thread FileUtil, xong ngồi chờ thread đó chạy xong để làm tiếp. Điều đó khác gì với việc cậu làm ở thread chính?
    Nó giống việc cậu thuê một người giúp việc quét dọn nhà cửa, nhưng cậu lại ngồi trông cô ấy làm xong việc thì cậu mới làm việc tiếp.
  • Cứ mỗi lần cậu cần lưu message vào file, cậu tạo 1 thread mới để làm điều đó. Nó giống như việc mỗi ngày cậu thuê một người giúp việc mới để dọn nhà cửa vậy.
  • Cậu không có bất cứ logic dọn dẹp nào khi cậu tắt app, trong khi cậu vẫn còn một mớ client socket đang mở. Điều đó sẽ tạo ra một mới zombie thread ở client.
  • Tất cả các thông tin như port, file location, etc. đều hard code và không config được.
  • Thay vì việc lấy username từ argument của main method, cậu tạo ra 4 main method với 4 username khác nhau.
  • Gson library được import bằng tay từ IDE của cậu, không quả pom.xml. Nó làm cho project của cậu không thể build được từ máy người khác.
  • ClientManager, historyQueuemessageQueue hoàn toàn vô dụng.
  • IOClientHandler, cậu có import static Server.ServerManager.clientSocketsList :face_vomiting:
  • Cậu tổ chức chat/lệnh dựa trên socket, không phải dựa vào username. Nếu 1 người login vào 2 thiết bị, 1 bên gõ lệnh history, tất cả các thiết bị nên được đồng bộ chứ?
  • Cậu có nested thread (cậu tạo 1 thread, trong đó tạo các threads khác). Thực ra, nó không phải vấn đề, nếu cậu biết chính xác cậu đang làm gì (tớ e là không). Xử lý concurrency tớ e là quá advance so với trình độ hiện tại của cậu.

Vấn đề thiết kế

  1. Với bài tập nhỏ như vậy, cậu có thể lưu thẳng dữ liệu chat vào memory (tạo một collection nào đó để xử lý), không cần tới file. Tối ưu nhất thì cậu phải lưu vào database, file hoàn toàn không phù hợp để tổ chức lịch sử chat và người dùng như vậy.
    Việc sử dụng file là điều khiến cậu gặp nhiều khó khăn đến vậy khi xử lý message.
    Vì đây là bài tập để hiểu về socket, tớ không nghĩ có ai phàn nàn nếu cậu lưu vào bộ nhớ đâu.
  2. Cậu thiếu kiến thức trầm trọng về một kiến trúc app cơ bản. Cậu nên thử học MVC và áp dụng vào bài tập xem.

Qua code của cậu, tớ hiểu cậu đang ở trình độ tương đương với 1 người mới học Java 1 tháng. Nó không quá tệ, nhưng còn rất nhiều thứ cậu cần cải thiện để có thể làm việc chuyên nghiệp được.

Hope it helps!

2 Likes

Yes, @library. You’re pointing to the right wound. This boy has countless problems, not 2 like he thinks.
As I said, this boy is addicted to threads and statics like a junkie addicted to meth and weed. He overuses threads and statics and gets caught in the maze of synchronization.

  • ClientManager: Main thread and 2 thread children. This boy seems to be learning threads but doesn’t know how and why threads are needed.Example: Listening is passive, so with one thread it’s fine. Communication with the server is active. An extra thread is a blatant nonsense here.
  • ServerManager: Besides the server thread, which should be started in an ExecutorService pool, this boy runs an IOClientHandler thread, which then unnecessarily runs two thread children, whose end must be waited for by calling join() - how stupid. And then with another thread to save the work… Excessive IOs that can choke the computer as the number of “chatters” keeps growing.

Next. Why on earth does he have to use a JSONMessage depending on Gson for the simple message? This is a real mystery.
The whole package is a mess and in my opinion the only thing to do is throw it in the trash bin and write a completely new package.

Please read @library’s comment carefully then you know why your mentor is so unhappy and why I am reluctantly correct your codes.

But what about handling duplicate history and new messages? Do you think it’s okay?

You’ve misled this forum with your thread as Socket-Problem. Your real problem is your patchy knowledge about thread. You learn thread like a machine without thinking about what-for. The way you use threads excessively is like a junkie smoking weed excessively. And like the junkie getting high on weed, you get “high” on threads and become a thread sleepwalker.

83% thành viên diễn đàn không hỏi bài tập, còn bạn thì sao?