Please Critique My Rough Design
I'm not going to post my entire design, as in the entire UML drawing, the reasons being 1) you simply can't post pictures and 2) I don't need a critique of the entire design, just some of my ideas. With that settled, here are my ideas for the object communication in an instant messenging program that I am making. By the way, this isn't double posting, as the content of this post is different.
I am making the client right now. The way the client works is like a cycle: the GUI produces events which when heard cause either packets to be sent to the server or interprogram communication, such as GUI objects talking to eachother or models being modified... the rest of the cycle is that the client receives packets from the server, parses those packets, and modifies the appropriate models and views and sometimes responds to an incoming packet with an immediate outgoing packet.
This is the simplified version: what's really going on is I have models, GUI views, GUI controllers, parsers, packets, and socket interfaces all violently colliding with eachother.
To be more specific, I am dividing the task of parsing into several parser objects. I made a socket interface that writes out and reads in packets. (It reads in packets by asking a protocol know-it-all questions about how many bytes the header is, how many bytes the message length is, etc.) Upon receiving a packet, the socket interface asks a parser factory for a parser associated with that packet (determined by message ID, sequence of bytes, etc.). The socket interface then sends the parser the packet and a reference to the socket interface itself so that the parser can respond to the server easily. The parser then modifies the necessary models and views as would a controller. There are many models and views it has to modify, so I am making facades for each parser so that it does not have to know about a multitude of models and views. For instance, I don't want my model parser to have to know about all the channel lists, friends lists, and games going on. If I was, then the GUI objects would have to register themselves somehow with the parser.Instead I am registering facades with each parser and then registering the parser with the parser factory.
As for how the client events generate packets, I have a packet builder class that returns packets to send given an identifying string. I may give the packet builder relevant information for each packet so that my GUI objects, which listen to the events, don't have to know about how to set up a packet. An example of this is sending a text message -- the chat window gui object asks the packet builder for a text message packet sending the identifier "txt msg" and the message itself as part of the relevant information. The builder returns a packet to the chat window and the chat window tells the packet to send itself. The packet has a reference to the socket interface, and so it sends itself to the socket interface. In its constructor it intiates its data.
Is this a flexible plan that I can easily work with? I know it's not a full design, but am I headed in the right direction? I also would like to know if I am overcomplicating things -- for instance, should the GUI objects just know about the socket interface and the packets and send the packets to the socket interfaces themselves, depending on the user-generated event? And can somebody please respond? I am a high school student who does not take a computer class and has no one to critique his designs. Thus, I could be headed in the wrong direction and walk miles down that path before even knowing it, wasting a lot of my time. If you want me to be more specific, I'll be happy to post more details.
[3753 byte] By [
ktm5124a] at [2007-10-2 17:09:21]
Please respond... am I overcomplicating things?
> Please respond... am I overcomplicating things?
I don't know I didn't read your post. But you do realize it's Saturday right? I mean not too many people are working or here on a Saturday. So waiting an hour is not really the end of the world.
Bump your thread if you haven't had any responses by Tuesday, until then relax.
> I'm not going to post my entire design, as in the
> entire UML drawing, the reasons being 1) you simply
> can't post pictures and 2) I don't need a critique of
> the entire design, just some of my ideas. With that
> settled, here are my ideas for the object
> communication in an instant messenging program that I
> am making. By the way, this isn't double posting, as
> the content of this post is different.
>
> I am making the client right now. The way the client
> works is like a cycle: the GUI produces events which
> when heard cause either packets to be sent to the
> server or interprogram communication, such as GUI
> objects talking to eachother or models being
> modified... the rest of the cycle is that the client
> receives packets from the server, parses those
> packets, and modifies the appropriate models and
> views and sometimes responds to an incoming packet
> with an immediate outgoing packet.
You're doing everything with raw sockets? You've written your own protocol? Is this really necessary? What was the requirement that drove this design decision?
> This is the simplified version: what's really going
> on is I have models, GUI views, GUI controllers,
> parsers, packets, and socket interfaces all violently
> colliding with eachother.
Sounds like a big mess.
> To be more specific, I am dividing the task of
> parsing into several parser objects. I made a socket
> interface that writes out and reads in packets. (It
> reads in packets by asking a protocol know-it-all
> questions about how many bytes the header is, how
> many bytes the message length is, etc.) Upon
> receiving a packet, the socket interface asks a
> parser factory for a parser associated with that
> packet (determined by message ID, sequence of bytes,
> etc.). The socket interface then sends the parser the
> packet and a reference to the socket interface itself
> so that the parser can respond to the server easily.
> The parser then modifies the necessary models and
> views as would a controller. There are many models
> and views it has to modify, so I am making facades
> for each parser so that it does not have to know
> about a multitude of models and views. For instance,
> I don't want my model parser to have to know about
> all the channel lists, friends lists, and games going
> on. If I was, then the GUI objects would have to
> register themselves somehow with the parser.
> Instead I am registering facades with each parser
> and then registering the parser with the parser
> factory.
Sounds like "small boy with a pattern" syndrome to me.
> As for how the client events generate packets, I have
> a packet builder class that returns packets to send
> given an identifying string. I may give the packet
> builder relevant information for each packet so that
> my GUI objects, which listen to the events, don't
> have to know about how to set up a packet. An example
> of this is sending a text message -- the chat window
> gui object asks the packet builder for a text message
> packet sending the identifier "txt msg" and the
> message itself as part of the relevant information.
> The builder returns a packet to the chat window and
> the chat window tells the packet to send itself. The
> packet has a reference to the socket interface, and
> so it sends itself to the socket interface. In its
> constructor it intiates its data.
Why did you have to invent your own protocol? Why weren't any of the standard ones good enough?
> Is this a flexible plan that I can easily work with?
> I know it's not a full design, but am I headed in the
> right direction? I also would like to know if I am
> overcomplicating things -- for instance, should the
> GUI objects just know about the socket interface and
> the packets and send the packets to the socket
> interfaces themselves, depending on the
> user-generated event?
Sounds overly complex to me. I don't think this is the right direction, but then I'll admit that I've only thought about it for five minutes.
> And can somebody please
> respond? I am a high school student who does not take
> a computer class and has no one to critique his
> designs.
Not even a teacher or professor?
> Thus, I could be headed in the wrong
> direction and walk miles down that path before even
> knowing it, wasting a lot of my time. If you want me
> to be more specific, I'll be happy to post more
> details.
This is a lot to ask of a volunteer forum. We don't normally do full design or code reviews. It's more about specific problems about code you've written.
I'd wonder if RMI wouldn't be a better choice here. The protocol is sorted out for you - RMI serializes and deserializes objects. You can write a RemoteObserver/RemoteObservable implementation to broadcast messages when they come in.
Sounds to me like you deserve a lot of credit for thinking hard about it and trying to do the right thing. (A full UML design is ambitious for anyone, let along a high school student.)
I'll ask how many other systems you've written to date? This is an ambitious project if it's your first.
I'd also ask if you are aware of Spring:
http://www.springframework.org
It's a framework that might help you think about how to design and structure an application.
%
duffymo, thanks a lot for responding. I've realized that one of the main problems I'm having is that I have many objects that have to know about many other objects. I have to pass references around the whole program, and am hesitant to go static- or Singleton-crazy. For instance, I am dividing up the parsing among many parsers. I have thought about making a ModelsParser which modifies databases based on packets received from the server (example: the "users online" list). I have many different models that the parser has to know about, however, such as all of my ChannelList s and my FriendsList and my UserSettings. And lets say I create a new Channel or a Game... I have to register it with the parser somehow. I want my GUI objects to be able to forget about my parsers though. So one solution is making "package" objects: a channel list database and a "currently playing games models" database, etc. so that I can pass references of these databases to my parser and only modify these databases when a new channel is created. Is this a good solution, or should I go with singletons / static / other?
>> Sounds like "small boy with a pattern" syndrome to me
Hahaha.
>> Not even a teacher or professor?
Not even.
>> I'd also ask if you are aware of Spring:
I wasn't. I looked at it and it looks like it will take a long time to learn and become accustomed to =p However, you didn't say use, you said that it will help me structure my programs. So I'll look into it.
>> Why did you have to invent your own protocol? Why weren't any of the standard ones good enough?
I have to distinguish between different packets. For example, I have to distinguish between a text message and a notification from the server that a user logged off. And my needs are specific to my program... so I don't really understand what you're saying. Unless you're talking about classes for object serialization and deserialization, but that's what I'm doing.
>> Sounds overly complex to me. I don't think this is the right direction, but then I'll admit that I've only thought about it for five minutes.
It sounds overly complex to me as well. I think I am going to forget about having a packet builder and having packets send themselves, and just have GUI objects simply instantiate them and pass them to my socket interface.
>> I'll ask how many other systems you've written to date? This is an ambitious project if it's your first.
I've been programming for 1.5 years. I've done other projects and written instant messenging programs and client emulators before. I recently made an IM program that had multi-channel messaging and friends list as its features. This new one is way more expansive, and I hope to produce better source code.
>>You're doing everything with raw sockets?
Well, if you mean using the Socket class, then yes, I embed Socket s in IO facades. I made a blocking IO package to satisfy the design of my IO. I explained some of this in my top post, but I'll go into more detail.
I have a BufferedSocket class which writes out packets (IBuffer s) and reads in packets (deserializing into IBuffer s). How does it know how to read in a packet? I supply it with an IMessageDetails (I denotes interface) object that knows about the header protocol of a packet. The BufferedSocket asks it for the header length (which remains constant in all packets of my program), reads in the header, sends the header to the IMessageDetails, and the IMessageDetails returns the message length.
After reading in an IBuffer, the BufferedSocket asks the IParserFactory for an IParser, and tells the IParser to parse the packet, giving the IParser a reference to the BufferedSocket itself (in case the IParser needs to make an immediate response to the server).
I provided implementations for the IBufferedSocket and IBuffer interfaces. You may think I'm going interface-crazy, but I think that it's important for some one to be able to use alternate implementations of an interface. The Buffer class is not worth posting, but I'll post the BufferedSocket class.
package abio; // Abstract Blocking IO
import java.net.Socket;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.LinkedList;
/**
* A class that writes out and reads in IBuffers
*/
/**
* BufferedSocketManager.java
* @author ktm5124
* Created on Mar 24, 2006 at 10:10:20 PM
*/
public class BufferedSocket
{
// protocol and parsing implementation
private IMessageDetails msgdetails;
private IParserFactory pfact;
// socket information
private Socket sock;
private InputStream in;
private OutputStream out;
private String host;
private int port;
// in and out queues
private LinkedList<IBuffer> outBuffers;
private LinkedList<IBuffer> inBuffers;
// send and receive threads
private Thread sendThread;
private Thread recvThread;
// flags
public int FLAGS;
public static int CONNECTED = 0x00000001;
public static int LOGGED_ON = 0x00000002;
/**
* Default constructor
*/
public BufferedSocket()
{
inBuffers = new LinkedList<IBuffer>();
outBuffers = new LinkedList<IBuffer>();
}
/**
* Creates a BufferedSocketManager with the specified IMessageDetails and IParserFactory
* @param m
* The IMessageDetails reference
* @param p
* The IParserFactory reference
*/
public BufferedSocket(IMessageDetails m, IParserFactory p)
{
this();
msgdetails = m;
pfact = p;
}
/**
* Creates a BufferedSocketManager and connects it upon construction
* @param host
* @param port
* @param readyToConnect
* @param m
* @param p
*/
public BufferedSocket(String host, int port, boolean readyToConnect, IMessageDetails m, IParserFactory p)
{
this(m, p);
if (readyToConnect)
connect(host, port);
else
setSocketInfo(host, port);
}
/**
* Registers a new IMessageDetails
* @param m
*/
public void registerIMessageDetails(IMessageDetails m)
{
msgdetails = m;
}
/**
* Registers a new IParserFactory
* @param p
*/
public void registerIParserFactory(IParserFactory p)
{
pfact = p;
}
/**
*Reads a buffer from the socket into the in queue
*/
private void doReceive()
{
boolean running = true;
while (running)
{
// read in header
int headerLen = msgdetails.headerLength();
byte[] header = new byte[headerLen];
try {
in.read(header);
} catch (IOException ioe) {
ioe.printStackTrace();
}
// read in rest of message
int msgLen = msgdetails.messageLengthFromHeader(header);
byte[] msg = new byte[msgLen]; // includes header
System.arraycopy(header, 0, msgLen, 0, headerLen);
try {
in.read(msg, headerLen, msgLen - headerLen);
} catch (IOException ioe) {
ioe.printStackTrace();
}
// get parser and parse message
IParser p = pfact.getParser(header);
p.parse(msg, this);
// naptime
try {
Thread.sleep(100);
} catch (InterruptedException ie) {
ie.printStackTrace();
}
}
}
/**
* Sends a buffer from the out queue to the server
*/
private void doSend()
{
boolean running = true;
while (running)
{
if (outBuffers.size() > 0)
{
// write out buffer in out queue
IBuffer outBuffer = outBuffers.remove();
try {
out.write(outBuffer.getBuffer());
} catch (IOException ioe) {
ioe.printStackTrace();
}
}
// naptime
try {
Thread.sleep(100);
} catch (InterruptedException ie) {
ie.printStackTrace();
}
}
}
/**
* Creates a socket with the specified hostname and port
* @param host
*The hostname of the server
* @param port
*The port to which data is directed
*/
public void connect(String host, int port)
{
setSocketInfo(host, port);
try {
sock = new Socket(host, port);
} catch (IOException ioe) {
ioe.printStackTrace();
}
// set connect flag
FLAGS |= CONNECTED;
// create threads, override Runnable interface
sendThread = new Thread(new Runnable() {
public void run()
{
doSend();
}
});
recvThread = new Thread(new Runnable() {
public void run()
{
doReceive();
}
});
// start up threads, get wheels moving
sendThread.start();
recvThread.start();
}
/**
* Disconnects from the socket
*/
public void disconnect()
{
try {
sock.close();
} catch (IOException ioe) {
ioe.printStackTrace();
}
sock = null;
sendThread = null;
recvThread = null;
// clear connect flag
FLAGS &= ~CONNECTED;
}
/**
* Sets the socket information without connecting
* @param host
*The hostname of the server
* @param port
*The port to which data is directed
*/
public void setSocketInfo(String host, int port)
{
this.host = host;
this.port = port;
}
/**
* Sends a buffer along the connection
* @param b
*/
public void send(IBuffer b)
{
outBuffers.add(b);
}
/**
* @return
* Returns the next buffer in the in queue
*/
public IBuffer getNextInBuffer()
{
return inBuffers.remove();
}
/**
* @return
* Returns the next buffer in the out queue
*/
public IBuffer getNextOutBuffer()
{
return outBuffers.remove();
}
/**
* @return
* Returns all of the in buffers
*/
public IBuffer[] getAllInBuffers()
{
return inBuffers.toArray(new IBuffer[inBuffers.size()]);
}
/**
* @return
* Returns all of the out buffers
*/
public IBuffer[] getAllOutBuffers()
{
return outBuffers.toArray(new IBuffer[outBuffers.size()]);
}
/**
* @return
* Returns the size of the in queue
*/
public int numInBuffers()
{
return inBuffers.size();
}
/**
* @return
* Returns the size of the out queue
*/
public int numOutBuffers()
{
return outBuffers.size();
}
}
> Is this a flexible plan that I can easily work with?
> I know it's not a full design, but am I headed in the
> right direction? I also would like to know if I am
> overcomplicating things -- for instance, should the
> GUI objects just know about the socket interface and
> the packets and send the packets to the socket
> interfaces themselves, depending on the
> user-generated event?
In a word no. But don't get ahead of yourself.
Sort of along the same lines as duffymo, I would suggest that you not bite off more than you can chew. My general feeling about this is that your heart is in the right place but you need to pace yourself.I'm very certain that experience is the number one indicator of whether a project will succeed and if we are to believe your self-description, you don't have much.
From my own experience and what I've seen in others' work, many people who are very eager tend to be overly ambitious in their projects. They also tend to think of them as a monolithic piece of software which is something experience teaches us to avoid.
Think how you can break down what you want to do into smaller libraries of applications. Then look around for things that you can use for these pieces and use them if possible. This will give you a much better chance of success.
