Conference Scheduler

Problem


You are planning a big programming conference and have received many
proposals which have passed the initial screen process but you're
having trouble fitting them into the time constraints of the day --
there are so many possibilities! So you write a program to do it for
you.


• The conference has multiple tracks each of which has a morning and afternoon session.

• Each session contains multiple talks.

• Morning sessions begin at 9am and must finish by 12 noon, for lunch.

• Afternoon sessions begin at 1pm and must finish in time for the networking event.

• The networking event can start no earlier than 4:00 and no later than 5:00.
• No talk title has numbers in it.

• All talk lengths are either in minutes (not hours) or lightning (5 minutes).

• Presenters will be very punctual; there needs to be no gap between sessions.

Test input:


Writing Fast Tests Against Enterprise Rails 60min


Overdoing it in Python 45min


Lua for the Masses 30min


Ruby Errors from Mismatched Gem Versions 45min


Common Ruby Errors 45min


Rails for Python Developers lightning


Communicating Over Distance 60min


Accounting-Driven Development 45min


Woah 30min


Sit Down and Write 30min


Pair Programming vs Noise 45min


Rails Magic 60min


Ruby on Rails: Why We Should Move On 60min


Clojure Ate Scala (on my project) 45min


Programming in the Boondocks of Seattle 30min


Ruby vs. Clojure for Back-End Development 30min


Ruby on Rails Legacy App Maintenance 60min


A World Without HackerNews 30min


User Interface CSS in Rails Apps 30min

Test output:


Track 1:


09:00AM Writing Fast Tests Against Enterprise Rails 60min


10:00AM Overdoing it in Python 45min


10:45AM Lua for the Masses 30min


11:15AM Ruby Errors from Mismatched Gem Versions 45min


12:00PM Lunch


01:00PM Ruby on Rails: Why We Should Move On 60min


02:00PM Comm

Only focusing on ScheduleTalks()

This method is violating the single responsibility principle because it is filling the Tracks, format the output and writing the output to file.

You should consider to extract these parts to separate methods which makes your code easier to read and also better to maintain.

That being said, let us take a look at the code

if (talks.Count() == 0)
        {
            Console.WriteLine("No talks to schedule");
            return;
        }

seeing using the Count() method instead of the Count property on a List always irritates me. Under the hood it will call for a ICollection the Count property also, but needs a cast to check if the object is a ICollection.

From the reference source

ICollection collectionoft = source as ICollection;
        if (collectionoft != null) return collectionoft.Count;

Will this

double totalDuration = talks.Sum(x => x.Duration);
            int numOfTracks = (totalDuration ();
            int maxSet = talks.Count() > 6 ? 6 : talks.Count() - 1;

ever be able to throw an exception ? I guess it won't. You should only place code inside a try..catch which throws an exception and the thrown exception should/could be handled.

Catching the general Exception isn't good either. You should always catch specific exceptions and if you need the algorithm not being interupted by an uncaught exception add the catching of Exception as the last catch.

This

for (int i = 0; i  0)
            {
                int remainingTalksDuration = talks.Sum(x => x.Duration);
                for (; maxSet > 0; --maxSet)
                {
                    for (int index = 0; index  0; ++index)
                    {
                        AllocateSessions(talks, index, Track.TotalMinInMorningSession, SessionType.MorningSession, maxSet);
                        AllocateSessions(talks, index, Track.TotalMinInAfterNoonSession, SessionType.EveningSession, maxSet);
                    }
                }
            }

and especially this

if (talks.Count() > 0)

irritated me and I needed to check the AllocateSessions() method. I din't expect that somehow magically the items of the talks list will be reduced. I would call this a side effect of the method which shouldn't happen like this.

A better and more obvious way would be to return a List form the method which is then assigned to the talks variable. By breaking out of both loops if talks.Count == 0 the inner loop condition would be more obvious.
A loop like for (; maxSet > 0; --maxSet) is IMHO not readable and should be replaced.

This would change the code above to

for (int i = 0; i  x.Duration);

            for (int mSet = maxSet; mSet > 0; --mSet)
            {
                if (talks.Count == 0) { break; }
                for (int index = 0; index < numOfTracks; ++index)
                {
                    talks = AllocateSessions(talks, index, Track.TotalMinInMorningSession, SessionType.MorningSession, mSet);
                    talks = AllocateSessions(talks, index, Track.TotalMinInAfterNoonSession, SessionType.EveningSession, mSet);
                    if (talks.Count == 0) { break; }
                }
            }

After these loops your Tracks property will be filled and the List talks won't be needed anymore, so you could anything until now extract to a method private void FillTracks(List talks).

Then I would create a method private string ComposeOutput() which composes the output based on the Tracks property by using a StringBuilder.

Writing the output to file or console should then be done by a method private void Export(string content) leaving you former ScheduleTalks() method like so

public void ScheduleTalks(List talks)
{

    if (talks.Count == 0)
    {
        Console.WriteLine("No talks to schedule");
        return;
    }

    FillTracks(talks);

    string output = ComposeOutput();

    Export(output);

}

StackExchange Code Review Q#98532, answer score: 3