1
\$\begingroup\$

I am new to F#. This code is basically a port I did from C#. I am sure there is room for a ton of improvement, so how can I improve and make this more efficient?

let SingleFIle (req : HttpRequestMessage) dirName typeDir (fileType : string) userName 
        clearDir deleteExistingFile =
            async{
                // Check Request
                if not (req.Content.IsMimeMultipartContent())
                then
                    return req.CreateResponse(HttpStatusCode.UnsupportedMediaType)
                else 

                    // create temp file dir
                    let tempFileFolder = 
                        HttpContext.Current.Server.MapPath(@"~/TempFileUploads")

                    // create the temp file folder
                    Directory.CreateDirectory(tempFileFolder) |> ignore

                    // create provider
                    let provider = new MultipartFormDataStreamProvider(tempFileFolder)

                    // save file data
                    try
                        // create the server path
                        let dirPath = ref ""
                        if String.IsNullOrWhiteSpace(userName)
                        then
                            dirPath := 
                                HttpContext.Current.Server
                                    .MapPath(@"~/Uploads/" + typeDir + @"/" + dirName)
                        else
                            dirPath := 
                                HttpContext.Current.Server
                                    .MapPath(@"~/Uploads/" + typeDir + @"/" + dirName +
                                        @"/" + userName)

                        // clear directory if true
                        if clearDir
                        then
                            if Directory.Exists(!dirPath)
                            then 
                                Directory.Delete(!dirPath, true)

                        // create the final directory path
                        Directory.CreateDirectory(!dirPath) |> ignore

                        // save to provider

                        let! readToProvider = 
                            (req.Content.ReadAsMultipartAsync provider) |> Async.AwaitIAsyncResult

                        // file name and type check
                        let fileNameAndTypeIsValid =

                            // file name length list
                            let nameLength =
                                [
                                    for x in provider.FileData ->
                                        x.Headers.ContentDisposition.FileName.Length
                                ]

                            // file type length
                            let fileTypeList =
                                [
                                    for x in provider.FileData ->
                                        x.Headers.ContentType.MediaType
                                         .Substring(0, fileType.Length)
                                ]

                            // nameLength & fileType valid check
                            let nameLengthNotValid =
                                    query{
                                        for x in nameLength do
                                        exists (x <= 2)
                                    }

                            let fileTypeNotValid =
                                query{
                                    for x in fileTypeList do
                                    exists (x <> fileType)
                                }

                            (not nameLengthNotValid) && (not fileTypeNotValid) && 
                                (provider.FormData.Count > 0)

                        // if valid then create file details
                        if fileNameAndTypeIsValid
                        then
                            // get the current file info
                            let fileInfo =
                                new FileInfo(provider.FileData.ElementAt(0)
                                    .LocalFileName)

                            // get the extension
                            let stripQuotes = new FileInfo(provider.FileData.ElementAt(0)
                                        .Headers.ContentDisposition.FileName
                                        .Replace(@"""", String.Empty))
                            let ext = stripQuotes.Extension

                            // create new file path and move file
                            File.Move(fileInfo.FullName, (Path.Combine(!dirPath, fileInfo.Name + ext)))

                            // create file Url
                            let fileUrl = ref ""
                            if (String.IsNullOrWhiteSpace(userName))
                            then
                                fileUrl := @"/Uploads/" + typeDir + @"/" + dirName + @"/" +
                                    fileInfo.Name + ext
                            else
                                fileUrl := @"/Uploads/" + typeDir + @"/" + dirName + @"/" +
                                    userName + @"/" + fileInfo.Name + ext

                            provider.FormData.Add("FileUrl", !fileUrl)

                            // delete existing files if true
                            if deleteExistingFile
                            then
                                for x in provider.FormData
                                            .GetValues("ExistingPath") do
                                    File.Delete(
                                        System.Web.Hosting.HostingEnvironment
                                            .MapPath(x))



                            return req.CreateResponse(HttpStatusCode.OK, provider.FormData)
                        else
                            return req.CreateResponse HttpStatusCode.InternalServerError
                    with
                        | ex -> return req.CreateResponse(HttpStatusCode.InternalServerError)
            }
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Here's a cleaned up version. It's by no means exhaustive, but may get you thinking about other ways to improve it. Some of the changes:

  • removed comments (they didn't add anything since the code is adequately descriptive)
  • replaced refs with let-bound expressions
  • replaced Substring with slicing
  • cleaned up some of the logic
  • refactored file validation into a separate function
  • replaced query expressions with (more suitable) collection functions
  • replaced unused exception with wildcard pattern

    let singleFile 
        (req : HttpRequestMessage) 
        dirName 
        typeDir 
        (fileType : string) 
        userName 
        clearDir 
        deleteExistingFile =
    
        let isValidFile (fileData: seq<MultipartFileData>) =
            let nameLength =
                [
                    for x in fileData ->
                        x.Headers.ContentDisposition.FileName.Length
                ]
    
            let fileTypeList =
                [
                    for x in fileData ->
                        x.Headers.ContentType.MediaType.[..fileType.Length-1]
                ]
    
            let nameLengthNotValid = nameLength |> List.exists ((>=) 2)
            let fileTypeNotValid = fileTypeList |> List.exists ((<>) fileType)
    
            not (nameLengthNotValid || fileTypeNotValid)
    
        async {
            if not (req.Content.IsMimeMultipartContent()) then
                return req.CreateResponse HttpStatusCode.UnsupportedMediaType
            else 
    
                let tempFileFolder = 
                    HttpContext.Current.Server.MapPath @"~/TempFileUploads"
    
                Directory.CreateDirectory tempFileFolder |> ignore
    
                let provider = new MultipartFormDataStreamProvider(tempFileFolder)
    
                try
                    let dirPath = 
                        let path = 
                            @"~/Uploads/" + typeDir + @"/" + dirName 
                            + if String.IsNullOrWhiteSpace userName then "" else @"/" + userName
                        HttpContext.Current.Server.MapPath path
    
                    if clearDir && Directory.Exists dirPath then 
                        Directory.Delete(dirPath, true)
    
                    // create the final directory path
                    Directory.CreateDirectory dirPath |> ignore
    
                    let! readToProvider = 
                        req.Content.ReadAsMultipartAsync provider |> Async.AwaitIAsyncResult
    
                    let fileNameAndTypeIsValid = isValidFile provider.FileData && provider.FormData.Count > 0
    
                    if fileNameAndTypeIsValid then
                        let fileInfo =
                            new FileInfo(
                                provider.FileData.ElementAt(0).LocalFileName
                            )
    
                        let ext = 
                            Path.GetExtension(
                                provider.FileData.ElementAt(0)
                                    .Headers.ContentDisposition.FileName
                                    .Replace(@"""", String.Empty)
                            )
    
                        File.Move(fileInfo.FullName, Path.Combine(dirPath, fileInfo.Name + ext))
    
                        let fileUrl = 
                            @"/Uploads/" + typeDir + @"/" + dirName + @"/"  
                            + if String.IsNullOrWhiteSpace userName then "" else userName + @"/" 
                            + fileInfo.Name + ext
    
                        provider.FormData.Add("FileUrl", fileUrl)
    
                        if deleteExistingFile then
                            for x in provider.FormData.GetValues("ExistingPath") do
                                File.Delete (System.Web.Hosting.HostingEnvironment.MapPath(x))
    
                        return req.CreateResponse(HttpStatusCode.OK, provider.FormData)
                    else
                        return req.CreateResponse HttpStatusCode.InternalServerError
                with
                    | _ -> return req.CreateResponse HttpStatusCode.InternalServerError
        }
    
\$\endgroup\$
1
  • \$\begingroup\$ Awesome! F# is simply amazing. The more I learn the more I'm blown away by how expressive it is. \$\endgroup\$ May 13, 2015 at 1:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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