PDA

View Full Version : Council to modify code



PcMax
02-02-2012, 04:32 AM
Hi,

I just finished writing the following code, now I wanted to ask what advice can you give me to improve writing.
I'll have to add Option Explicit to declare variables,
then I have to wonder how any change?


Sub Ciclo()
NumS = 0
Columns("H:I").ClearContents
Num = 1
Vmin = 1000
RMax = 5
Vmax = 0
Dn = Cells(Rows.Count, "B").End(xlUp).Row
Vmin = Cells(5, 2).Value
Vmax = Cells(5, 2).Value
For Each cell In Range("B6:B" & Dn)
If Num = 1 Then
If cell < Vmin Then
Vmin = cell
RMin = cell.Row
End If
If cell >= Vmax Then
Vmax = cell
RMax = cell.Row
End If
If Not Cells(cell.Row, 8) = 3 Then
If cell > cell.Offset(1, 0) Then
If cell >= cell.Offset(2, 0) Then
If cell >= cell.Offset(3, 0) Then
If Cells(RMax, 8) = 0 Then
Cells(RMax, 8) = 2
Cells(RMax, 9) = 2
Cells(RMax + 1, 8) = 3
Cells(RMax + 1, 9) = 3
Else
Cells(RMax, 9) = """ 1 - 2"
Cells(RMax + 1, 8) = 3
Cells(RMax + 1, 9) = 3
End If
'-------------------------
If Cells(RMax + 1, 2) < Cells(RMax + 2, 2) Then
If Cells(RMax + 2, 2) < Cells(RMax + 3, 2) Then
If Cells(RMax + 3, 2) < Cells(RMax + 4, 2) Then
Cells(RMax + 1, 8) = 4
Cells(RMax + 1, 9) = """ 3 - 4"
Cells(RMax + 2, 8) = 1
Cells(RMax + 2, 9) = 1
Vmin = 1000
Vmax = 0
RMax = RMax + 2
Num = 1
GoTo 10
End If
End If
End If
'-------------------------
NumS = cell
Vmin = 1000
Vmax = 0
Num = Num + 1

End If
End If
End If
End If
Else
Riga = RMax
If Not Cells(cell.Row - 1, 8) = 2 Then
If Not Cells(cell.Row, 8) = 3 Then

If cell < cell.Offset(-1, 0) Then
If cell < cell.Offset(1, 0) Then
If cell >= cell.Offset(2, 0) Then
If cell >= cell.Offset(3, 0) Then
If cell >= cell.Offset(4, 0) Then
'--------------------------------------
XX = 0
If cell < cell.Offset(1, 0) Then XX = 1
If cell < cell.Offset(2, 0) Then XX = 1
If XX = 1 Then GoTo 10
'--------------------------------------
Cells(cell.Row, 8) = 4
Cells(cell.Row + 1, 8) = 1
Vmin = cell.Offset(1, 0)
Vmax = cell.Offset(1, 0)
RMax = Riga + 1
Num = 1
GoTo 10
End If
End If
End If
End If
End If
For Each cellx In Range("B" & cell.Row & ":B" & cell.Row + 14)
If cellx.Offset(-1, 0) > cellx Then
If cellx.Offset(1, 0) > cellx Then
If cellx.Offset(2, 0) > cellx Then
If cellx.Offset(3, 0) > cellx Then
Riga = cellx.Row
Exit For
End If
End If
End If
End If
Next
If cell.Row = Riga Then
Cells(Riga, 8) = 4
Cells(Riga, 9) = 4
Cells(Riga + 1, 8) = 1
Cells(Riga + 1, 9) = 1
Vmin = cell.Offset(1, 0)
Vmax = cell.Offset(1, 0)
RMax = Riga + 1
Num = 1
End If
End If

End If
End If
10
Next
End Sub

I have yet to finish the tests, ask advice

PcMax
02-03-2012, 02:22 AM
Hi,
Place a new request because of translation problems

I am completing the code I attached, now I ask you to code comments and constructive criticism.

Do not ask to change it completely, there are rules that you can advise me.

Thank you.

Excel Fox
02-04-2012, 12:16 PM
PcMax,

The best thing to do would be to attach your file, and tell the expected output. We can then device a code that will do the needed, and you can then study the code and understand what could have been done better. Otherwise, from just looking at the code, yes one change that I would suggest is to use Option Explicit and declare all the variables as required.

PcMax
02-05-2012, 01:08 AM
Hi,

I removed all sensitive data and now I attach the file with the complete procedure.
One of my particular research, I ask only to assess improvements to the code.

178

Rasm
02-05-2012, 11:03 PM
PcMax

The Columns("H:I").ClearContents line only clears the cells - not any formatting - so if the cells are already formatted - then you may have trouble. If you want formatting removed then use .ClearFormats - in addition to .ClearContents

You have a lot of If then statements right after each other - maybe rewrite it as illustarted below

If cellx.Offset(-1, 0) > cellx and cellx.Offset(1, 0) > cellx Then


I am not familiar with using quatation marks this way - what does that mean Cells(RMax, 9) = """ 1 - 2"