Return to Revisions

##Instantiate Objects Once, and early##

• Move declarations to the top, above any while or using blocks
• Move assignment of constants to the top.

Here are some: .

const string Sql = @"select top 1 d.UID, d.CreatedDate, d.Location, m.number from master m with (NOLOCK)
                                inner join documentation_attachments da with (NOLOCK)
                                    on m.number = da.accountid
                                inner join documentation d with (NOLOCK)
                                    on da.documentid = d.uid
                                where m.qlevel = 999
                                and d.location is not null
                                and uid not in (select documentid from JM_DocumentationIssues)
                                order by m.number desc";

.

string msg;

.

const string RootDir = @"\\192.168.22.23";

.

const string Sql =
            @"insert into JM_DocumentationIssues (documentid, oldLocation, newLocation, errorMessage, dateAdded, number)
                        values (@id, @prevLocation, @newLocation, @msg, GETDATE(), @number)";

.

const string Sql = "update documentation set location = @newLocation where uid = @id";

##System.Data.SqlClient objects are composites. Instantiate the parts at the top and then compose them as needed##

• This means you will not be "newing-up" objects in the using block parameter. But it does not mean you can't use using.
• If you need multiple SqlConnection objects, SqlConnection implements ICloneable

.

SqlConnection oneConnToRuleThemAll = new SqlConnection(connString);
SqlCommand command = new SqlCommand(oneConnToRuleThemAll);
SqlCommand updateCommand = CreateUpdateCommand(oneConnToRuleThemAll);

// if we swap out parameters for a command, make individual SqlParameter collections
SqlParameterCollection updateParameters = new SqlParameterCollection() {
    new SqlParameter( "id", SqlDbType.UniqueIdentifier ),
    new SqlParameter( "prevLocation", SqlDbType.VarChar, 260 )
};
SqlCommand logErrorCommand = CreateLogErrorCommand(oneConnToRuleThemAll);

Then

using ( SqlCommand thisCommand = command  )
using ( SqlCommand updateCommand = updateCommand )
using ( SqlCommand logErrorCommand = logErrorCommand )

##String Theory##

Turn multiple WriteLines into a single write:

 Console.WriteLine( "Processed {0} files successfully.", fileCount );
 Console.WriteLine( "Did not process {0} files successfully.", errorCount );
 Console.WriteLine( "No more files were found with the current query" );

into this:

// and do this "at the top"!
string processNotice = "Processed {0} files successfully.\n" +
                       "Did not process {1} files successfully.\n" +
                       "Did not process {0} files successfully.";

 // and use it thus:
 Console.Writeline(processNotice,fileCount, errorCount);

##One more thing(s)##

• Open the connection immediately before you execute the command. Close it ASAP after reading.
• Make a struct rather than separate variables - documentID, fileName,location,createDate, number. Keeps the record association. Will simplify method parameters. Makes sense.
• Remove ExecuteNonQuery from CreateLogErrorCommand(), UpdateDocument(), etc. Keep configuration separate from execution. This will help with refactoring this code monstrosity.