Updating a Sheet

Here’s some code from Jan for you to criticize. If you’ve been working on the ‘choices’ game all morning, you can get Jan back by ripping his code.


Private Sub Worksheet_Activate()
On Error GoTo errhandler

‘ this array determines which row each department has been assigned on the sheet ‘Resumen’
Dim a(1000) As Integer
a(11) = 4: a(12) = 5: a(13) = 6: a(14) = 7: a(15) = 8: a(16) = 9: a(17) = 10: a(101) = 11
a(102) = 12: a(103) = 13: a(104) = 14: a(105) = 15: a(106) = 16: a(107) = 17: a(201) = 18
a(202) = 19: a(203) = 20: a(204) = 21: a(205) = 22: a(206) = 23: a(207) = 24: a(301) = 25
a(302) = 26: a(303) = 27: a(304) = 28: a(305) = 29: a(306) = 30: a(307) = 31: a(401) = 32
a(402) = 33: a(403) = 34: a(404) = 35: a(405) = 36: a(406) = 37: a(407) = 38: a(501) = 39
a(502) = 40: a(503) = 41: a(504) = 42: a(505) = 43: a(506) = 44: a(507) = 45: a(601) = 46
a(602) = 47: a(603) = 48: a(604) = 49: a(605) = 50: a(606) = 51: a(607) = 52: a(701) = 53
a(702) = 54: a(703) = 55: a(704) = 56: a(705) = 57: a(706) = 58: a(707) = 59: a(801) = 60
a(802) = 61: a(803) = 62: a(804) = 63: a(805) = 64: a(806) = 65: a(807) = 66

‘ the current year sits in a cell on the worksheet ‘Control panel’
nyear = Worksheets(“Control panel”).Range(“gyear”)

‘ to remember the sheet names for the twelve months, they are called ‘Julio 04’ etc
Dim b(12) As String
b(1) = “Enero”: b(2) = “Febrero”: b(3) = “Marzo”: b(4) = “Abril”: b(5) = “Mayo”
b(6) = “Junio”: b(7) = “Julio”: b(8) = “Agosto”: b(9) = “Septiembre”: b(10) = “Octubre”
b(11) = “Noviembre”: b(12) = “Diciembre”
For i = 1 To 12
  b(i) = b(i) & ” ” & Right(nyear, 2)
Next

‘ clear the 12 columns with old amounts
Worksheets(“Resumen”).Range(“D4:D66,G4:G66,J4:J66,M4:M66,P4:P66,S4:S66,V4:V66,Y4:Y66,AB4:AB66,AE4:AE66,AH4:AH66,AK4:AK66?).Value = “”

‘ update for all 12 months
For nmonth = 1 To 12
   ‘ the range that shows the month number for a payment in month n is called monthn
  cdummy = “month” + Trim(Str(nmoth))
  ndep = Worksheets(b(nmonth)).Cells(cell.Row, cell.Column – 1).Value
  For Each cell In Worksheets(b(nmonth)).Range(cdummy)
    ‘ only look at payments for the current year and with a department number present
    If cell.Value >= 1 And cell.Value <= 12 _
        And Worksheets(b(nmonth)).Cells(cell.Row, cell.Column + 1).Value = Val(nyear) _
       And ndep <> “” Then
      ‘ find amount paid
      namount = Worksheets(b(nmonth)).Cells(cell.Row, cell.Column + 2).Value _
                + Worksheets(b(nmonth)).Cells(cell.Row, cell.Column + 3).Value
      ‘ add amount paid to the summary sheet called ‘Resumen’
      Worksheets(“Resumen”).Cells(a(ndep), 3 * cell.Value + 1).Value _
        = Worksheets(“Resumen”).Cells(a(ndep), 3 * cell.Value + 1).Value + namount
    End If
  Next cell
Next nmes

Exit Sub

errhandler:
  MsgBox Err.Description
End Sub

I’ll start: Declare your variables!

Posted in Uncategorized

2 thoughts on “Updating a Sheet

  1. [Me again!]

    Sorry Jan, but “Eurgh!”. Where to start?

    Firstly, this doesn’t even compile on my Excel 2002 install: “Next nmes” refers to something that’s not declared, much less a loop control variable. (Was the code changed for presentation?) As Dick points out, judicious application of Option Explicit would help: I’ve just noticed a typo near the end: “nmoth” instead of “nmonth”

    Next: that fabulous a() array: could that not be loaded from a worksheet? Or a declared Array? I note that we seem to have a run of consecutive values from 4 to 66, how about

    Dim arrDeptRows
    Dim arrDeptRowInit
    Dim iArrayIndex as Integer

    arrDeptRowInit = Array(11, 12, 13, 14, 15, 16, 17, 101, 102, 103 … 806, 807)

    Redim arrDeptRows(1000) As Integer

    For iArrayIndex = 0 to UBound(arrDeptRowInit)
    arrDeptRows(arrDeptRowInit(iArrayIndex)) = iArrayIndex + 4
    Next

    … would work if there has to be code, but in Excel we have the ability to keep data like this in a more friendly form – worksheets – why don’t we make more use of this advantage? The month names could be loaded from a sheet, and I’d be inclined to do the same with the list of Resumen column letters for clearing, since we seem to be clearing rows 4 to 66 each time. In fact, we ought to be able to derive the 4 and 66 from the a() array, which would mean a reduced risk of screwup when a new department turns up.

    I’m not too enthusiastic abut the For..For..IF structure at the end: it just looks like a candidate for breaking down. I rather tend to feel that if the loop variable needs to be specified on a “Next” then the code’s too complicated. (BTW, the last time I checked, which is not recently, those specified Next variables were a slight drag on performance).

    I have an almost pathological dislike of comments (it’s a medical condition). I don’t get the rash with some kinds: references to printed sources of algorithms, for example, cause me no discomfort. Where a comment is seen as necessary to “explain” what the code is trying to do, then I start to feel pain. One problem can be that the comment gets out of date or is wrong: for example: “‘ the range that shows the month number for a payment in month n is called monthn”. Is it? Where? I see “nmonth” a lot. The comment is not exactly helping here.

    Comments like: “‘ the current year sits in a cell on the worksheet ‘Control panel'” are redundant when the code on the next line says exactly that. If we wrote a new function such as
    Function CurrentYear() As Integer
    CurrentYear = Worksheet(“Control panel”).Range(“gyear”)
    End Function
    .., then we make the meaning more clear using just code.
    Getting back to that nasty nested loop, we could remove comments and clarify code by creating functions/subs such as:
    IsDepartmentPaymentForCurrentTear(cell)
    AmountPaid(nmonth, cell)
    AddAmountPaidToResumen(ndep, cell, namount)

    I hope that doesn’t come across as too nasty (I have been – affectionately, I’m sure – nicknamed “the Code Nazi” at work). Dick is in possession of a snippet of my stuff for the library: perhaps we’ll see if I can take it as well as dish it out!

    Phew!

  2. Jan asked me to post this because he can’t seem to get comments posted:

    Mike, thanks for the criticism! I will try to incorporate your advices in my future programs.

    Yes, the program was translated from Spanish. ‘nmes’ was changed to ‘nmonth’, but unfortunately not everywhere.

    Dick left out the introduction to the code. Here it is:

    The administrator in the block where I live uses a worksheet for each month to record money coming and going, including the monthly variable fee paid by tenants. He asked me to find a way to easily spot tenants who paid too little, or too much (penalty is charged for latecomers and he would like to know who they were at a glance). Some tenants pay six months in advance while most pay before the end of the month in question. For each payment, the apartment number, the month number (1 to 12)and year the payment is for, is recorded.

    I thought first to export all the data to Access and run a query over there. However, in the end I created a worksheet that is updated whenever it is selected. It extracts for each tenant the fee he should pay for each month. This information is sitting in separate monthly workbooks. The sheet also adds up all payments for each tenant for each month on the twelve worksheets in the workbook. For each month and for each tenant what should be paid, what has been paid, and the difference is displayed. Colour coding is used to display in red amounts owed. A button is used to hide what should be paid and what has been paid to just highlight what is owed and extra fees paid (positive difference).

    The code runs fast enough. The delay when opening the sheet is only a few seconds. The administrator is happy with the solution, but I am curious to know of other approaches to the problem.

    Feel also free to critise my code for updating the summary sheet ‘Resumen’:

    I love detailed code critique, but I am also interested in the bigger design picture. Any comments on my global approach?

Leave a Reply

Your email address will not be published. Required fields are marked *