我这段代码 还可以如何优化?真心请教

Question

本帖最后由 SomethingJack 于 2012-10-23 09:40:29 编辑

Answer 1

不错，GetResponseUrl放到最后面执行一次就好了，string swfpath定义一次也可以

Answer 2

return 了，就没必要else 了

Answer 3

public static void AsConvertFile(string uploadFilepath, string file)
检查参数file,有必要传吗?代码里根本没用到file参数

Answer 4

if (uploadFilepath == null || string.IsNullOrEmpty(AsFileHelper.AsCheckFileType(uploadFilepath)))
            {
                return;
            }
            elsereturn 直接返回了，就不要 else 
呵呵，
只看这么一处

Answer 5

1.首先,只要通过测试,代码就不需要修改了;
2.对以前的代码的总结和改进有利于以后的工作中用更高效的编程手段通过测试,应当受到鼓励;
3.大概提2点我的建议,仅供参考
1)对参数的验证,可以采用断言的形式,比如:
if (uploadFilepath == null 
    || string.IsNullOrEmpty(
    AsFileHelper.AsCheckFileType(uploadFilepath))){
    return;
    }替换如下:
MyHelper.Assert(uploadFilepath!=null,new ArgumentNullException("errMsg1"));
string _s=AsFileHelper.AsCheckFileType(uploadFilepath);
MyHelper.Assert(!string.IsNullOrEmpty(_s),new ArgumentException("errMsg2"));
2)过多的语句块的嵌套不利于快速通过测试

Answer 6

        #region  private static String ConvertToPDF(String uploadFilePath) //转为PDF文件
        //-----------------------------------------------------------------------------------------
        /// <summary>
        /// 转为PDF文件
        /// </summary>
        /// <param name="uploadFilePath"></param>
        /// <returns></returns>
        private static String ConvertToPDF(String uploadFilePath)
        {
            String strPDFPath = String.Empty;
            if (AsConvertHelper.AsConvertPDF(uploadFilePath, strPDFPath))
            {
                return strPDFPath;
            }            return String.Empty;
        }
        //-----------------------------------------------------------------------------------------
        #endregion        #region  private static String ConvertToSwf(String uploadFilePath) //转为SWF文件
        //-----------------------------------------------------------------------------------------
        /// <summary>
        /// 转为SWF文件
        /// </summary>
        /// <param name="uploadFilePath"></param>
        /// <returns></returns>
        private static String ConvertToSwf(String uploadFilePath)
        {
            String strSwfPath = uploadFilePath.Replace(".pdf", ".swf");
            if (AsConvertHelper.AsConvertToSwf(uploadFilePath, strSwfPath))
            {
                return strSwfPath;
            }            return String.Empty;
        }
        //-----------------------------------------------------------------------------------------
        #endregion        #region  public static void AsConvertFile(String uploadFilePath) //PDFToSWF
        //-----------------------------------------------------------------------------------------
        /// <summary>
        /// PDFToSWF
        /// </summary>
        /// <param name="uploadFilepath"></param>
        public static void AsConvertFile(String uploadFilePath)
        {
            if (String.IsNullOrEmpty(uploadFilePath) || String.IsNullOrEmpty(AsFileHelper.AsCheckFileType(uploadFilePath)))
            {
                return;
            }            String strSwfPath = String.Empty;            // 如果上传的文件是PDF格式
            if (uploadFilePath.EndsWith(".pdf"))
            {
                strSwfPath = ConvertToSwf(uploadFilePath);
            }
            else
            {
                String strPDF = ConvertToPDF(uploadFilePath);
                if (!File.Exists(strPDF))
                {
                    strSwfPath = ConvertToSwf(strPDF);
                }
            }            if (!File.Exists(strSwfPath))
            {
                GetResponseUrl(Path.GetFileName(strSwfPath));
            }
        }
        //-----------------------------------------------------------------------------------------
        #endregion简单优了一化,还是很垃圾

Answer 7

从LZ的代码上看是用来实现上传文件转化swf格式的，如果是pdf直接转swf，否则就先转为pdf再转swf。so，提出小白建议：
1.显然pdf转swf的代码可以提出来单做一个方法，私有、公有看需求。
2.这里只有if没else，如果是同名文件怎么处理呢？(提示、重命名、覆盖？)
   file = uploadFilepath.Replace(uploadFile, ".pdf");
   if (!File.Exists(file))

Answer 8

分析下你的逻辑哈。。
1.是.pdf 文件 就转成.swf ..... 
2.不是的话 先要变成.pdf 再转成.swf 这思路不知道对不对..
如果对的话,你有些代码就是重复了..
至少这个可以重用
if (AsConvertHelper.AsConvertToSwf(PDFFolder, swfpath))
{
   GetResponseUrl(Path.GetFileName(swfpath));
}
其次你不管怎样都是要转成.swf 
,所以这里完全可以独立出来

Answer 9

嗯可以这么理解，你的目的是要转成.swf，所以这里可以做成一个方法去处理 。处理完之后再给
 AsConvertFile（）这个函数使用这样层次感就强了

Answer 10

换句话来说，你所有的转换可以在ConvertToSwf(...)这个里面实现好了再拿过来用...

Answer 11

当然如果ConvertToSwf(...)这方法其他地方还有用就不要破坏它了，建议写个重载

Answer 12

1.if (uploadFilepath == null || string.IsNullOrEmpty(AsFileHelper.AsCheckFileType(uploadFilepath)))
            {
                return;
            }
            else
return 了就不需要else，下面直接写就是2. if (uploadFilepath.EndsWith(".pdf"))//如果上传的文件是PDF格式 这里可以直接使用  System.IO.Path.GetExtension()方法判定后缀
3.你最终还是要执行pdf2swf方法滴，所以把他放在最后。前面做相应的xxx2pdf转换
4.其实你整个东西就是一个职责链
 
  

Answer 13

string uploadFile 
我习惯这样写
string strUploadFile

Answer 14

return 之后就不需要在写 else 了吧

Answer 15

给出我的版本吧：        private static bool isValidPath(string filepath) {
            return !(uploadfilepath==null || string.IsNullOrEmpty(AsFileHelper.AsCheckFileType(uploadFilepath)));
        }        private static bool isPdfFile(string filepath) {
            return uploadFilepath.EndsWith(".pdf");
        }        private static string convertPathToPdf(string path) {
            string ext = AsFileHelper.AsCheckFileType(path);
            return path.Replace(ext, ".pdf");
        }        private static string fullpathFrom(string path) {
            return path.Replace("UploadFile", "PdfFile");
        }        private static void getResponseUrlByPdfPath(string path) {
            string swfpath = PDFFolder.Replace(".pdf", ".swf");
            if (AsConvertHelper.AsConvertToSwf(uploadFilepath, swfpath))
            {
                GetResponseUrl(Path.GetFileName(swfpath));
            }                
        }        private static string convertUnknownFileToPdf(string unknownPath) {
            pdfPath = convertPathToPdf(unknownPath);            // 文件存在的情况在原始代码中未处理，这会遗留流程缺陷，所以应直接抛出异常
            // 以尽早暴露问题，而不是忽略掉
            if (File.Exists(pdfPath))
                throw new UnexceptedArgumentException("...");            // 理由如上，既然该方法没有返回值表示转换是否成功，则应抛异常！
            if (!AsConvertHelper.AsConvertToPDF(unknownPath, fullpathFrom(pdfPath)))
                throw new RuntimeException("未能将文件转换成PDF格式");            return pdfPath;
        }
 
        public static void AsConvertFile(string uploadFilepath, string file) {
            // 既然没有进行真正的转换，则应该告诉外界，否则调用者无从得知
            // 转换是否成功。当然，使用bool返回值也是可取的。抛异常只是一个
            // “未完成”的解决方案
            if (!isValidPath(uploadFilepath))
               throw new UnexceptedArgumentException(".....");            string pdfPath = isPdfFile(uploadpath) ? uploadFilepath : convertUnknownFileToPdf(uploadFilepath);
            getResponseUrlByPdfPath(pdfPath);
        }
其实代码复用只是重构的目的之一，更重要的原因在于使代码清晰易读，这样才能保证代码的可持续维护性。关键点有一些：
1）去除没必要的注释
2）使变量和方法名更具自描述性
3）将逻辑概念层次化当然，重构是一门很具有哲学和艺术性的话题，不是三言两语能概括的。推荐书籍：
《重构》
《代码简洁之道》
《敏捷软件开发：原则、模式与实践》

Answer 16

34楼贴出来的代码和原始代码最大的区别在于，AsConertFile方法已经能清晰地看出它的实际逻辑只有两步：
1.根据传入的uploadPath得到一个对应的pdf文件路径，该路径对应转换后（也许不需要转换）的pdf内容
2.根据pdf文件路径获取相应地址(responseUrl)这才是重构的价值——使逻辑更清晰，一读就懂。而原始代码将复合的代码逻辑全部都堆在了一起，使代码复杂化了。打个比方，经理的工作应该仅止于管理，而底下的员工应负责实施，如果经理将所有工作都揽在身上，那么他的工作一定难以进行，他根本没法掌握工作的重点所在。

Answer 17

当代码中出现相同或者类似的代码，就应该考虑重构了，可以运用一下这个简单的规则    
   if (AsConvertHelper.AsConvertToSwf(PDFFolder, swfpath))
                            {
                                GetResponseUrl(Path.GetFileName(swfpath));
                            }
这个出现了2次，所以可以考虑单独封装在一个函数中。

Answer 18

public interface IFileConvert
{
void Convert(string inputFileName, InputDocumentType inputDocumentType, string ouputFileName);
}public class ConvertFactory
    {
        public static bool Convert(string inputFileName, string ouputFileName, OnputDocumentType onputDocumentType)
        {
            var inputDocumentType = InputDocumentTypeHepler.GetDocumentType(inputFileName);            IFileConvert converter = null;            if (onputDocumentType == OnputDocumentType.Pdf)
            {
                converter = GetPdfFileConvert(inputDocumentType);
            }
            else
            {
                converter = GetSwfFileConvert(inputDocumentType);
            }
            if (converter != null)
            {
                converter.Convert(inputFileName, inputDocumentType, ouputFileName);
            }            return converter != null;
        }        public static IFileConvert GetPdfFileConvert(InputDocumentType inputDocumentType)
        {
            switch (inputDocumentType)
            {
                case InputDocumentType.Excel:
                    return new Imp.Pdf.ExcelConvert();
                case InputDocumentType.PPT:
                    return new Imp.Pdf.PPTConvert();
                case InputDocumentType.Word:
                    return new Imp.Pdf.WordConvert();
                case InputDocumentType.TXT:
                    return new Imp.Pdf.TxtConvert();
                default:
                    return null;
            }
        }        public static IFileConvert GetSwfFileConvert(InputDocumentType inputDocumentType)
        {
            return new Imp.Swf.Pdf2SwfExeConvert();
        }    }
/// <summary>
    /// 文档类型
    /// </summary>
    public enum OnputDocumentType
    {
        /// <summary>
        /// Pdf
        /// </summary>
        Pdf = 1,
        /// <summary>
        /// Swf
        /// </summary>
        Swf = 2,
    }
    /// <summary>
    /// 文档类型
    /// </summary>
    public enum InputDocumentType
    {
        /// <summary>
        /// Word
        /// </summary>
        Word = 1,
        /// <summary>
        /// Excel
        /// </summary>
        Excel = 2,
        /// <summary>
        /// PPT
        /// </summary>
        PPT = 3,
        /// <summary>
        /// TXT
        /// </summary>
        TXT = 4,
        /// <summary>
        /// Pdf
        /// </summary>
        Pdf = 5,
    }
CreatePDF
ExcelConvert、PPTConvert、TxtConvert、WordConvertCreateSwf
Pdf2SwfExeConvert这些类都是都是IFileConvert实现。

Answer 19

补充个。。    /// <summary>
    /// 输入文档类型辅助类
    /// </summary>
    public class InputDocumentTypeHepler
    {
        public static InputDocumentType GetDocumentType(string fileName)
        {
            string extension = System.IO.Path.GetExtension(fileName).ToLower();
            switch (extension)
            {
                case ".doc":
                case ".docx":
                    return InputDocumentType.Word;
                case ".xls":
                case ".xlsx":
                    return InputDocumentType.Excel;
                case ".txt":
                    return InputDocumentType.TXT;
                case ".ppt":
                case ".pptx":
                    return InputDocumentType.PPT;
                case ".pdf":
                    return InputDocumentType.Pdf;
                default:
                    return InputDocumentType.Word;
            }
        }        /// <summary>
        /// 获取扩展名
        /// </summary>
        /// <param name="inputDocumentType"></param>
        /// <returns></returns>
        public static string GetExtension(InputDocumentType inputDocumentType)
        {
            switch (inputDocumentType)
            {
                case InputDocumentType.Excel:
                    return ".xls";
                case InputDocumentType.Pdf:
                    return ".pdf";
                case InputDocumentType.PPT:
                    return ".ppt";
                case InputDocumentType.TXT:
                    return ".txt";
                case InputDocumentType.Word:
                    return ".doc";
                default:
                    return ".doc";
            }
        }    }

Answer 20

这种convert的工具类写好了一万年都不会变，还重构个屁啊。

Answer 21

 FileInfo f = new FileInfo(this.openFileDialog1.FileName);
这样就可以获取到 扩展名
你这是何苦呢 ？