1
\$\begingroup\$

I am new to C# and I have a simple /upload controller to upload an image to S3. Everything seems to work, just fine but it seems messy.

The function:

  • Takes multipart form data
  • Reads it async
  • Picks out the image
  • Picks out the associated identity of the image
  • Reads the identity as a stream
  • Gets a stream of the image by reading the byte array.
  • sends the image on its way.

Seems pretty ugly. Anything I can do to make it more efficient with less code?

Sample payload

------WebKitFormBoundaryRmgsjwjP3nVPh7C7
Content-Disposition: form-data; name="file"; filename="10385095.jpeg"
Content-Type: image/jpeg


------WebKitFormBoundaryRmgsjwjP3nVPh7C7
Content-Disposition: form-data; name="ident"

someguid

------WebKitFormBoundaryRmgsjwjP3nVPh7C7--

Code

    [HttpPost, Route("api/aws/upload")]
    public async Task<IHttpActionResult> Upload()
    {
        if (!Request.Content.IsMimeMultipartContent())
            throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType);

        var provider = new MultipartMemoryStreamProvider();
        await Request.Content.ReadAsMultipartAsync(provider);
        var file = provider.Contents[0];
        var identity = provider.Contents[1];
        Stream _id = postId.ReadAsStreamAsync().Result;
        StreamReader reader = new StreamReader(_id);
        string identity = reader.ReadToEnd();
        string filename = file.Headers.ContentDisposition.FileName.Trim('\"');
            var buffer = await file.ReadAsByteArrayAsync();
            string mime = file.Headers.ContentType.ToString();
            Stream stream = new MemoryStream(buffer);
            AwsStore store = new AwsStore();
            object response = store.UploadMultiple(stream, filename, mime, identity);


        return Ok(response);
    }
\$\endgroup\$
3
\$\begingroup\$

The naming and variable conventions in your method look like it was written by at least three different persons:

var provider ..
Stream _id ..
StreamReader reader ..

You really should pick just one style and stick to it for the entire application. It's very hard to read.


But the variables are not the only thing that is extremely incosistent.

Stream _id = postId.ReadAsStreamAsync().Result;

Why are you not awaiting this result like you do with the other ..Async methods?


All streams need to be disposed.


Overall I've got the impression that you put this method together just for the sake of this question and mixed variables and fields which would explain why some of them are prefixed with the underscore.

\$\endgroup\$
  • \$\begingroup\$ Thanks... I think... I slapped this together and it took quite a bit of effort to get it to work right, so I overlooked the variable names. I disposed the streams and was able to condense some of it down. \$\endgroup\$ – Taylor Ackley Nov 18 '17 at 4:01

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.