1

I wrote an async function for calling data from Facebook, it works, but the problem is I dun suppose it works. Can someone explain to me?

public class FacebookData
    {
        static string fb_api_version = ConfigurationManager.AppSettings["fb_ver"];
        static string accessToken = ConfigurationManager.AppSettings["accessToken"];
        static string fb_id = "";
        private HttpClient _httpClient;
        public FacebookData(string input_id)
        {
            fb_id = input_id;
            _httpClient = new HttpClient
            {
                BaseAddress = new Uri("https://graph.facebook.com/" + fb_api_version + "/"),
                Timeout = TimeSpan.FromSeconds(15)
            };
            _httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        }

        public async Task<T> getData<T>()
        {
            var response = await _httpClient.GetAsync($"{fb_id}?access_token={accessToken}");
            if (!response.IsSuccessStatusCode)
                return default(T);

            var result = await response.Content.ReadAsStringAsync();
            return JsonConvert.DeserializeObject<T>(result);
        }
    }

The calling class is typical, I make it await for the response.

But the problem is where I call it.

In main

static void Main(string[] args)
{
   string[] data_Set = [//ids_group]

   for (int i = 0; i < data_Set.length; ++i){
       Console.WriteLine("Running Thread " + (i+1).ToString());
       var dataSet = facebookRequestCmd(data_Set[i]);
       writeToTXT(dataSet);
       Console.WriteLine("Finished Thread " + (i + 1).ToString());
       //do sth
   }
}

In facebookRequestCmd

static Dictionary<string, string[]> facebookRequestCmd(string ids){
   Dictionary<string, string[]> allData = new Dictionary<string, string[]>();
   string[] ids_arr = ids.split(",")
   for (var i = 0; i < ids.length; i++){
      var facebook_client = new FacebookData(sqlData);
      var response = facebook_client.getData<dynamic>();
      Task.WaitAll(response);

      //then use the result to do sth
   }
}

In my understanding, each time I call getData, it already come back to main thread, as it is awaiting the data. So the Task doesn't really start a new thread.

Thus, async await works for waiting the http request, but the Threading should not work.

However,

Console.WriteLine("Running Thread " + (i+1).ToString());

jumps out simultaneously like I really make the Thread in the for loop in main function.

Why? And is that the way to use Multithreading with Async-await. As I want to make multiple calls at the same time.

Originally I use Parallel.ForEach to kick starting the calling, however, thats not asynchronous and will block the thread.

9
  • 1
    I am personally not excited about how this is written at all but to each their own. I don't see you using getData butI do see you using getPostData which you don't provide code for (so maybe that's it?) The other thing is you're using a Task.WaitAll for a single task and although it'll work it's meant for awaiting an enumeration of tasks. Also, if you're not going to use a proper await for any tasks then I suggest just using facebook_client.getPostData<dynamic>().Wait(), assuming that's asynchronous, because that's all it's doing anyway. Commented Jun 27, 2018 at 2:47
  • @MichaelPuckettII Sorry, that's a typo, all should be getData. Besides, I already await _httpClient.GetAsync(...), why do I still need to wait outside? Or I should remove the async of getData ? It doesn't make sense if I remove it right? Commented Jun 27, 2018 at 2:58
  • There's a lot here that I would change personally but let's start with just that question. You can change it in the FacebookData but it's built as a type; and if that type is meant to be asynchronous then changing it there doesn't make sense. The only place it's not really asynchronous is when you're using it in your code now because you're basically blocking the thread for it to complete with Task.WaitAll. Using await here may not be appropriate since this is a Console App but this isn't the proper way to implement the Task.WaitAll is all I'm saying. Commented Jun 27, 2018 at 3:03
  • I would like to re-write this but posting the re-written version may not be a complete answer to what you need to know. Commented Jun 27, 2018 at 3:04
  • Tell you what, since I love C# and this looks like you came from Java I think I'm gonna do it anyway and hopefully get you in the right direction. (I'm bias but I work in Java a decent amount of time here and there so I'm not against it ;) ) Commented Jun 27, 2018 at 3:05

1 Answer 1

4

Ok, feel free to ignore all the changes I've made but I couldn't help but modify the way some of the variables read and the code looked. This is not a working application and I have obviously not tested it. This is just a cleaned up version of what you have with a suggested way of using Task. It's also mocked using just the code you've provided so it is what it is. #2 is, what I believe, the answer you needed.

  1. In Main I removed the words 'thread' since that's not actually what's happening. It may be, but we don't know if the HttpClient is indeed starting a new thread or just holding / returning from the rest call. Using async / await does not always mean a Thread was started (although it's common to think of it that way).
  2. I used .Result (not Wait() as I suggested in comments) to get the result of the task. This is ok since it's a console app but not ideal for a real world app that needs to operate without blocking. I also removed Task.WaitAll with this change.
  3. I renamed functions to have verbage because, IMO, functions should be doing work and the naming should describe the work being done.
  4. I renamed some variables because, IMO, variables should be PascalCase when their scope isn't in a method or private and camelCase when they are. The names should also, IMO, be what it is followed by the Type that makes sense.
  5. I appended 'Async' to function names that return a running Task.
  6. Changed FacebookClient to be singleton and allowing only one HttpClient to be used instead of many and allowing it to be disposed; plus more.
  7. Added alternate version of the GetFacebookData function that calls the tasks and awaits them all simultaneously.

static void Main(string[] args)
{
    string[] dataSet = new string[] { /* mocked */ };  // [ids_group]; <- no idea what this is so I mocked it.

    for (int i = 0; i < dataSet.Length; i++)
    {
        Console.WriteLine("Main... " + (i + 1).ToString());
        var result = GetFacebookData(dataSet[i]);
        WriteToTxt(result);
        Console.WriteLine("Complete... " + (i + 1).ToString());
        //do sth
    }

    Console.Read();
}

private static Dictionary<string, string[]> GetFacebookData(string idsString)
{
    var allDataDictionary = new Dictionary<string, string[]>();
    var idsArray = idsString.Split(',');

    foreach (var id in idsArray)
    {
        var response = FacebookClient.Instance.GetDataAsync<string[]>(id).Result;
        allDataDictionary.Add(id, response);
    }

    return allDataDictionary;
}

public class FacebookClient
{
    private readonly HttpClient httpClient;
    private readonly string facebookApiVersion;
    private readonly string accessToken;

    public static FacebookClient Instance { get; } = new FacebookClient();

    FacebookClient()
    {
        facebookApiVersion = ConfigurationManager.AppSettings["fb_ver"];
        accessToken = ConfigurationManager.AppSettings["accessToken"];

        httpClient = new HttpClient
        {
            BaseAddress = new Uri("https://graph.facebook.com/" + facebookApiVersion + "/"),
            Timeout = TimeSpan.FromSeconds(15)
        };

        httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    }

    public async Task<T> GetDataAsync<T>(string facebookId)
    {
        var response = await httpClient.GetAsync($"{facebookId}?access_token={accessToken}");
        if (!response.IsSuccessStatusCode) return default;
        var result = await response.Content.ReadAsStringAsync();
        return JsonConvert.DeserializeObject<T>(result);
    }

    ~FacebookClient() => httpClient.Dispose();
}

Here's a version that's starting all the tasks and then awaiting them all at the same time. I believe this might give you some issues on the HttpClient but we'll see.


private static Dictionary<string, string[]> GetFacebookData(string idsString)
{
    var allDataDictionary = new Dictionary<string, string[]>();
    var idsArray = idsString.Split(',');
    var getDataTasks = new List<Task<string[]>>();

    foreach (var id in idsArray)
    {
        getDataTasks.Add(FacebookClient.Instance.GetDataAsync<string[]>(id));         
    }

    var tasksArray = getDataTasks.ToArray();
    Task.WaitAll(tasksArray);
    var resultsArray = tasksArray.Select(task => task.Result).ToArray();

    for (var i = 0; i < idsArray.Length; i++)
    {
        allDataDictionary.Add(idsArray[i], resultsArray[i]);
    }

    return allDataDictionary;
}
Sign up to request clarification or add additional context in comments.

9 Comments

Thx, but I have a problem. As I understand, this will do the job in a flow that one id by one id. So it becomes useless for me to divide the ids into few groups right? So I dun need String[] dataSet = new string[], but just merge them into String dataSet = 'All ids in a single string'. And that 'for loop' looping the dataSet is completely useless?
I don't get what you're saying... It sounds like you want to use an array of tasks and wait them all you just weren't doing it properly. I don't know if that's wise since you're constructing and using a new HttpClient each use.. You're looking at socket exceptions I'm sure but I'll do it for the sake of showing.
Originally, the dataSet is just like "id1, id2, id3, id4, id5, id6". As I am thinking about making few thread to process them, so I make it becomes ["id1, id2", "id3, id4", "id5, id6"]. That I can let Thread Task 1 to do "id1, id2", Task 2 to do "id3, id4", and so on. Is that I mix up some concept with multithreading?
I've updated the question and modified the FacebookClient to be more static. I added using multiple tasks at once and awaiting them all. Hopefully it helps you get the answer you're looking for. I'm still not quite sure I know exactly what you want.
Get it, thx a lot, I think I need to take more reading about Thread and Task... seems like I can't mix them up with Asynchronous.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.