C# How to optimize this small piece of code correctly?

166 views Asked by At

I've written a small program where I need to insert some objects (employees in my case) in my MongoDB database. Each employee needs to get a random name from a predefined list with strings.

This is what I came up with and is working:

class Program
{
    public class Employee
    {
        public string Name { get; set; }
    }

    static void Main(string[] args)
    {

        MongoClient client = new MongoClient();
        var server = client.GetServer();
        var db = server.GetDatabase("testdb");
        var collection = db.GetCollection<Employee>("testcollection");

        Random random = new Random();
        List<string> names = new List<string>() {"John","Matthew","Chris"};
        for (int i = 0; i < 5; i++)
        {
            int nameIndex      = random.Next(0, 3);  
            string nameValue   = names[nameIndex];  
            Employee employee = new Employee
            { 
                Name = nameValue
            };

           collection.Save(employee);
         }
     }

}

However, this code feels like a bit dirty and I am lookingfor a nice way to clean this up. Especially if I want to add more properties to my employee object.

Edit: @Eric Lippert Of course! To start with, practically everything happens in the main method. With a single list it's fine, but I'd like to add more data to my employee object. It won't be tiny in the future.

So, if I want to add a few more List's, I have to duplicate my current code. For example, I'd like to iterate over another list with job types (10 items) and another one for locations (50 items). For each property I want to add to my Employee-object, I have to write a new line of the Next()-function from my Random object.

Edit 3:

public static IMongoDatabase Connect()
{
    MongoClient client = new MongoClient();
    var db = client.GetDatabase("testdb");
    return db;
}

static void Main(string[] args)
{
    Connect();
    var collection = db.GetCollection<Employee>("testcollection");
}

Error: The name 'db' doesnt exist in the current context.

2

There are 2 answers

8
Dacker On

If you want to apply more random values from other lists you can do the following, which basically comes down to removing the unnecessary nameIndex and nameValue variables:

for (int i = 0; i < 5; i++)
{
    Employee employee = new Employee
    { 
        Name = names[random.Next(0, names.Count)]
    };

    collection.Save(employee);
}
5
Kevin Smith On

You could abstract the separate parts in to new classes say a DatabaseSeeder and Database, this just cleans up the responsibilities instead of having it all in the main method

class Program
{
    public class Employee
    {
        public string Name { get; set; }
    }

    public class Database
    {
        public ICollection<Employee> GetCollection()
        {
             MongoClient client = new MongoClient();
            var server = client.GetServer();
            var db = server.GetDatabase("testdb");
            var collection = db.GetCollection<Employee>("testcollection");
        }
    }

    public class DatabaseSeeder
    {
        private ICollection<Employee> collection;

        public DatabaseSeeder(ICollection<Employee> collection)
        {
            this.collection = collection;
        }

        public void Seed()
        {
            Random random = new Random();
            List<string> names = new List<string>() {"John","Matthew","Chris"};
            for (int i = 0; i < 5; i++)
            {
                int nameIndex      = random.Next(0, 3);  
                string nameValue   = names[nameIndex];  
                Employee employee = new Employee
                { 
                    Name = nameValue
                };

               collection.Save(employee);
             }
        }

    }

    static void Main(string[] args)
    {
        var collection = new Database().GetCollection();

        var databaseSeeder = new DatabaseSeeder(collection);

        databaseSeeder.Seed();
    }

}