Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

A couple of things that @Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

 rng.EntireRow.Select
 With Selection
 ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
 Sheets(ActiveSheet.Name).Name = "EURUSD"
 rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
 End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

A couple of things that @Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

 rng.EntireRow.Select
 With Selection
 ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
 Sheets(ActiveSheet.Name).Name = "EURUSD"
 rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
 End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

A couple of things that @Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

 rng.EntireRow.Select
 With Selection
 ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
 Sheets(ActiveSheet.Name).Name = "EURUSD"
 rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
 End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

added 33 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238

A couple of things that Mat's@Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

rng.EntireRow.Select
With Selection
ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
Sheets(ActiveSheet.Name).Name = "EURUSD"
rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

A couple of things that Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

rng.EntireRow.Select
With Selection
ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
Sheets(ActiveSheet.Name).Name = "EURUSD"
rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

A couple of things that @Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

rng.EntireRow.Select
With Selection
ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
Sheets(ActiveSheet.Name).Name = "EURUSD"
rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

Source Link
RubberDuck
  • 31.1k
  • 6
  • 73
  • 176

A couple of things that Mat's Mug didn't mention.

  1. Use Option Explicit

You set Betreich equal to a range, but the variable isn't declared anywhere. You'll nip a lot of runtime errors in the bud if you let the compiler catch the lack of variable declaration.

  1. Avoid Select and Activate

Again, we're looking to avoid runtime errors, but there is a style issue here as well. You use worksheet and range objects elsewhere in your code. Why not here? Be consistent.

rng.EntireRow.Select
With Selection
ActiveWorkbook.Sheets.Add After:=Worksheets(Worksheets.Count)
Sheets(ActiveSheet.Name).Name = "EURUSD"
rng.EntireRow.Copy Worksheets("EURUSD").Cells(5, 1)
End With

Which brings me to..

  1. Properly indent your code.

Ok, so it was already mentioned, but it's a biggie for readability.

lang-vb

AltStyle によって変換されたページ (->オリジナル) /