+ Reply to Thread
Results 1 to 8 of 8

Code does what its suppose to do, how can i make it better

Hybrid View

  1. #1
    Registered User
    Join Date
    10-26-2007
    Posts
    55

    Code does what its suppose to do, how can i make it better

    Hi,

    I'm new to programming in general and looking at learning VBA due to the fact my work has endless copy and paste spreadsheets that do my nut in!

    I've produced a simple copy and paste from one workbook to another via the Macro Recorder and then I've cleaned it up a bit.

    What could i do to improve it
    Sub CopyFromAnotherWorkbook()
    
    ' CopyFromAnotherWorkbook - Copy data from Previous 7 days service disruption.xlsm to MI&SD Tracking.xlsm
    
    Dim LastRowColB As Long 'Count last row in Previous 7 days service disruption.xlsm
    Dim FinalRowColA As Integer 'Count last row in MI&SD Tracking.xlsm
    Dim Path As String
    
    Path = ThisWorkbook.Path & "\"
    
    Application.ScreenUpdating = False
    
    Application.Workbooks.Open (Path & "Previous 7 days Service Disruptions.xlsm")
            
    Worksheets("Report 1").Activate
    
            LastRowColB = Range("b65536").End(xlUp).Row
        
        'Get Region code and autofill
        Range("H5").Formula = "=TRIM(LEFT(F5,FIND(""SD"",F5)-1))"
        Range("H5").Select
        Selection.AutoFill Destination:=Range("H5:H" & LastRowColB)
      
        'Copy Data in Previous 7 days service disruption.xlsm
        Range("B5:K" & LastRowColB).Select
        Selection.Copy
        
        'Activate MI&SD Tracking.xlsm and Activate SD Raw Data Worksheet
        Windows("MI&SD Tracking.xlsm").Activate
        Worksheets("SD Raw Data").Activate
        
        'Find row to paste new data to end of current data
        FinalRowColA = Range("A65536").End(xlUp).Row
        Range("A" & FinalRowColA + 1).Select
        'Paste Data
        Selection.PasteSpecial Paste:=xlPasteValues
         
    Application.DisplayAlerts = False
    Workbooks("Previous 7 days Service Disruptions.xlsm").Close
    Application.DisplayAlerts = True
    Application.ScreenUpdating = False
    
    End Sub

  2. #2
    Forum Expert royUK's Avatar
    Join Date
    11-18-2003
    Location
    Derbyshire,UK
    MS-Off Ver
    Xp; 2007; 2010
    Posts
    26,200

    Re: Code does what its suppose to do, how can i make it better

    Can't check this so try it on a back up workbook

    Option Explicit
    Sub CopyFromAnotherWorkbook()
    
    ' CopyFromAnotherWorkbook - Copy data from Previous 7 days service disruption.xlsm to MI&SD Tracking.xlsm
    
    Dim LastRowColB As Long 'Count last row in Previous 7 days service disruption.xlsm
    Dim FinalRowColA As Long 'Count last row in MI&SD Tracking.xlsm
    Dim sPath As String
    Dim wbSource As Workbook
    
    sPath = ThisWorkbook.Path & "\"
    
    With Application
    .ScreenUpdating = False
     
               Set wbSource = Workbooks.Open(sPath & "Previous 7 days Service Disruptions.xlsm")
    With wbSource.Worksheets("Report 1")
            LastRowColB = .Range("b65536").End(xlUp).Row
        'Get Region code and autofill
        .Range("H5").Formula = "=TRIM(LEFT(F5,FIND(""SD"",F5)-1))"
        Range("H5").AutoFill Destination:=.Range("H5:H" & LastRowColB)
        'Copy Data in Previous 7 days service disruption.xlsm
        .Range("B5:K" & LastRowColB).Copy
     End With
     
        With ThisWorkbook.Worksheets("SD Raw Data")
        'Find row to paste new data to end of current data
        FinalRowColA = .Range("A65536").End(xlUp).Row + 1
          .Range("A" & FinalRowColA).PasteSpecial Paste:=xlPasteValues
        End With
    .DisplayAlerts = False
       wbSource.Close
    .DisplayAlerts = True
    .ScreenUpdating = False
    End With
    End Sub
    Hope that helps.

    RoyUK
    --------
    For Excel Tips & Solutions, free examples and tutorials why not check out my web site

    Free DataBaseForm example

  3. #3
    Registered User
    Join Date
    10-26-2007
    Posts
    55

    Re: Code does what its suppose to do, how can i make it better

    thats great, yes was wondering about how to setup the workbook as an object.

    Not sure about with statements so i'll need to read up on them

  4. #4
    Forum Expert Domski's Avatar
    Join Date
    12-14-2009
    Location
    A galaxy far, far away
    MS-Off Ver
    Darth Office 2010
    Posts
    3,950

    Re: Code does what its suppose to do, how can i make it better

    Made a few changes like removing all selecting of cells and activating of worksheets which is unnecessary along with not needing to use Autofill to populate the cells with formula and also turning screen updating back on which your code didn't do...


    Sub CopyFromAnotherWorkbook()
    
    ' CopyFromAnotherWorkbook - Copy data from Previous 7 days service disruption.xlsm to MI&SD Tracking.xlsm
    
        Dim LastRowColB As Long    'Count last row in Previous 7 days service disruption.xlsm
        Dim FinalRowColA As Integer    'Count last row in MI&SD Tracking.xlsm
        Dim Path As String
    
        Path = ThisWorkbook.Path & "\"
        Application.ScreenUpdating = False
        Application.Workbooks.Open (Path & "Previous 7 days Service Disruptions.xlsm")
        With Workbooks("Previous 7 days Service Disruptions.xlsm").Worksheets("Report 1")
            LastRowColB = .Range("B" & Rows.Count).End(xlUp).Row
            'Get Region code and autofill
            .Range("H5:H" & LastRowColB).Formula = "=TRIM(LEFT(F5,FIND(""SD"",F5)-1))"
            'Copy Data in Previous 7 days service disruption.xlsm
            .Range("B5:K" & LastRowColB).Copy
        End With
        'Paste Data to end of current data
        With Workbooks("MI&SD Tracking.xlsm").Worksheets("SD Raw Data")
            FinalRowColA = .Range("A" & Rows.Count).End(xlUp).Row
            .Range("A" & FinalRowColA + 1).PasteSpecial Paste:=xlPasteValues
        End With
        Workbooks("Previous 7 days Service Disruptions.xlsm").Close False
        Application.ScreenUpdating = True
    
    End Sub
    Dom
    "May the fleas of a thousand camels infest the crotch of the person who screws up your day and may their arms be too short to scratch..."

    Use code tags when posting your VBA code: [code] Your code here [/code]

    Remember, saying thanks only takes a second or two. Click the little star to give some Rep if you think an answer deserves it.

  5. #5
    Forum Expert Domski's Avatar
    Join Date
    12-14-2009
    Location
    A galaxy far, far away
    MS-Off Ver
    Darth Office 2010
    Posts
    3,950

    Re: Code does what its suppose to do, how can i make it better

    A With...End With construct enables you to perform multiple operations on an object without repeatedly referring to it which is more efficient and involves less typing

    As an example this:

    Range("A1").Formula = "=A2+A3"
    Range("A1").Copy
    Range("A1").PasteSpecial xlPasteValues
    Can be written as:

    With Range("A1")
        .Formula = "=A2+A3"
        .Copy
        .PasteSpecial xlPasteValues
    End With
    Dom

  6. #6
    Registered User
    Join Date
    10-26-2007
    Posts
    55

    Re: Code does what its suppose to do, how can i make it better

    ok cool, not to much to it then. cheers

    Quote Originally Posted by Domski View Post
    A With...End With construct enables you to perform multiple operations on an object without repeatedly referring to it which is more efficient and involves less typing

    As an example this:

    Range("A1").Formula = "=A2+A3"
    Range("A1").Copy
    Range("A1").PasteSpecial xlPasteValues
    Can be written as:

    With Range("A1")
        .Formula = "=A2+A3"
        .Copy
        .PasteSpecial xlPasteValues
    End With
    Dom

  7. #7
    Forum Expert snb's Avatar
    Join Date
    05-09-2010
    Location
    VBA
    MS-Off Ver
    Redhat
    Posts
    5,649

    Re: Code does what its suppose to do, how can i make it better

    This might suffice:

    Sub snb()
      with getobject(Thisworkbook.path & "\Previous 7 days Service Disruptions.xlsm")
        sn=.sheets("Report 1").cells(1).currentregion.offset(,1).resize(,10)
        .close false
      end with
    
      thisworkbook.sheets("SD Raw Data").cells(rows.count,1).end(xlup).offset(1).resize(ubound(sn),ubound(sn,2))=sn
    End Sub
    Last edited by snb; 01-19-2012 at 10:44 AM.



  8. #8
    Registered User
    Join Date
    10-26-2007
    Posts
    55

    Re: Code does what its suppose to do, how can i make it better

    Get a compile error at sn =

    Quote Originally Posted by snb View Post
    This might suffice:

    Sub snb()
      with getobject(Thisworkbook.path & "\Previous 7 days Service Disruptions.xlsm")
        sn=.sheets("Report 1").cells(1).currentregion.offset(,1).resize(,10)
        .close false
      end with
    
      thisworkbook.sheets("SD Raw Data").cells(rows.count,1).end(xlup).offset(1).resize(ubound(sn),ubound(sn,2))=sn
    End Sub

+ Reply to Thread

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts

Search Engine Friendly URLs by vBSEO 3.6.0 RC 1