patternMinor
Server code to save uploaded files
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
```
/// 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
I don't see any reason to have
And as Daniel mentioned in a comment, the check is not actually necessary. So, I would simplify this code to just:
You can use
Also, you're not using any ordering in the query, are you sure the largest
You can use
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
} + 1Also, 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() + extensionYou 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
} + 1let finalServPath =
serverDirPath + @"\" + fileNumber.ToString() + extensionContext
StackExchange Code Review Q#48106, answer score: 2
Revisions (0)
No revisions yet.