Menu

Bug in XlsDocument.Save

Help
2008-03-21
2013-04-29
  • Wesley Witt

    Wesley Witt - 2008-03-21

    The code in XlsDocument.Save that attempts to create a full path for the file using the current directory will fail when the file name contains a full path.  The code is as follows:

    if (path == null)
        path = Environment.CurrentDirectory;
    string separator = Path.DirectorySeparatorChar.ToString();
    if (!path.EndsWith(separator))
        path += separator;
    string fileName = path + FileName;

    This code assumes that FileName contains just the file name portion of the path.  Also, this code is forming the full path incorrectly.  In .Net 2.0 is is better to use Path.GetFullPath or if you need to create a path from parts of a path you should use Path.Combine.  Personally I think this code can be replaced by Path.GetFullPath.

     
    • Tim Erickson

      Tim Erickson - 2008-03-21

      This has been updated since the last release to use Path.Combine - you can see it if you go to Code / SVN Browse from the MyXls Sourceforge project pages (then select trunk, MyXls, MyXls, XlsDocument.cs, view).  This will go out with the next release which should be soon in the next week or two.

      I will look at enforcing filename only (i.e. no path) on the XlsDocument.FileName setter.

      Thanks for the input!

       
    • Wesley Witt

      Wesley Witt - 2008-03-21

      I have worked around this, but this really raises a question around the programming model presented by MyXls.  It seems strange to split the directory from the file name in this way.  If you look at the programming model presented by the .NET Framework, for example, I don't think this is ever done.  It seems like it would be better to just have the caller  pass in a file name string and have the MyXls code call GetFullPath on the string.  Splitting out the directory confuses the use of the class isn't really inline with the .NET zen.

       
      • Tim Erickson

        Tim Erickson - 2008-03-21

        I understand what you're saying, but the Save method was only a recent addition - the original intended application for MyXls was essentially in an ASP.NET context, with only the Send method available.  In ASP, a path cannot be specified in the HTTP response.  Keeping the directory and filename separate allows both use cases without duplicating code or needlessly overriding methods.  The use of the path parameter for the Save method is documented and points the user to the FileName property for the filename, which itself documents that no path should be provided.

        I won't get into whether or not this is "really inline with the .NET zen", but am open to discussing whether this is the most intuitive and best way to handle these different MyXls-specific use cases.

        Perhaps the Save method should accept a path and/or a filename, setting the FileName in the latter case?  This seems to introduce potentially confusing side-effects.

        Perhaps the Send method should require a filename, and remove the FileName property altogether, requiring a filename on the Save method?  This could maintain clarity and distinction between two separate methods and use cases and seems the best alternative I can think of to the current interface.

        But what if a user wants to both Send and Save the XlsDocument?  They must then manipulate the file and path portions themself rather than simply setting a FileName, then calling Save (optionally specifying a path), and then Send.  I really don't know how much of an edge case this is.

        Overall, I don't have strong feelings one way or another, beyond not wanting to clutter the XlsDocument with additional/unnecesary overloads, or keep changing this every release.

        What do you think?  Does anyone else have an opinion?

         

Log in to post a comment.