Hi mọi người , đây là dự án socket chat thu gọn của em
Em có 2 vấn đề
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
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.
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.
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).
File IOs are not completely protected from overwriting/intermixture by a synchronized() block. A separate protection procedure must be developed here.
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.
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.
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.
Cậu có chắc cậu chỉ có 2 vấn đề không?
Tớ có đọc code của cậu, và tớ nghĩ cậu phải có tới hơn 30 vấn đề cơ
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ả:
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.
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, historyQueue và messageQueue hoàn toàn vô dụng.
Ở IOClientHandler, cậu có import static Server.ServerManager.clientSocketsList
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ế
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.
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.
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.
cảm ơn bạn , sau khi đọc kĩ lại các vấn đề mà bạn và Joe liệt kê , nên mình đã quyết định rebuild lại lần nữa
về rebuild lần này , mình loại bỏ việc lưu vào file và dùng database mysql , áp dụng về quy chuẩn oop , quy ước đặt tên đúng
vì sử dụng database mysql nên vấn đề 2 user cùng gửi tin nhắn 1 lúc tại 1 LocalDataTime.now() sẽ ko bị trùng ID nhờ vào cơ chế auto-increment của csdl và việc sử dụng transaction management của JPA
còn vấn đề đang load lịch sử mà có tin nhắn từ user khác nhắn tới thì nhánh input của client sẽ chia if - else , từ đây khi user A yêu cầu load lịch sử , server sẽ gửi cái số ID lớn nhất trong 5 cái tin lịch sử gửi về cho A trước rồi tiếp theo là 5 tin lịch sử . Sau đó , trong quá trình chạy mà B có gửi tin nhắn tới thì lấy số ID lớn nhất vừa nãy xét với ID tin nhắn của B , chắc chắn 100% là ID tin nhắn của B sẽ lớn hơn nhưng làm vậy để sàng lọc tin nhắn của B về bên phía hiện tại , còn đâu thuộc else thì cứ rơi vào history
vấn đề luồng thì hiện tại còn 2 luồng
server - client
server khởi chạy -> tạo 1 luồng được quản lý bởi
ExecutorService threadPool = Executors.newFixedThreadPool
sử dụng threadPool sẽ dễ quản lý thread và tối ưu hơn thay vì cứ new Thread() như những gì mình nghiên cứu lại
git mới của mình , mong bạn dành thêm chút tgian đọc và đánh giá git mới của mình , mình cảm ơn
(Vì là chương trình dạng test nên mình thiết kế 2 user với nhau thôi nên khi client chạy phải đảm bảo là 2 user khác username )
mình loại bỏ việc lưu vào file và dùng database mysql
In your previous version, you make your app dependent on GSON. Now you make it dependent on MYSQL and Telegram. Do you really think that people who want (or buy) your app will buy MYSQL and download Telegram just to run this little app? It’s amazing how you complicate your project.
áp dụng về quy chuẩn oop , quy ước đặt tên đúng
What do you mean chuẩn oop ? You write an app in an OOP language and that doesn’t make your app OOP compliant. An object is a complete entity. When an entity is split, it is no longer an entity, but an incomplete entity. Also: no longer an object.
The ServerManager is an un-OOP class with no constructor that accepts parameters (e.g. port number) but with an inflexible fixed port number. What happens if that port number is already taken by another server?
You separate the spawned children (IOClientHandler) from the parent (Server) and that makes the entity server incomplete, so you have to pass, for example, the entire private data as parameters to the children. Nothing is more complicated than the way you do it here.
The server’s job is to accept incoming clients, spawn a thread to serve the client, then immediately wait for the next client. What you are doing here is creating a dangerous latency between accepting and spawning with several unnecessary steps. Why on earth doesn’t the spawned thread do this when it has all the private server variables as parameters?
I think you have no idea and concept of what you are doing and the more you struggle, the worse the result. Sorry that’s the truth!
83% thành viên diễn đàn không hỏi bài tập, còn bạn thì sao?