Reputation: 77
In my code I have 3 sections, but in 2 of the 3 sections the code is almost identical, I was wondering if anyone could help me structure them properly so that I would only need one line written once, in a scope so that it can be seen by both sections so I don't need to have so much code. (Framework 3.5)
public static void FakeDriveInfo()
{
List<DriveInfo> driveList = DriveInfo.GetDrives().Where(x => x.IsReady).ToList<DriveInfo>();
Server server = new Server();
Console.WriteLine();
Console.ForegroundColor = ConsoleColor.White;
Console.WriteLine("Server ID : {0}", server.ServerID = 0);
Console.WriteLine("Server Name : {0}", server.ServerName = string.Concat(System.Environment.MachineName));
Console.WriteLine();
for (int i = 0; i < driveList.Count; i++)
{
ServerDrive serverDrives = new ServerDrive();
Console.WriteLine("Drive Letter: {0}", driveList[i].Name);
Console.WriteLine("Total Size: {0}", FormatBytes(driveList[i].TotalSize));
Console.WriteLine("Volume Label: {0}", driveList[i].VolumeLabel);
Console.WriteLine("Free Space: {0}", FormatBytes(driveList[i].TotalFreeSpace));
Console.WriteLine("Drive Format: {0}", driveList[i].DriveFormat);
Console.ReadLine();
}
}
public static void RealDriveInfo()
{
//Create the server object - You will need create a list of the server objects.
Server server = new Server();
//Get all drives information
List<DriveInfo> driveList = DriveInfo.GetDrives().Where(x => x.IsReady).ToList<DriveInfo>();
//Insert information of one server - You will need get information of all servers
server.ServerID = 0; //Here is necessery put PK key. I recommend doing the SQL server will automatically generate the PK.
server.ServerName = string.Concat(System.Environment.MachineName);
//Inserts information in the newServers object
for (int i = 0; i < driveList.Count; i++)
{
ServerDrive serverDrives = new ServerDrive();
//Put here all the information to obeject Server
serverDrives.DriveLetter = driveList[i].Name;
serverDrives.TotalSpace = driveList[i].TotalSize;
serverDrives.DriveLabel = driveList[i].VolumeLabel;
serverDrives.FreeSpace = driveList[i].TotalFreeSpace;
serverDrives.DriveType = driveList[i].DriveFormat;
// server.ListServerDrives.Add(serverDrives);
server.ServerDrives.Add(serverDrives);
}
//Add the information to an SQL Database using Linq.
DataClasses1DataContext db = new DataClasses1DataContext(@"sqlserver");
// db.Servers.InsertAllOnSubmit(server);
db.Servers.InsertOnSubmit(server);
db.SubmitChanges();
}
What I want to do is move the SQL to Linq part at the bottom of the code into it's own section. But to do that I have to have the whole RealDriveInfo
Section too..
Another part that I want to do is make the List<DriveInfo> driveList = DriveInfo.GetDrives().Where(x=>x.IsReady).ToList<DriveInfo>();
so that it can be seen by FakeDriveInfo and RealDriveInfo..
Any feedback would be greatly appreciated, thanks.
EDIT : At the moment I am calling two methods, FakeDriveInfo(); = Executing the console app and showing me the info it is going to submit. Name, Letter, Label, Server Name, ID, etc. RealDriveInfo(); = Connecting to the SQL Server and inserting the information into the two tables.
I want to have a third method WriteInToDB(); = The DB Writing code would be taken from the RealDriveInfo Method and moved here instead.
At the moment I have two methods that practically look identical due to the scope. I want the scope shifted so that I can put the List part of the code into the Main() Method and both FakeDriveInfo and RealDriveInfo can use it from there instead of having the code duplicated.
Hopefully this adds a bit more sense to it all :)
Upvotes: 0
Views: 257
Reputation: 77
After pain-stakeingly realising, the answer was in method and class scope..
Just incase anyone would like to see what I did ::
public class Program
{
List<DriveInfo> driveList = DriveInfo.GetDrives().Where(x => x.IsReady).ToList<DriveInfo>(); //Get all the drive info
Server server = new Server(); //Create the server object
ServerDrive serverDrives = new ServerDrive();
public void Main()
{
Program c = new Program();
c.FakeDriveInfo();
c.RealDriveInfo();
c.WriteInToDB();
}
public static string FormatBytes(long bytes)
{
const int scale = 1024;
string[] orders = new string[] { "GB", "MB", "KB", "Bytes" };
long max = (long)Math.Pow(scale, orders.Length - 1);
foreach (string order in orders)
{
if (bytes > max)
{
return string.Format("{0:##.##} {1}", Decimal.Divide(bytes, max), order);
}
max /= scale;
}
return "0 Bytes";
}
public void FakeDriveInfo()
{
Console.WriteLine();
Console.ForegroundColor = ConsoleColor.White;
Console.WriteLine("Server ID : {0}", server.ServerID = 0);
Console.WriteLine("Server Name : {0}", server.ServerName = string.Concat(System.Environment.MachineName));
Console.WriteLine();
for (int i = 0; i < driveList.Count; i++)
{
Console.WriteLine("Drive Letter: {0}", driveList[i].Name);
Console.WriteLine("Total Size: {0}", FormatBytes(driveList[i].TotalSize));
Console.WriteLine("Volume Label: {0}", driveList[i].VolumeLabel);
Console.WriteLine("Free Space: {0}", FormatBytes(driveList[i].TotalFreeSpace));
Console.WriteLine("Drive Format: {0}", driveList[i].DriveFormat);
Console.ReadLine();
}
}
public void RealDriveInfo()
{
//Insert information of one server - You will need get information of all servers
server.ServerID = 0; //Here is necessery put PK key. I recommend doing the SQL server will automatically generate the PK.
server.ServerName = string.Concat(System.Environment.MachineName);
//Inserts information in the newServers object
for (int i = 0; i < driveList.Count; i++)
{
//Put here all the information to obeject Server
serverDrives.DriveLetter = driveList[i].Name;
serverDrives.TotalSpace = driveList[i].TotalSize;
serverDrives.DriveLabel = driveList[i].VolumeLabel;
serverDrives.FreeSpace = driveList[i].TotalFreeSpace;
serverDrives.DriveType = driveList[i].DriveFormat;
server.ServerDrives.Add(serverDrives);
}
}
public void WriteInToDB()
{
//Add the information to an SQL Database using Linq.
DataClasses1DataContext db = new DataClasses1DataContext(@"cspsqldev");
// db.Servers.InsertAllOnSubmit(server);
db.Servers.InsertOnSubmit(server);
db.SubmitChanges();
}
Simply by moving the parts I wanted to be seen by all methods, into the Class scope it now works.
Thanks for your feedback guys :)
Upvotes: 0
Reputation: 51634
Create a method:
public static List<DriveInfo> GetDrives {
return DriveInfo.GetDrives().Where(x => x.IsReady).ToList<DriveInfo>();
}
Call it in both places:
public static void FakeDriveInfo()
{
List<DriveInfo> driveList = GetDrives();
// ...
}
public static void RealDriveInfo()
{
List<DriveInfo> driveList = GetDrives();
// ....
}
This is one of the basic Refactorings. It's called Extract Method and has been described by Martin Fowler over here.
Upvotes: 2
Reputation: 10645
I've taken the approach of creating a new method, ProcessDriveInfo
, which is called by your two original methods. These two methods fetched a list of drives, created a server, did something with the server, and did something for each drive. I moved the first two steps into the ProcessDriveInfo
, and passed in actions for the last two steps.
The first action (i.e. method) does something with the server, and the second action does something for a single driveInfo
.
n.b. I've "answered" the question in that it removes the duplication, but it could be argued that it is less readable.
public Server ProcessDriveInfo(Action<Server> initialAction, Action<Server, DriveInfo> driveInfoAction)
{
var driveList = DriveInfo.GetDrives().Where(x => x.IsReady).ToList();
var server = new Server();
initialAction(server);
driveList.ForEach(dl => driveInfoAction(server, dl));
return server;
}
public void FakeDriveInfo()
{
ProcessDriveInfo(WriteServerToConsole, WriteDriveInfoToConsole);
}
private void WriteServerToConsole(Server server)
{
Console.WriteLine();
Console.ForegroundColor = ConsoleColor.White;
Console.WriteLine("Server ID : {0}", server.ServerID = 0);
Console.WriteLine("Server Name : {0}", server.ServerName = string.Concat(System.Environment.MachineName));
Console.WriteLine();
}
private void WriteDriveInfoToConsole(Server server, DriveInfo t)
{
Console.WriteLine("Drive Letter: {0}", t.Name);
Console.WriteLine("Total Size: {0}", FormatBytes(t.TotalSize));
Console.WriteLine("Volume Label: {0}", t.VolumeLabel);
Console.WriteLine("Free Space: {0}", FormatBytes(t.TotalFreeSpace));
Console.WriteLine("Drive Format: {0}", t.DriveFormat);
Console.ReadLine();
}
public void RealDriveInfo()
{
var server = ProcessDriveInfo(InitialiseServer, WriteDriveInfoToServer);
//Add the information to an SQL Database using Linq.
var db = new DataClasses1DataContext(@"sqlserver");
// db.Servers.InsertAllOnSubmit(server);
db.Servers.InsertOnSubmit(server);
db.SubmitChanges();
}
private static void InitialiseServer(Server server)
{
// Insert information of one server - You will need get information of all servers
server.ServerID = 0;
// Here is necessery put PK key. I recommend doing the SQL server will automatically generate the PK.
server.ServerName = Environment.MachineName;
}
private static void WriteDriveInfoToServer(Server server, DriveInfo t)
{
var serverDrives = new ServerDrive
{
DriveLetter = t.Name,
TotalSpace = t.TotalSize,
DriveLabel = t.VolumeLabel,
FreeSpace = t.TotalFreeSpace,
DriveType = t.DriveFormat
};
server.ServerDrives.Add(serverDrives);
}
Upvotes: 1
Reputation: 3518
I hope I captured your functionality correctly.
3 regions, create, output, persist. You can ofcourse take each section and create reusable methods. You can move the output section to the ToString()
method of Server
.
#region create graph
var server = new Server()
{
ServerID = 0,
ServerName = string.Concat(Environment.MachineName),
ServerDrives = DriveInfo.GetDrives()
.Where(x => x.IsReady)
.Select(di => new ServerDrive()
{
DriveLetter = di.Name,
TotalSpace = di.TotalSize,
DriveLabel = di.VolumeLabel,
FreeSpace = di.TotalFreeSpace,
DriveType = di.DriveFormat
})
.ToList()
};
#endregion
#region output graph
Console.WriteLine();
Console.ForegroundColor = ConsoleColor.White;
Console.WriteLine("Server ID : {0}", server.ServerID);
Console.WriteLine("Server Name : {0}", server.ServerName);
Console.WriteLine();
server.ServerDrives.ForEach(sd =>
{
Console.WriteLine("Drive Letter: {0}", sd.DriveLetter);
Console.WriteLine("Total Size: {0}", sd.TotalSpace);
Console.WriteLine("Volume Label: {0}", sd.DriveLabel);
Console.WriteLine("Free Space: {0}", sd.FreeSpace);
Console.WriteLine("Drive Format: {0}", sd.DriveType);
Console.ReadLine();
});
#endregion
#region persist graph
DataClasses1DataContext db = new DataClasses1DataContext(@"sqlserver");
db.Servers.InsertOnSubmit(server);
db.SubmitChanges();
#endregion
Upvotes: 1