Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Rubberduck has been using an extension method to export code files, notably for source control purposes.

While testing source control, I ran into a weird bug in the VBIDE API, which caused UserForm modules to perpetually be treated as "modified", because every time we exported them, the VBIDE API would add an empty line at the beginning of the code-behind.

So I had to modify our ExportAsSourceFile extension method to work around that glitch:

  • Export the form to a temporary file
  • Read the contents of the file
  • Locate the last line of the file heading (attributes that are now showing in the VBE)
  • Determine if we have an extraneous empty line, and remove it
  • Write the modified content into the intended export folder

Modifying the file directly in the export folder wasn't an option, because then Git would have considered it as modified anyway.

For context/reference, here's what a UserForm code file looks like:

VERSION 5.00
Begin {C62A69F0-16DC-11CE-9E98-00AA00574A4F} UserForm1 
   Caption         =   "UserForm1"
   ClientHeight    =   3165
   ClientLeft      =   45
   ClientTop       =   390
   ClientWidth     =   4710
   OleObjectBlob   =   "UserForm1.frx":0000
   StartUpPosition =   1  'CenterOwner
End
Attribute VB_Name = "UserForm1"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Option Explicit 'first line of actual user code visible in the VBE

Notice the VB_PredeclaredId attribute is always the last one specified: my fix relies on that detail.

Here's the extension method in question:

/// <summary>
/// Exports the component to the directoryPath. The file is name matches the component name and file extension is based on the component's type.
/// </summary>
/// <param name="component">The component to be exported to the file system.</param>
/// <param name="directoryPath">Destination Path for the resulting source file.</param>
public static string ExportAsSourceFile(this VBComponent component, string directoryPath)
{
    var path = Path.Combine(directoryPath, component.Name + component.Type.FileExtension());
    if (component.Type == vbext_ComponentType.vbext_ct_Document)
    {
        var lineCount = component.CodeModule.CountOfLines;
        if (lineCount > 0)
        {
            var text = component.CodeModule.get_Lines(1, lineCount);
            File.WriteAllText(path, text);
        }
    }
    else if(component.Type == vbext_ComponentType.vbext_ct_MSForm)
    {
        // VBIDE API *sometimes* inserts an extra newline when exporting a UserForm module.
        // this issue causes forms to always be treated as "modified" in source control, which causes conflicts.
        // we need to remove the extra newline before the file gets written to its output location.
        var tempFile = component.ExportToTempFile();
        var contents = File.ReadAllLines(tempFile);
        var vbExposedAttributeLine = 0;
        for (var i = 0; i < contents.Length; i++)
        {
            if (contents[i].StartsWith("Attribute VB_Exposed = "))
            {
                vbExposedAttributeLine = i;
                break;
            }
        }
        if (contents.Length > vbExposedAttributeLine && contents[vbExposedAttributeLine + 1].Replace(Environment.NewLine, string.Empty).Trim() == string.Empty)
        {
            // first line in module is empty - assume it wasn't in the VBE
            var edited = contents.Take(vbExposedAttributeLine).Union(contents.Skip(vbExposedAttributeLine + 1)).ToArray();
            File.WriteAllLines(path, edited);
        }
    }
    else
    {
        // other module types can just be exported directly
        component.Export(path);
    }

    return path;
}

public static string ExportToTempFile(this VBComponent component)
{
    var path = Path.Combine(Path.GetTempPath(), component.Name + component.Type.FileExtension());
    component.Export(path);
    return path;
}

Is there a better way to do this? It works.. but is there anything that could go wrong with it?

share|improve this question
up vote 5 down vote accepted

The logic flaw and bad assumptions were already pointed out, so how about a general review?

  1. It's time for a switch. The old code had only two cases. "If doc type else..." Now we're looking at two special cases, it's time to switch(type).

  2. The insides of those switches need extracted into private methods, ExportDocumentModule() and ExportFormModule(). The logic is non-trivial enough that doing so would both clarify things and contain future changes. Prevent the spaghetti!

  3. I'm not convinced that ExportToTempFile should be public. It seems like an implementation detail at the moment. I wouldn't expose it until you're ready to expose it's been thought out as part of the API to support other features of the app.

  4. This line should be extracted into a well named private Boolean method in order to clarify the intent.

    if (contents.Length > vbExposedAttributeLine && contents[vbExposedAttributeLine + 1].Replace(Environment.NewLine, string.Empty).Trim() == string.Empty)
    
share|improve this answer

Well, here's a form saved from Excel 2013.

Notice the VB_Exposed, VB_HelpID and VB_Description attributes. I set the Description and Help attributes using the Object Browser Context Menu ("Properties"), but a user might have manually edited the attributes in a previous export/import, and so they might be in a different order

VERSION 5.00
Begin {C62A69F0-16DC-11CE-9E98-00AA00574A4F} UserForm1 
   Caption         =   "UserForm1"
   ClientHeight    =   3165
   ClientLeft      =   45
   ClientTop       =   390
   ClientWidth     =   4710
   OleObjectBlob   =   "UserForm1.frx":0000
   StartUpPosition =   1  'CenterOwner
End
Attribute VB_Name = "UserForm1"
Attribute VB_GlobalNameSpace = False
Attribute VB_Creatable = False
Attribute VB_PredeclaredId = True
Attribute VB_Exposed = False
Attribute VB_HelpID = 55
Attribute VB_Description = "This is a description"
Private Sub UserForm_Click()
  Debug.Print "Test"
End Sub

There is a trailing blank line, but CR MarkDown isn't showing it.

Edit: With Excel working again, I imported the file, and I do get the extra header line, and if it is exported with a header line, and reimported, there are then 2 header lines, so I'm getting the same behaviour as you.

You won't be able to rely on the position/order of VB_Attributes, so you'll have to parse the file.

You're also assuming a user won't have intentionally placed a blank line at the top of the module. And what happens if the user has more than one blank line at the top of the module?

The safest approach would probably be to count the blank lines at the top of the code module, before the export, and then adjust the number of blank lines found after the last VB_Attribute but before the actual code begins.

That would potentially deal with any fix that might come to VBE.

share|improve this answer
    
Import your form, then re-export it - you'll see the new blank line at the top of the code file. Nice point about the additional attributes, that totally wrecks my solution. – Mat's Mug Mar 20 at 19:51
    
The "fix" will only remove the line immediately after that specific attribute if it's empty though, so no harm done really - I'll adjust the code to look for the last specified attribute instead of a specific one, the bug is in the VBE API, AFAIK the VBE is no longer maintained/updated my MS; assuming the buggy behavior isn't ideal indeed... I'll try to count the number of empty lines before/after then (which was my first idea, but I succumbed to the "easy" solution) – Mat's Mug Mar 20 at 22:37

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.