patternMinor
Are these good examples of functions that do one thing only or am I getting a little carried away?
Viewed 0 times
awaythesearegettingexamplesonethatlittlegoodcarried
Problem
I'm reading Robert C. Martin's "Clean Code" and for the past few days I've been fiddling with its function writing guidelines. At first I thought the idea of functions being two/three lines long to be ludicrous, but I'm slowly getting the hang of it. The problem is, I'm pretty sure I'm overdoing it now.
These are the first lines of the module I decided to refactor using the advice from the book.
And this is how it looks like today.
```
Public Sub ExportAvailabilityData()
Dim aplicativo As Excel.Application
Set aplicativo = CreateObject("Excel.Application")
aplicativo.Visible = True
Dim pastaDeTrabalho As Excel.Workbook
Set pastaDeTrabalho = aplicativo.Workbooks.Add
CreateSheets pastaDeTrabalho
NameSheets pastaDeTrabalho
End Sub
Private Sub CreateSheets(ByRef pastaDeTrabalho As Excel.Workbook)
CreateDailySheets pastaDeTrabalho, Month(Date)
CreateSummarySheet pastaDeTrabalho
End Sub
Private Sub CreateDailySheets(ByRef pastaDeTrabalho As Excel.Workbook, ByVal month As Integer)
Do Until pastaDeTrabalho.Sheets.Count = DaysInMonth(month)
pastaDeTrabalho.Sheets.Add
Loop
End Sub
Private Sub CreateSummarySheet(ByRef pastaDeTrabalho As Excel.Workbook)
pastaDeTrabalho.Shee
These are the first lines of the module I decided to refactor using the advice from the book.
Public Sub ExportAvailabilityData()
Dim aplicativo As Excel.Application
Set aplicativo = CreateObject("Excel.Application")
aplicativo.Visible = True
Dim pastaDeTrabalho As Excel.Workbook
Set pastaDeTrabalho = aplicativo.Workbooks.Add
Do Until pastaDeTrabalho.Sheets.Count = DaysInMonth(Month(Date)) + 1
pastaDeTrabalho.Sheets.Add
Loop
Dim day As Integer
For day = 1 To DaysInMonth(Month(Date))
pastaDeTrabalho.Sheets(day).Name = Format(day, "00") & "." & Format(Month(Date), "00")
Next day
pastaDeTrabalho.Sheets(DaysInMonth(Month(Date)) + 1).Name = "Summary " & UCase(MonthName(Month(Date)))
End Sub
Private Function DaysInMonth(ByVal month As Integer) As Integer
DaysInMonth = Day(DateSerial(Year(Date), month + 1, 1) - 1)
End FunctionAnd this is how it looks like today.
```
Public Sub ExportAvailabilityData()
Dim aplicativo As Excel.Application
Set aplicativo = CreateObject("Excel.Application")
aplicativo.Visible = True
Dim pastaDeTrabalho As Excel.Workbook
Set pastaDeTrabalho = aplicativo.Workbooks.Add
CreateSheets pastaDeTrabalho
NameSheets pastaDeTrabalho
End Sub
Private Sub CreateSheets(ByRef pastaDeTrabalho As Excel.Workbook)
CreateDailySheets pastaDeTrabalho, Month(Date)
CreateSummarySheet pastaDeTrabalho
End Sub
Private Sub CreateDailySheets(ByRef pastaDeTrabalho As Excel.Workbook, ByVal month As Integer)
Do Until pastaDeTrabalho.Sheets.Count = DaysInMonth(month)
pastaDeTrabalho.Sheets.Add
Loop
End Sub
Private Sub CreateSummarySheet(ByRef pastaDeTrabalho As Excel.Workbook)
pastaDeTrabalho.Shee
Solution
I think you have definitely taken it in the right direction. Excellent work!
I don't see any compelling reason to break
My only complaint is that I don't really like the separation of adding and naming the sheets. I see both steps as being part of creating a sheet. I think it would be nice to set the name of the sheet immediately after adding it (in the same function). That will likely eliminate some code as well. I suspect
I don't see any compelling reason to break
AssembleDailySheetName() down any further.My only complaint is that I don't really like the separation of adding and naming the sheets. I see both steps as being part of creating a sheet. I think it would be nice to set the name of the sheet immediately after adding it (in the same function). That will likely eliminate some code as well. I suspect
Sheets.Add may return the new sheet. If that's the case, there would be no need to look up the sheet by index to name it.Context
StackExchange Code Review Q#26682, answer score: 5
Revisions (0)
No revisions yet.