HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Server code to save uploaded files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codesavefilesuploadedserver

Problem

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 = cr

Solution

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)

Code Snippets

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

// create directory
let createDir() = 
    if not (pathExists()) then
        Directory.CreateDirectory(serverDirPath) |> ignore
createDir()
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
            }
// get file number
let fileNumber = 
    query {
        for row in db.Uploads do
        select row.FileNumber
        headOrDefault
    } + 1
let finalServPath = 
    serverDirPath + @"\" + fileNumber.ToString() + extension

Context

StackExchange Code Review Q#48106, answer score: 2

Revisions (0)

No revisions yet.