1
\$\begingroup\$

This is a method that calls a web service, it uploads an audio file and fetches metadata back.

While the advantage is that there is only one method to call there are a few concerns about it :

  • There is some UI-related code (upload progress)
  • Some audio-related code (format detection)
  • Web-service related code (first purpose)

In the end, while it tries to be smart, it's completely tied to a particular audio library, it will report progress anyway and well, it doesn't only upload ...

How would you apply separation of concerns to this current method ?

  • Would transforming the fileFormat to a parameter be acceptable ?
  • What about handler, how can it be used without being tied to the upload process ?

I'm not necessarily asking for code, directions to some documentation on the subject is fine too.

public class TrackUpload
{
    public async Task<string> Upload(string apiKey, string path, ProgressEventHandler handler)
    {
        if (apiKey == null) throw new ArgumentNullException("apiKey");
        if (path == null) throw new ArgumentNullException("path");
        using (var stream = new ReadOnlyFileStream(path))
        {
            if (handler != null)
            {
                stream.ProgressChanging += handler;
            }

            string fileFormat = TrackFormatDetector.Detect(path);
            if (fileFormat == null)
            {
                throw new InvalidAudioFormatException();
            }

            var dataContent = new MultipartFormDataContent();
            dataContent.Add(new StringContent(apiKey), "api_key");
            dataContent.Add(new StringContent(fileFormat), "filetype");
            dataContent.Add(new StreamContent(stream), "track");
            var client = new HttpClient();
            client.DefaultRequestHeaders.UserAgent.Add(
                new ProductInfoHeaderValue(new ProductHeaderValue("xxxx", "1.0")));
            HttpResponseMessage response =
                await client.PostAsync(@"http://developer.echonest.com/api/v4/track/upload", dataContent);
            response.EnsureSuccessStatusCode();
            HttpContent content = response.Content;

            if (handler != null)
            {
                stream.ProgressChanging -= handler;
            }

            using (var reader = new StreamReader(await content.ReadAsStreamAsync()))
            {
                string s = await reader.ReadToEndAsync();
                return s;
            }
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$ – BCdotWEB Feb 10 at 8:19
3
\$\begingroup\$
\$\endgroup\$
0

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.