Hot answers tagged

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 ...


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 ...


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 ...


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

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

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 ...


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

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

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

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

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

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

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

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

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

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

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

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 ...


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

Shouldn't: #region Find/Replace parameters ... $findWrap = [Microsoft.Office.Interop.Word.WdReplace]::wdReplaceAll $format = $false $replace = [Microsoft.Office.Interop.Word.WdFindWrap]::wdFindContinue #endregion be: #region Find/Replace parameters ... $replace = [Microsoft.Office.Interop.Word.WdReplace]::wdReplaceAll $format = $false $findWrap = [...


2

I would try to use bulkcopy to load all of the IDs and their CachedText at once into a staging table in Azure, and then do a single update on your document table. CREATE TABLE document (docKey BIGINT IDENTITY(1, 1) PRIMARY KEY, CachedText NVARCHAR(MAX), id INT ); CREATE TABLE document_stage (CachedText NVARCHAR(MAX), id INT ); ...


1

You're doing a lot with .Selection. By default, that is going to be slow. Doing things behind the scenes is always faster. Either way, working with selection isn't the best way to go. Either pass the document to the sub or prompt the user. Is the selection a document, text, body, header, title, section? Be explicit. The way you use .Find makes me thing you'...


1

COM Automation (which is what you are using) is always going to be slow. There's not much you can do about that except to try find new ways to do what you want with as few operations as possible. An alternative you could investigate is the Open XML SDK. I've never tried it myself, but it is supposed to be a lot faster than COM Automation. The Open XML SDK ...


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