How to rewrite web api with best practice and best performance?

Question

1.00/5 (1 vote)

See more:

, +

I work on asp.net core 2.2 web API using C# language

I need to rewrite function below with best syntax and with best practice

web API below get Excel file from upload and return Excel file

it working without any issue but I need to rewrite it with best syntax and practice

some points I need to changes :

concatenate path is best thing using

are streaming using correctly

are memory copying file is correct

What I have tried:

ASP.NET

[HttpPost, DisableRequestSizeLimit]
    [Route("Upload")]
    public IActionResult Upload()
    {
        try
        {
            var DisplayFileName = Request.Form.Files[0];
            string fileName = DisplayFileName.FileName.Replace(".xlsx", "-") + Guid.NewGuid().ToString() + ".xlsx";
            string Month = DateTime.Now.Month.ToString();
            string DirectoryCreate = myValue1 + "\\" + Month + "\\" + fileName;
            string exportDirectory = myValue2 + "\\" + Month;
            string exportPath = myValue2 + "\\" + Month + "\\" + fileName;
            string FinalPath = exportPath;
           
            if (!Directory.Exists(DirectoryCreate))
            {
                Directory.CreateDirectory(DirectoryCreate);
       
            }
            if (!Directory.Exists(exportDirectory))
            {
                Directory.CreateDirectory(exportDirectory);
      
            }
            CExcel ex = new CExcel();
            if (DisplayFileName.Length > 0)
            {
                var filedata = ContentDispositionHeaderValue.Parse(Request.Form.Files[0].ContentDisposition).FileName.Trim('"');
                var dbPath = Path.Combine(DirectoryCreate, fileName);
            
                using (var stream = new FileStream(dbPath, FileMode.Create))
                {
                    Request.Form.Files[0].CopyTo(stream);
                    stream.Flush();
                    stream.Close();
                }
                GC.Collect();
                string error = "";
                int rowCount = 0;
                string inputTemplatePath = "";
              
                var InputfilePath = System.IO.Path.Combine(GetFilesDownload, "DeliveryGeneration_Input.xlsx");
                bool areIdentical = ex.CompareExcel(dbPath, InputfilePath, out rowCount, out error);
                if (areIdentical == true)
                {
                    List<InputExcel> inputexcellist = new List<InputExcel>();
                    inputexcellist = ex.Import(dbPath);
                    List<string> mods = new List<string>();
                    mods = inputexcellist.Select(x => x.ModuleName).Distinct().ToList();
                    var OutputfilePath = System.IO.Path.Combine(GetFilesDownload, "DeliveryGeneration_Output.xlsx");
                    if (System.IO.Directory.Exists(Path.Combine(exportDirectory, fileName)))
                    {
                        throw new Exception("Ok so the error message IS right.");
                    }
                    System.IO.File.Copy(OutputfilePath, Path.Combine(exportDirectory, fileName), true);
    
                    SqlConnection con;
                    foreach (var m in mods)
                    {
                        List<InputExcel> inputmodulelist = new List<InputExcel>();
                        inputmodulelist = inputexcellist.Where(x => x.ModuleName == m).ToList();
                        var dtimport = DatatableConversion.ToDataTable(inputmodulelist);
                        DataTable dtexport = new DataTable();
                        dtexport = _deliveryService.LoadExcelToDataTable(_connectionString, dtimport);
                        ex.Export(dtexport, m, exportPath);
    
                    }
                }
                var memory2 = new MemoryStream();
                using (var stream = new FileStream(exportPath, FileMode.Open))
                {
                    stream.CopyTo(memory2);
                }
                memory2.Position = 0;
    
                return File(memory2, "text/plain", Path.GetFileName(exportPath));
    
            }
            else
            {
                return BadRequest();
            }
        }
        catch (Exception ex)
        {
            return StatusCode(500, $"Internal server error: {ex}");
        }
    }

Posted 28-Aug-21 4:51am

ahmed_sa

Updated 31-Aug-21 0:26am

Add a Solution

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Richard Deeming · Answer 1 · 2021-08-31T00:26:00

Where to start?

Quote:

C#

string fileName = DisplayFileName.FileName.Replace(".xlsx", "-") + Guid.NewGuid().ToString() + ".xlsx";

Should be:

C#

string fileName = Path.GetFileNameWithoutExtension(DisplayFileName.FileName) + "-" + Guid.NewGuid().ToString() + Path.GetExtension(DisplayFileName.FileName);

Quote:

C#

string DirectoryCreate = myValue1 + "\\" + Month + "\\" + fileName;
string exportDirectory = myValue2 + "\\" + Month;
string exportPath = myValue2 + "\\" + Month + "\\" + fileName;

You should probably be using Path.Combine here - eg:

C#

string DirectoryCreate = Path.Combine(myValue1, Month, fileName);

Also myValue1 and myValue2 are not defined anywhere.

Quote:

C#

var filedata = ContentDispositionHeaderValue.Parse(Request.Form.Files[0].ContentDisposition).FileName.Trim('"');

You already have the file name - DisplayFileName.FileName. You don't need to parse the ContentDisposition header to extract it again.

Quote:
C#
GC.Collect();

That is almost always the wrong thing to do - especially in a multi-user environment like ASP.NET.

Quote:

C#

throw new Exception("Ok so the error message IS right.");

You should be throwing a more specific exception type, and providing a meaningful message here.

Quote:
C#
SqlConnection con;

Why have you declared this variable when you never use it?

I'm sure there would be plenty of other problems if we knew what the code was supposed to be doing.