4
\$\begingroup\$

This code basically saves an uploaded file to the server. I am wondering if there is anything that I can do to tighten this code up. I am very new to F# so I'm still having trouble breaking away from the C# way of doing things.

/// Create file paths
/// Returns a tuple (server path * link path * file number)
let createPath (file : HttpPostedFileBase) =

    // server directory path
    let serverDirPath = 
        HttpContext.Current.Server.MapPath(@"~/Uploads")

    // get the file name
    let origFileName = file.FileName

    // get the extension
    let extension = Path.GetExtension(origFileName)

    // get the file size in bytes
    let fileSize = file.ContentLength

    // directory check
    let pathExists() = Directory.Exists(serverDirPath)

    // create directory
    let createDir() = 
        if not (pathExists()) then
            Directory.CreateDirectory(serverDirPath) |> ignore
    createDir()

    // find current file name
    let findCurrentFileName() = 

        // check if row exist
        let rowCount = 
            query{
                for row in db.Uploads do
                select row
                count
            }

        // get file number
        let fileNumber = 
            if rowCount < 1
                then
                    1
                else
                    query{
                        for row in db.Uploads do
                        select (row.FileNumber + 1)
                        head
                    }

        // final path
        let finalServPath = 
            serverDirPath + @"\" + fileNumber.ToString() + extension

        // download link
        let linkPath = 
            finalServPath.Replace(serverDirPath + @"\", @"~/Uploads/") 

        finalServPath, linkPath, fileNumber

    findCurrentFileName()

/// Save file to server and path to db.
let SaveUpload (file: HttpPostedFileBase) (title : string) = 


    // create the path including filename
    let servPath, linkPath, fileNumber = createPath file 

    // save file to server
    file.SaveAs(servPath)

    // create new row for db table
    let newUpload = 
        new dbSchema.ServiceTypes.Uploads(Title = title,
                                            FilePath = servPath,
                                            Size = file.ContentLength.ToString(),
                                            FileNumber = fileNumber,
                                            LinkPath = linkPath)
    // insert new row
    insertRowIn db.Uploads newUpload

    // save to db
    saveToDb()
\$\endgroup\$
  • 1
    \$\begingroup\$ It looks fine to me. I'd probably define a type for createPath's return value instead of using a 3-tuple. The only other observation is it's unnecessary to check if a directory exists before calling CreateDirectory. It already does that check for you. \$\endgroup\$ – Daniel Apr 25 '14 at 14:08
  • \$\begingroup\$ Thanks for the comment, Daniel. Can you provide an example of the type to return. \$\endgroup\$ – user1206480 Apr 25 '14 at 15:23
  • \$\begingroup\$ A record would work: type PathInfo = { ServerPath: string; LinkPath: string; FileNumber: int } \$\endgroup\$ – Daniel Apr 25 '14 at 15:25
2
\$\begingroup\$

I think that you're using comments unnecessarily. If you set a variable called fileNumber, the comment // get file number doesn't tell the reader anything new and only clutters the code.


// directory check
let pathExists() = Directory.Exists(serverDirPath)

// create directory
let createDir() = 
    if not (pathExists()) then
        Directory.CreateDirectory(serverDirPath) |> ignore
createDir()

I don't see any reason to have createDir or pathExists as separate functions, since they're short and not reusable.

And as Daniel mentioned in a comment, the check is not actually necessary. So, I would simplify this code to just:

Directory.CreateDirectory(serverDirPath) |> ignore

// check if row exist
let rowCount = 
    query{
        for row in db.Uploads do
        select row
        count
    }

// get file number
let fileNumber = 
    if rowCount < 1
        then
            1
        else
            query{
                for row in db.Uploads do
                select (row.FileNumber + 1)
                head
            }

You can use headOrDefault here, to get rid of the first query:

// get file number
let fileNumber = 
    query {
        for row in db.Uploads do
        select row.FileNumber
        headOrDefault
    } + 1

Also, you're not using any ordering in the query, are you sure the largest FileNumber is always going to be the first one?


let finalServPath = 
    serverDirPath + @"\" + fileNumber.ToString() + extension

You can use Path.Combine() to concatenate parts of a path:

let finalServPath = 
    Path.Combine(serverDirPath, fileNumber.ToString() + extension)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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