Hot answers tagged

8

No need for a loop at all; just use Word's built-in ability to find text based on its style and other formatting. Like so: Public Function FindHeading(strHeadLevel As String) As String Dim rng As Range 'set a range to the selection first so we can avoid ' the selection jumping around as we do our find Set rng = Selection.Range With ...


8

Optimization: You've done a good job of making sure you're not processing more than you have to, but there are a couple of things you can do here to improve performance. Instead of looping through all of the MiniTOCs you've just inserted, updating each one individually. Just update all of the fields in the entire document at once. Replace This: ' Update ...


7

Maybe I'm deeply wrong, but please test: -- Almost all your variables are of type Variant, even the ones which could be Long, like counter. Define each variable with the correct type (it will be faster), in the form: Option Explicit Dim counter as Long Dim ColumnName1 as String, ColumnName2 as String -- You dont really use pIndex. Try: Dim ...


7

First things first, let's clean everything up. Proper descriptive naming, proper validation variables, making the code clear and obvious about what's happening where Public Function GetHeadingFromStyle(ByVal styleToFind As String) As String '/ Iteratively checks the style of all paragraphs, starting at the current selection and working towards the ...


7

Before diving into the code itself, I'm going to go over a couple "structural things". First, I'm not entirely sure what the utility of a tool like this actually is. When I'm writing code in the Visual Basic Editor, I get all of this great help like IntelliSense, syntax highlighting, an Object Browser, etc., etc. (and this is before using custom add-ins ...


7

There are 5 steps you could take after which you could repost your code here. Use 'Option Explicit' in each module and address the errors that this will show. If you are able, install the RubberDuck addin. Use the 'RubberDuck' addin to do a code inspection and then address all the issues you find. Split your code into simple functions/subs. At the moment ...


6

Thanks for using Rubberduck! As @Vogel612 mentioned, the parser error you're getting is a known issue in the v1.4.3 release. In short: foo = bar(x)(y) Accessing a subscript immediately after a function call isn't supported. An easy work-around is to introduce a local variable to hold the collection/array result: Dim result result = bar(x) foo = result(y) ...


6

I have two tiny things right now. The variable count is a system reserved name for something else, so I would avoid naming the variable that way. countComponents might be good. ElseIf (ModuleExists(Module) = False) Then This logic can be simplified by editing the ModuleExists function to return the opposite. ElseIf NoModuleExists(Module) Then A simple ...


6

Things you could try: Set shorter wait times. This shows you how. This is likely the main source of delay. You can assess if this is true by timing the total time, and calculating the time taken with each vfile. Avoid Selecting unless mandatory. For instance, replace ActiveSheet.Columns("C").Select ActiveSheet.Paste with ActiveSheet.Columns("C")....


6

General Notes First - In addition to replacing the magic numbers, I'd put your file path into a constant at the top of the module for easier maintenance: Private Const WORD_DOCUMENT_LOCATION As String = "C:\Users\<user>\Desktop\CyberAwareness.docx" Second - Get a reference to the Worksheet object at the start of the function instead of relying on ...


6

Using case-sensitive string matching like that is in my opinion the first nail in the coffin. Instead a case-insensitive dictionary would be a much better option. Example: var replacements = new Dictionary<string, Func<string>>(StringComparer.OrdinalIgnoreCase) { ["Plan"] = () => $"{DateTime.Now.Year} Q2 Plan", ["Project Name"] = () =...


5

You can replace inner foreach loops with LINQ replace if-else with switch replace string.Format with string interpolation add const modifier for path variables if they will be constants remove string[] args from Main if you won't use them Result using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Wordprocessing; using System; namespace ...


5

Security: Credentials in ENV It is less than ideal to store your credentials in environment variables, as they may easily leak. phpinfo for example will print all environment variables. Of course, you don't want to allow access to phpinfo to just anyone either, but it may still leak, and not everyone who should be allowed to see phpinfo should be allowed ...


5

Performance I see three major issues with the performance of this code. GUI operations are expensive and slow. Remove these lines, they're doing nothing but slowing you down. ActiveSheet.Range(currentCellToAdd).Activate ActiveWindow.ScrollRow = ActiveCell.Row You should avoid using activate and select anyway. They tend to lead to nasty bugs. Don't ...


5

One thing that takes a lot of time in VBA code is switching windows. I usually disable graphical updates with Application.Screenupdating = false at the top of my function, then Application.Screenupdating = true at the exit(s). More details on this, here


5

In the Word document Click on an underlined word In the Home menu, in the Editing section, click Select > Select Text with Similar Formatting Copy Open Excel and paste You may need to clean it up in Excel, but you don't need VBA to do this.


5

Well that's great progress. I now have some more comments. Brute force rather than precision Your search for highlighted words is using a 'brute force' approach as you are examining every word rather than using the Word search function to search for words with a specific highlight. 2 Multiple variables rather than grouped data Your are using multiple ...


5

As this is a code review request, some of my comments may be considered "best practices" by me and not by others (though most of my habits I've picked up from sites and reviews such as this one). Your code is successful already because it accomplishes the task for which you have designed. Most of the improvements I can suggest are in terms of software design ...


4

Those waits will kill you. A second doesn't seem like much, until you stack a lot of them together. I don't think you need to set CutCopyMode to False until the very end, either. What I would try is something like the following. Instead of this, which has two built-in wait periods: rngTemp.Select Application.Wait (Now + TimeValue("0:00:01")) Selection.Copy ...


4

Instead of decoding each field within the CSV as CP1252, you should open the entire file that way. I think that you should approach this more as a templating problem than a document-generation problem. Templating is a challenge that has been solved many times before — though mainly generating HTML rather than OOXML. Authoring OOXML directly looks like it ...


4

Answers Question 1: No, this is now Hungarian-free™. The article that was referred to in your prior post differentiated between "Apps Hungarian" and "Systems Hungarian", and your previous code was using the latter (i.e. strName). You were basically indicating that yourFunction SearchWordDoc(strPath, strName)was expecting String's as parameters. The current ...


3

Give this a shot. I was able to identify 30,000 underlined words out of a total 140,000 words in about 25 seconds. I also posted this on the SO question. This might a more flexible approach if you want to add various criteria to the search. To provide some more context as to how this works. This subroutine works by iterating over each StoryRange, e.g. ...


3

The following is a start at an answer to speed things up. Basically, I have begun to replace all instances in which you individually .select and or reference the same object over and over(esp. Activesheet) Take a look and compare to your version. Please note that I did not have a chance to run this: Sub ExcelToWord() Dim fileName, Pathname As String ...


3

It's not clear why you would need to p/invoke the native MessageBoxU function, given you're not using it anywhere, correctly preferring the language's own MsgBox wrapper. Remove all the dead/commented-out code if you're not using it - otherwise is obscures the intent of the code. As far as I know, Universal_Converter_v1.0 is only a valid VBA identifier if ...


3

This post is tagged with object-oriented, but what I'm seeing is very ironically procedural code. It doesn't matter that this is "just a quick tool": we're here to write professional, quality code that is easy to read and maintain, performs well, and correctly. Comintern has given great feedback and highlighted a number of bugs and edge cases - which is ...


3

Answering my Own question to share with community the Height of stupidity in my code and gradually how over 1 or 2 sleepless nights of testing, the problem was reduce to workable. First the height of stupidity is the line If i < .Paragraphs.Count Then even after knowing well that while working with such giant document interaction with document is ...


2

ScreenUpdating In order for you to interact with a spreadsheet (e.g. moving the mouse, selecting a cell) , the display on your monitor is re-drawn something like 30+ times per second. This takes a lot of processing power. If you set Application.ScreenUpdating = False then, while it is false, the screen will not be updated, and your code will run much ...


2

The name Update_Word_DocMs_In_Folder isn't following the PascalCase convention that's otherwise observed by your code; underscores should generally be avoided in procedure names in VB6/VBA (especially for Public members), because underscores have a very specific meaning in identifier names: [Interface]_[Member] [EventSource]_[EventName] So ...


2

Naming variables Give your variables meaningful names. Characters are free and when you know what something is by looking at its name, it's so much easier to follow the code! A few examples: rngP1 might be better off as something like firstParagraph You'll notice I said "to hell!" with that hungarian notation - it's unneeded! Here: LastRow1 As Long, ...


2

First of all, kudos for naming all these controls! You have a lot of duplication going on; extract functionality into more specialized functions/procedures. For example, this: If (AllDocumentsPostedCheckbox.Value = True) Then Section8Complete.Caption = "Complete": Section8Complete.BackColor = RGB(0, 255, 0): DocInputBy.Caption = UpgradeTechnic.Text ...


Only top voted, non community-wiki answers of a minimum length are eligible