PDA

View Full Version : Looking for a better way to write this code please



webcommerce
06-17-2017, 07:51 PM
Greetings All,


My first post, so please excuse any unintentional errors.


I have a macro which inserts a new row at A24, which is top row of a list of data.
It then copies the row below (now 25) to the empty row 24, including formulas and conditional formatting (data bars).
To copy the data bars I discovered I needed to put a value in the currency cells so the formula would work.
However, this code looks 'clunky' and I'm sure it can be improved upon.


I hope someone will accept the challenge - if it is one!! - and come up with a better solution.
Any and many will be appreciated - I understand Excel has many ways to reach the same end.




Range("B25:AR25").Copy Destination:=Range("B24:AR24")
Range("F24:G24,I24:J24,L24:M24,O24:P24,R24:S24,U24:V24,X24:Y24,AA24:AB24,AD24:AE24 ,AG24:AH24,AJ24:AK24,AM24:AN24,AP24:AQ24") = 0.0001
Range("B24:E24").ClearContents





The second row is the one most concerning me.
Thanks,
Ron S

mdmackillop
06-18-2017, 01:15 AM
Can you post a sample workbook. Go Advanced/Manage Attachments

Bob Phillips
06-18-2017, 03:38 AM
Come on,that code is fine. It's 3 lines for crying out loud, if you are worried someone won't understand, put a comment in front of it. Any 'improvements' would be cosmetic, or worse, unclear.

webcommerce
06-18-2017, 03:49 AM
Can you post a sample workbook. Go Advanced/Manage Attachments

I tried putting the whole macro in, got refused 3 times, something about "first post" and "too many urls"!!
Definitely not a friendly experience.
All I was hoping for was a chance to learn a better way to do the code.

webcommerce
06-18-2017, 03:51 AM
Come on,that code is fine. It's 3 lines for crying out loud, if you are worried someone won't understand, put a comment in front of it. Any 'improvements' would be cosmetic, or worse, unclear.

xld, I'm unsure who that was intended for, me or mdmackillop.
Sorry if my inexperience at posting on your forum .. I was hoping to actually learn something.

Bob Phillips
06-18-2017, 07:33 AM
It was aimed at you, and I have given you an opinion supported by decades in IT. Learning might just be understanding that there is nothing wrong with what you have done. Long doesn't mean bad.

SamT
06-18-2017, 07:55 AM
I hope someone will accept the challenge - if it is one!! - and come up with a better solution.
It's not. And, there isn't.

Paul_Hossler
06-18-2017, 01:53 PM
I KNOW that it will be a matter of opinions, but while there is nothing wrong with the second line, I find that I tend to make mistakes when I have a lot of finicky addresses like that (happens as we get older).

It might also be worthwhile thinking ahead to when you need to change the macro to meet new requirements, so I prefer to fix (or hard code) as few things as possible, so you might look at this as see if there's anything that you want to consider


I personally don't worry much about line count or how many characters / statements I can get in a line.








Option Explicit
Sub One()
Const csStartOfTable As String = "B24"

Dim rTableFirstRow As Range
Dim i As Long

Set rTableFirstRow = Range(Range(csStartOfTable), Range(csStartOfTable).End(xlToRight))

rTableFirstRow.Offset(1, 0).Copy Destination:=rTableFirstRow

For i = 6 To 43 Step 3
Cells(rTableFirstRow.Row, i).Value = 0.0001
Cells(rTableFirstRow.Row, i + 1).Value = 0.0001
Next I

Range(csStartOfTable).Resize(1, 4).ClearContents
End Sub

webcommerce
06-18-2017, 02:06 PM
It was aimed at you, and I have given you an opinion supported by decades in IT. Learning might just be understanding that there is nothing wrong with what you have done. Long doesn't mean bad.

And I thank you and respect your opinion and your longevity. I do understand there is nothing wrong with the code: it works. Neither is there anything wrong in discovering other ways to achieve the same result. Whilst it may not matter in this instance, it may be applicable elsewhere in the future, or by knowing the procedure, point me in the right direction.

I am not a young man and I have lived my life gathering knowledge, now I am learning VBA, with the help of others generous enough to share it. I live in hope of a step forward every day.

webcommerce
06-18-2017, 02:08 PM
Thank you SamT: knowledge is precious, ignorance stupidity.....

This is a general observation, nothing personal.

webcommerce
06-18-2017, 02:20 PM
Thank you Paul, much appreciated. Not that I understand it at this my stage of VBA knowledge. I'm also of the belief it is better to be explicit and avoid misunderstandings. I have learned from past mistakes that future memory is uncertain so it's best to plan for the future in the present. With your implied permission I will store your code with that intent.

The whole point of the code was to avoid the Div/0 error message I was getting by copying the empty currency cells which were parts of the formula and the conditional formatting. In the end, by scrabbling around on google searches I have changed the formulas to an IFERROR solution =IFERROR(G27/F27,0), in the copying process,; hence deleting that long line of code altogether. So, problem solved.

Again, if there is an alternative or "better'" way I will be interested to learn about it.

Regards,
Ron S

Bob Phillips
06-19-2017, 12:23 AM
I do understand there is nothing wrong with the code: it works. Neither is there anything wrong in discovering other ways to achieve the same result. Whilst it may not matter in this instance, it may be applicable elsewhere in the future, or by knowing the procedure, point me in the right direction.

You asked for a BETTER solution, I quote ... and come up with a better solution.

Paul has given you an alternative which falls firmly into my statement ... Any 'improvements' would be cosmetic, or worse, unclear. No criticism of Paul, his offering or trying to give you alternatives, but if you think that is better, then we clearly live in different worlds. Part of gathering knowledge is to acknowledge that if it ain't broke, don't fix it.

I will sign-off now, with nothing more to add.

MINCUS1308
06-20-2017, 09:36 AM
I Never Back Down From A Challenge...

Sub CreateChaos()
'VARIABLES
VeryImportantRow = (2 * 6) + 12
VeryImportantValue = 1 / 10000
'COPY ROW 25 TO ROW 24
Set Point = Range(CStr("B" & VeryImportantRow & ":AR" & VeryImportantRow))
Set RowTo = Range(CStr("B" & (VeryImportantRow + 1) & ":AR" & (VeryImportantRow + 1)))
RowTo.Copy Destination:=Point
'SELECT AKWARD RANGE AND SET VALUES
NecessaryIncrement = 1
For TargetedColumn = Columns("F").Column To Columns("AQ").Column
If NecessaryIncrement = 3 Then
NecessaryIncrement = 1
GoTo SkipThisColumn
End If
Cells(VeryImportantRow, TargetedColumn).Value = VeryImportantValue
NecessaryIncrement = NecessaryIncrement + 1
SkipThisColumn:
Next TargetedColumn
'CLEAR CONTENTS OF B24:E24
For TargetedColumnNumberTwo = Columns("B").Column To Columns("E").Column
Cells(VeryImportantRow, TargetedColumnNumberTwo).ClearContents
Next TargetedColumnNumberTwo
End Sub

I find that it is important to complicate things just enough that it looks hard... that way when your boss takes a look you appear even smarter ;)

SamT
06-20-2017, 05:06 PM
:jester:

Bob Phillips
06-21-2017, 04:16 AM
I find that it is important to complicate things just enough that it looks hard... that way when your boss takes a look you appear even smarter ;)

That won't work for me, I am my boss, and I know just how dumb I am!

MINCUS1308
06-21-2017, 05:16 AM
I wish I knew more about VBA / had more time to play with this request. People call my methods round-about but I call it getting the job done despite my inabilities.

Hope everyone has a great day,

#Solved ?

Paul_Hossler
06-21-2017, 07:11 AM
http://dilbert.com/strip/1995-01-22