-2

I'm doing an assignment for my school site and I'm trying to find a "best practice" way to go about solving my problem.

So, whenever a certain method is run I have to reupload five different files, to five different FTP. Each one of these FTPs require different credentials, so for each upload I have to create a new client with the corresponding set of credentials.

Right now, my code looks something like this, which I feel is very dirty (but I'm not sure of a better way!):

public static string url;
public static string username;
public static string password;

public static void UploadFiles()
{
   for (var i = 0; i < 5; i++)
       GetCredentials(i)

   using (var client = new FtpClient(url, username, password))
   {
       // connect to client
       // and upload the file
       // using the parameters set in GetCredentials()
   }
}

private static void GetCredentials(int id)
{
    case 0:
        username = "user0"
        password = "pass0"
    case 1:
        username = "user1"
        password = "pass1"
    case 2:
        username = "user2"
        password = "pass2"
    case 3:
        username = "user3"
        password = "pass3"
    case 4:
        username = "user4"
        password = "pass4"
}

This works all well and good and I'm about to set this into production, but I'd really like to learn something from this, instead of just using the first solution that comes to my mind. Any advice is appreciated!

Tauropi
  • 15

1 Answers1

0

try this

public class Client
{
    public string username;
    public string password;
}

public void main(string[] args)
{
   List<Client> clients = new List<Client>()
   clients.Add(new Client() {username = "user1", password = "pass1"});
   clients.Add(new Client() {username = "user2", password = "pass2"});
   clients.Add(new Client() {username = "user3", password = "pass3"});
   ....
   //really you should load them from a file
   var url = "get this form the user?";
   UploadFiles(url, clients).Wait;
}

public async Task UploadFiles(string url, IEnumerable<Client> clients)
{
    List<Task> tasks = new List<Task>();
    foreach(var c in clients)
    {
        tasks.Add(ftpUpload(url,c));
    }
    await Task.WhenAll(tasks);
}

public async Task ftpUpload(string url, Client client)
{
    using (var ftpClient = new FtpClient(url, c.username, c.password))
    {
       // connect to client
       // and upload the file 
       await ftpClient.UploadAsync(file);
    }
}

Explanation:

You should avoid global variables. Your example is very simple but potentially if GetCredentials is called from another thread while you are in your upload loop the wrong username and password will be used.

Secondly, because the ftp upload requires no computation, its makes sense to do it asynchronously. Although, this might depend on your network bandwidth

Ewan
  • 83,178