Iterating over multiple sets of credentials

Question

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!

Possible duplicate of [Most efficient method for large switch statements](https://softwareengineering.stackexchange.com/questions/322788/most-efficient-method-for-large-switch-statements) — gnat, Jan 12 '18 at 13:15
I don't agree - I'm not asking for efficiency in the switch statement, I'm asking about how to generally doing these things, ie alternate methods. — Tauropi, Jan 12 '18 at 13:19
For starters, are you sure the username/passwords will never change? They should probably be moved out of the code and into a configuration file. That might help organize the code for you too. That way you can just pull out username/password/url from a configuration object like a Dictionary. — neilsimp1, Jan 12 '18 at 13:26
Great idea. I'll move them into environment variables, unless a dictionary is much better? I read that environment variables are safer. — Tauropi, Jan 12 '18 at 13:30
Questions asking for assistance in explaining, writing or debugging code are off-topic here. Voting to close. — David Arno, Jan 12 '18 at 13:40
conceptually this question is no different from the 'how do i simplify this if..else code' which is so popular atm — Ewan, Jan 12 '18 at 17:05
I'm actually on the fence as to whether this question is on topic ... So, let me try to give you some advice here: **Don't put credentials in your code.** Put that stuff in a config file as a list or dictionary of connection strings or something (with encrypted creds if possible). So, your code would look more like, `var endpoints = Config["endpoints"]; endpoints.forEach(t => uploadStuffTo(t); )` ... or whatever. — svidgen, Jan 13 '18 at 02:03

Ewan · Answer 1 · 2018-01-12T17:08:18.373

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

edited Jan 12 '18 at 17:08

answered Jan 12 '18 at 16:51

Ewan

70,664
5
76
161

1

downvotes due to spelling errors? – Ewan Jan 12 '18 at 16:55
Code without explanation (though I didn't downvote). – Robert Harvey Jan 12 '18 at 16:56
1

my code being so squeeky clean needs no explaination!!!!!! – Ewan Jan 12 '18 at 16:57
That wasn't the explanation I was expecting. I was expecting you to say something like "Favor lists over if/else ladders whenever possible, because they are iterable" (in simpler language, of course, since the OP is quite inexperienced). – Robert Harvey Jan 12 '18 at 18:02
1

LISTS ARE BETTAR!!! – Ewan Jan 12 '18 at 19:02
1

Umm. Well, I'll give you a `-1` for offering a solution that *still has a list of credentials baked into source code* ... A practice we may all be guilty of at some point. Not a practice we should be proud of or publicly *suggest.* – svidgen Jan 13 '18 at 02:05
2

I guess you always get a few down votes from people who stop reading at the top – Ewan Jan 13 '18 at 07:16

Iterating over multiple sets of credentials

1 Answers1