patterncsharpMinor
Lattitude document organizer
Viewed 0 times
lattitudeorganizerdocument
Problem
I'm not familiar with threads but I am not against them either. A bit of background the original program would have taken about 2 years to run. I had modified it and knocked down that time to around 40 days.
Can I make it even faster by using threads or updating my code?
The reason it is 40 days is because it has to navigate through a directory of about 6 million files. That amount of files slows this program down.
```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data.SqlClient;
using System.IO;
using System.Data;
using System.Threading;
namespace LattitudeDocumentationOrganizer
{
class Program
{
static string connString = LattitudeDocumentationOrganizer.Properties.Settings.Default.ConnectionString;
static void Main(string[] args)
{
string fileName;
Guid documentID;
string location;
DateTime createdDate;
bool runLoop = true;
int x = 0;
string msg;
int fileCount = 0;
int errorCount = 0;
int number = 0;
while (runLoop)
{
Console.Clear();
// reset variables
fileName = "";
documentID = Guid.Empty;
location = "";
using (SqlConnection connection = new SqlConnection(connString))
{
connection.Open();
// Run through
string sql = @"select top 1 d.UID, d.CreatedDate, d.Location, m.number from master m
inner join documentation_attachments da
on m.number = da.accountid
inner join documentation d
on da.documentid = d.uid
where m.qlevel in (998,999)
and d.lo
Can I make it even faster by using threads or updating my code?
The reason it is 40 days is because it has to navigate through a directory of about 6 million files. That amount of files slows this program down.
```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data.SqlClient;
using System.IO;
using System.Data;
using System.Threading;
namespace LattitudeDocumentationOrganizer
{
class Program
{
static string connString = LattitudeDocumentationOrganizer.Properties.Settings.Default.ConnectionString;
static void Main(string[] args)
{
string fileName;
Guid documentID;
string location;
DateTime createdDate;
bool runLoop = true;
int x = 0;
string msg;
int fileCount = 0;
int errorCount = 0;
int number = 0;
while (runLoop)
{
Console.Clear();
// reset variables
fileName = "";
documentID = Guid.Empty;
location = "";
using (SqlConnection connection = new SqlConnection(connString))
{
connection.Open();
// Run through
string sql = @"select top 1 d.UID, d.CreatedDate, d.Location, m.number from master m
inner join documentation_attachments da
on m.number = da.accountid
inner join documentation d
on da.documentid = d.uid
where m.qlevel in (998,999)
and d.lo
Solution
I see a number of things in this code. I'll address all of them, then get to multi-threading. Keep in mind, I'm writing these down as I make changes in the code, so I might suggest something, then change that suggestion.
I will get to multithreading when I found out what version of .Net you are using :)
- I like your variable names. Very descriptive and I understand what information they are holding. The only exception is
connString, I think it should be make readonly and changed to 'ConnString' indicating a private static readonly variable.
- The declarations of the variables used in the function should be moved closer to where they are being used. This does a couple of things: makes the declaration a little more clear to any other developers, and it ensures the value hasn't been unintentionally altered between declaration and usage.
- Learn about
var. I find that using it really cleans up extra code that just causes clutter.
- I would make the declaration of
sqlconst, and move it to be a class variable. At the same time, it should be renamedSelectFileInformationSql
- Good job with the using
using.
- The
runLoopvariable is set in places it is never needed because of thebreakstatements just below it. You can remove this variable, and replace the main loop to awhile(true)
- The
xvariable is unused, remove it.
- The variable
rootDirshould be declaredconst, moved to a class variable and renamedRootDir
- The second declaration of
sqlshould be declaredconst, moved to a class varible and renamedUpdateDocumentLocationSql
- The thrid declaration of
sqlshould be declaredconst, moved to a class varible and renamedLogIssueSql
- I would like to see document moved out into its own class. This way you can do things like
FilenameAndPath = Path.Combine(Location, Filename)only once, instead of 2-3 times in the original code. It also packages all the information needed for processing that file into one nice package.
- I find the
msgvariable to be unneeded. You can just put the string right in the function call.
I will get to multithreading when I found out what version of .Net you are using :)
Context
StackExchange Code Review Q#32389, answer score: 9
Revisions (0)
No revisions yet.