# Spreadsheet to calculate how to cut pipes

I have written a piece of code that calculates how to cut pipes based on an order and fixed raw pipe length (in this case 288 feet), but I am not 100% happy with my current code as it counts the pipe pieces that have been created for each length in column C (see comment in code) and clears column C at the end. Is there an way to store the values without having to create a variable for each pipe length?

This is what my input sheet looks like:

Here is my code:

Sub CalcPipe()

Dim StartPipeLng As Long, LastLng As Long, TotalLng As Long, PipeName As Long

StartPipeLng = Cells(2, 4).Value

LastLng = ActiveSheet.UsedRange.Rows(ActiveSheet.UsedRange.Rows.Count).Row

TotalLng = Application.WorksheetFunction.SumProduct(Range("A2:A" & LastLng), Range("B2:B" & LastLng))
TotalLng = Application.WorksheetFunction.RoundUp(TotalLng / StartPipeLng, 0)

Cells(5, 4).Value = TotalLng

PipeName = 1

For y = 6 To TotalLng + 5

Cells(1, y).Value = "Pipe " & PipeName
PipeName = PipeName + 1
StartPipeLng = Cells(2, 4).Value

For x = 2 To LastLng
If StartPipeLng - Cells(x, 1).Value >= 0 And Cells(x, 2).Value <> Cells(x, 3).Value Then
StartPipeLng = StartPipeLng - Cells(x, 1).Value
Cells(x, 3).Value = Cells(x, 3).Value + 1 ' Here is the issue: Rather than write the values in column C, I would like to store them rather then write them in a cell
Cells(x, y).Value = Cells(x, y).Value + 1
x = x - 1
End If
Next x
Next y

Columns(3).Clear

End Sub


And this is what my output looks like:

• "Is there an way to store the values without having to create a variable for each pipe length?" - yes, you can use an array. excel-easy.com/vba/array.html – Máté Juhász Feb 2 '17 at 23:49

You didn't declare y or x.

Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

When you don't define your variable, VBA will declare it as a Variant type that can hold any type of data. While this may be more flexible, it adds processing time to your macro as VBA decides or tests for the type.

Additionally, since a Variant can be any type of data, you may miss out on valuable troubleshooting information on Type Mismatch

By not declaring variables, you could possibly be paying a penalty.

It's good practice to indent all of your code that way Labels will stick out as obvious. You also have a ton of unneeded white space. To begin, it might look like this:

Option Explicit

Sub CalcPipe()
Dim StartPipeLng As Long, LastLng As Long, TotalLng As Long, PipeName As Long
StartPipeLng = Cells(2, 4).Value
LastLng = ActiveSheet.UsedRange.Rows(ActiveSheet.UsedRange.Rows.Count).Row
TotalLng = Application.WorksheetFunction.SumProduct(Range("A2:A" & LastLng), Range("B2:B" & LastLng))
TotalLng = Application.WorksheetFunction.RoundUp(TotalLng / StartPipeLng, 0)
Cells(5, 4).Value = TotalLng
PipeName = 1
Dim y As Long
Dim x As Long
For y = 6 To TotalLng + 5
Cells(1, y).Value = "Pipe " & PipeName
PipeName = PipeName + 1
StartPipeLng = Cells(2, 4).Value
For x = 2 To LastLng
If StartPipeLng - Cells(x, 1).Value >= 0 And Cells(x, 2).Value <> Cells(x, 3).Value Then
StartPipeLng = StartPipeLng - Cells(x, 1).Value
Cells(x, 3).Value = Cells(x, 3).Value + 1
Cells(x, y).Value = Cells(x, y).Value + 1
x = x - 1
End If
Next x
Next y
Columns(3).Clear
End Sub


Variable names - give your variables meaningful names. Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

Dim initialPipeLength As Long
Dim lastPipeRow As Long
Dim totalPipes As Long
Dim pipeNumber As Long
Dim rowCounter As Long
Dim y As Long


A loop like this -

For x = 2 To lastPipeRow
If firstPipeRow -  Then
x = x - 1
End If
Next x


Could probably just be

For x = lastPipeRow to 2 Step -1
Next


But, since it's not a standard loop and that won't work, you should use variable names to make it more obvious what is happening there.

There is a standard way to find lastRow and lastColumn. That post explains why.

lastPipeRow = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row


Also, since you reuse your range, I'd add something to catch if you're accidentally overwriting data

Dim result As VbMsgBoxResult
If Not IsEmpty(ActiveSheet.Cells(1, 6)) Then
result = MsgBox("There is still data on this sheet, do you want to overwrite it?", VbMsgBoxStyle:=vbYesNo)
If result = vbNo Then Exit Sub
End If


I'd also clear out old data

If Not IsEmpty(ActiveSheet.Cells(1, 6)) Then
result = MsgBox("There is still data on this sheet, do you want to overwrite it?", vbYesNo)
If result = vbNo Then Exit Sub
Dim lastColumn As Long
lastColumn = ActiveSheet.Cells(1, Columns.Count).End(xlToLeft).Column
ActiveSheet.Range(Columns(6), Columns(lastColumn)).Clear
End If


To avoid using column 3, use an array. But, I'd say since you want to use an array, you could optimize your solution to the cutting stock problem with the Greedy algorithm.

• :+1:, except a Variant is not an Object. A Variant can be anything including an object, but it's different. An Object variable can only contain an object reference; a Variant can contain anything, including types that aren't even known to the VBA runtime. – Mathieu Guindon Feb 3 '17 at 19:54
• @Mat'sMug I gave it another try, thanks. – Raystafarian Feb 3 '17 at 20:02
• @Raystafarian Thanks so much for taking the time to provide me with your feedback! One thing in regards to the loop, how would For x = lastPipeRow to 2 Step -1 replace For x = 2 To lastPipeRow & 'x = x - 1'? The first loop only loops backwards, while 'x=x-1 is supposed to prevent the loop to move to the next value if the IF` statement is valid. – VBA Pete Feb 3 '17 at 20:19
• @VBAPete the step-1 won't work for this problem, but when you usually see your structure, you assume it would. – Raystafarian Feb 3 '17 at 20:24
• @VBAPete I became curious on this problem and wrote some code that does something along the same lines as yours - codereview.stackexchange.com/questions/154618/… – Raystafarian Feb 6 '17 at 19:15