User:LukeRobinson/Design study
LukeRobinson (Talk | contribs) |
LukeRobinson (Talk | contribs) (→Design Critique) |
||
Line 123: | Line 123: | ||
== Design Critique == | == Design Critique == | ||
− | * As seen in the initial UML diagram, I have used poor [[User:LukeRobinson#Other | naming style]]. Class, variable and method names are poor in other regards as well, names need to be changed so that they better reflect their purpose. [[ | + | * As seen in the initial UML diagram, I have used poor [[User:LukeRobinson#Other | naming style]]. Class, variable and method names are poor in other regards as well, names need to be changed so that they better reflect their purpose. [[Image:tick.gif|thumb|alt=Alt text|Caption]] |
* "Parser" should really be an interface rather than an abstract class. Not sure that this problem is called, still looking for the name. | * "Parser" should really be an interface rather than an abstract class. Not sure that this problem is called, still looking for the name. |
Revision as of 22:24, 2 August 2010
Contents |
Project
Introduction
I am doing my assignment on my Honors project, which is a program to visualize network data. So far, I have already created quite a bit of the program, although it is not finished.
Background
The goal of the project is to display network logs in a simple way so the people with little training can get an understanding of whats happening in the network. We want to help people both identify possible threats and better understand the normal flow of network usage. The network logs I am using come from a small network Bob Ward takes care of here in Christchurch, he has given us access to anonymized logs, I currently have just over one month of logs, which amount to a few hundred megabytes. Here is an example network packet log:
Time | protocol | size | source ip | source port | destination ip | destination port | packet type |
---|---|---|---|---|---|---|---|
1269687620.676725 | IP | 48 | 192.168.100.6 | 4212 | 192.168.83.37 | 9101 | TCP |
1269687625.489346 | IP | 48 | 192.168.100.6 | 4213 | 192.168.110.12 | 9100 | TCP |
1269687632.684662 | IP | 328 | 192.168.109.26 | 68 | 192.168.99.1 | 67 | UDP |
So basically the program needs to read data from the log files, parse the packet records, and present them to the user by why of the graph as desired.
Design Study
Requirements
There are two major requirements that this program must meet. The first is speed, because the user will need to interact with the interface, this means that it cannot lag while reading or parsing the large log files.
The second is extensibility, the program needs to be extensible in three different ways.
The first way in which it needs to be extensible to allow new data types. This will allow it to support new firewalls and packet formats.
The program will also need to be extensible to allow new display types. Currently the program only displays information in a graph, but we may want to add new displays.
Finally, because this is my Hons project, I will only be working on it until the end of this year, therefore, afterward others may want to change and extend it so its needs to allow such changes and be easily understood by others.
Constraints
The only really constraints are speed, as mentioned before, and that the program is written in C#.
Initial Design
When I initially wrote the program, I did try to make it extensible to allow new packet formats. However, I did not make it extensible in other areas. Although I did try to keep good OOD in mind, the program has several poor design issues.
UML Diagram
This UML class diagram on 16/7/10, it is the beginning state of the project. I will make changes to it an update this page as I go.
Here is the UML class diagram with SOME changes, there is still much work to do on it yet.
In this copy I have fixed most of the poor naming problems, but I probably missed a few.
Description of Classes
- FileReader
As the name suggests, this is an object that reads data from a file, it is a spectral kind of reader that can asynchronously read packet records backward or forward from the current position in a CSV log file.
- Graph
A graph is the actual object that displays the pretty pictures, it has a bunch of controls and internal objects but we don't need to worry about them here.
- GUI
The overall GUI, contains a graph and controls.
- Model
main control center of the program, controls the parsers etc., is sent commends from the GUI.
- ListWithChangedEvent<T>
C# "List" but with change events added.
- NetInterface
This object represents an interface on a firewall or other networking device.
- DataSource
A currently open CSV log file, can only read forward and is synchronous.
- Packet
Basic packet object, holds all data for each packet recorded.
- Parser
Abstract parser, has all abstract methods which are overridden in subclasses, lets higher ups relay on a standard interface.
- CSVParser
A real parser, overrides abstract methods from "Parser", reads data using one of the file readers and creates packets, also gives overall stats about number of packets etc.
- PaserBuffer
Acts a buffer between the parser and the "Model", was included so that the program could cope with real time data if needed in the future.
- EventListener
Inner class of ListWithChangedEvent, used to listen for changes to the list.
- ChangedEventHandler
Also an inner class of ListWithChangedEvent, handles said events.
Design Critique
- As seen in the initial UML diagram, I have used poor naming style. Class, variable and method names are poor in other regards as well, names need to be changed so that they better reflect their purpose.
- "Parser" should really be an interface rather than an abstract class. Not sure that this problem is called, still looking for the name.
- Very poor encapsulation is shown with access to the "Graph", the "GUI" should be the only class with access to "Graph" and provide public methods to other classes such as "Model" that want to update the "Graph".
- To better allow extensibility, "Packet" should really be a base class which can be extended as needed to support new packet types. In its current state as a potential base class, it violates the "avoid concrete base classes" principal.
- The "Packet" class has a very strong "data class smell", this should be looked at, the NetInterface also has this issue.
- Almost all of the fields of classes are public, this is very bad, it breaks the "Information Hiding" principal.
Final Design
Design Improvements
The first improvement, a very minor one, renamed classes so that they all start with uppercase letters.
Changed names to improve their understandability, (almost) all names are now ok.
Files
When this is finished I'll post a zip so you can have a go with the program if you want.