Re: My First C# (warning - long post)

On Jan 31, 1:00 am, LX-i <lxi0...@xxxxxxxxxxxx> wrote:
Here 'tis. It looks like there's a little word-wrapping going on - I
wasn't really attentive to ensuring that I didn't go past 80 on some lines.

What this does...
- Counts lines of code, along with "executable" and "commented" lines
- Creates a cross-reference between programs and procs (copybooks).
All our procs are named Annn-something (where A is a letter and nnn is a
three-digit number). For programs, we parse copy statements, and for
procs, we parse perform statements. (This gives us the ability, from
our CM system, to generate a list of proc dependencies.)
- Creates a cross-reference between programs/procs and reject codes.
The system we support uses a table of reject codes with associate
narratives. When a code is changed, all program owners who are affected
must sign off. This xref enables that, as well as finding rejects that
are not used by any programs - these numbers can be reused.
- Creates a cross-references between programs/procs and called
procedures. It can be used for many different things.
- [TODO] Creates a cross-reference between programs/procs and database
records/tables and fields/columns. (This is the missing "cull" stuff.)

Some notes as well...
- the "cull" stuff isn't done yet, so if the method has "cull" in the
name, ignore it. (There are a few utility methods under the cull stuff.)
- I've replaced our server names and passwords with placeholders, for
obvious reasons.
- I really, REALLY, *REALLY* dislike the auto-formatting that VS2005
does with putting the braces on the line under the declaration.
However, that's our shop standard, so that's what this looks like.
- I've tried to comment things as I go along, but this isn't a
finished product. If you see a spot where more comments would be
helpful, feel free to identify them to me...
- Having to have a "break" after the "default" case on a switch is
BS... but I digress...

Here we go - 824 lines....



Snipped code....

(Take these comments as good will ones, not character destroying..etc)

Ok so a quick scan through tells me that this class....

Whilst 'doing' one job of analysing the code files, is actually
several (currently) hidden classes working together. This is backed up
by the large number of IFs Switch statements, loops and repeated code.

The style of coding is more procedural than OO. This is evident when
we have large methods, containing multple 'if' blocks or/and 'Switch'
statements.

A general guideline to use for OO methods, is to keep their size to no
more than 10 lines (including whitespace, empty lines etc).

Lots of string literals embedded within the code, making the
maintenance of the program difficult. - These strings (esp db ones)
are better moved into a property file or constants.

However, all is not lost... I'll try and briefly show how using a
series of small changes, we can kepp your code compiling, running and
working, whilst slowly changing the design to be OO instead of
Procedural....

So lets start with the large methods... within your class there are
numerous occurrences of code duplication...

for example, several places you create an SQLCommand, insert some data
then close and dispose:

SqlCommand scCheck = new SqlCommand("....
... dbConn);

SqlDataReader drCheck = scCheck.ExecuteReader();
if (!drCheck.HasRows)
{ // It's not there - insert it
executeSQL("...");
}
drCheck.Close();
drCheck.Dispose();
scCheck.Dispose();

If you moved (aka Refactored) this chunk of code into its own method,
and then did the same with all other occurrences of similar code,
you'd soon see that there are numerous small private methods doing the
same basic job, just with different data.

Your IDE will even do the extraction for you.... highlight some code
with the mouse and right click, somewhere there will be a 'refactor'
menu option with something like 'extract method...' on it.

Once we have these small methods , we can then remove all of them but
One, and Pass It The Data to use... we do this, one method at a
time.

Another useful technique is to change inline comments into the names
of private methods, and move the code there...

time for another example...

// Update DML records in cull
protected void updateCullRecord(String ucrLine)
{

// --- CHECK #1 --- record name //
checkRecordName(...)

// --- CHECK #2 --- set name //
checkSetName(...)

}


Once again, select the code blocks to extract( the code between each
comment) and use your IDEs refactoring tool to extract method...

Once you have broken your methods down into sizes of no more than 10
lines, you will probably have 10, 20 or more little methods working
together to do the job as it currently does.

Now, the next step is to find the groupings of these methods that
naturally fit into their own class.

So for example.....

All of the db related methods are retrieving/inserting data, but
actually don't need to be part of the CodeStatistics class. Indeed,
moving those db related methods into their own class called
StatisticsModel? StatisticsDataSource? StatisticsGateway? (Gateway
is a design pattern...) means the Responsibility of getting and
updating the db is now in its own class, and that new class has only
one Responsibility.

time for a break....(aka kids playing up... ;-) )

Andrew

.