Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am writing a simple-ish c# CLI app to improve our telephony reporting system.

At present, I have a nightly DTS package which dumps a couple of tables from the Cisco telephony box into a database on our corporate cluster. An hour later, I have a SQL Server Agent job that runs a variety of messy stored procedures to generate queue level statistics.

I was advised by a telephony consultant to do it this way, but have never been 100% happy with it, due to the lack or error handling.

I have found 6 stored procedures on the uccx Cisco telephony box, which are used by the Historical Cisco Reporting application. These contain a lot more statistics than I can currently provide.

So... my plan is as follows:

  1. Get a list of current queue names, including their open and close times from a SQL Server table

  2. Run a stored procedure x amount of times, once for each queue, passing in the parameters from step 1

  3. For each row generated from the stored procedure in step 2, write a row in a table on the SQL Server cluster

  4. Do this for each stored procedure.

My code, which works for one of the stored procedures is here.

It is very very basic, with poor errrmmm everything ha. I want to make this a bit more OO, remove duplicate code, and make it a bit better but I am not too great at that.

I can code, but not at a very advanced standard.

Any advice on how to tackle this beast? I am willing to learn new techniques, and am willing to chuck out what I've done thus far and start again. I really need to skill up, as I'll be getting more projects in a similar vein

Thanks guys and gals!

I have two classes, the main class and the logFile class:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;
using System.Configuration;

namespace UCCXtoSQL
{
public static class LogFile
{
    public static void write(string logMessage)
    {
        string message = string.Empty;
        string logFileLocation = @"C:\debug\UCCXtoSQL.log";
        StreamWriter logWriter;

        message = string.Format("{0}: {1}", DateTime.Now, logMessage);

        if (!File.Exists(logFileLocation))
        {
            logWriter = new StreamWriter(logFileLocation);
        }
        else
        {
            logWriter = File.AppendText(logFileLocation);
        }

        logWriter.WriteLine(message);

        logWriter.Close();
    }   
}



}

Main:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using System.Data.Sql;
using System.Data.SqlClient;
using System.Data.SqlTypes;

namespace UCCXtoSQL
{
class Program
{
    static void Main(string[] args)
    {
        getData();
    }

    static void getData()
    {
        string connString = @"Data Source=uccx-pri\crssql;Initial Catalog=db_cra;Integrated Security=SSPI;";
        string sql2k5ConnString = @"Data Source=sql2k5;Initial Catalog=db_cra;User Id=sqlsupport;Password=blahblahblah;";
        string procedure = @"sp_csq_activity";
        int id = 0;
        //string queueName = @"|CSQ-SHG01";

        SqlConnection conn = new SqlConnection(connString);
        SqlConnection sql2k5conn = new SqlConnection(sql2k5ConnString);

        DataTable csq = getCSQTable(sql2k5conn);

        foreach (DataRow row in csq.Rows)
        {
            id++;
            string name = row["CSQName"].ToString();
            string open = row["CSQOpen"].ToString();
            string close = row["CSQClose"].ToString();

            string format = "yyyy-MM-dd ";
            DateTime today = DateTime.Now.Date.AddDays(-1);

            Console.WriteLine(id + " " + name);
            LogFile.write(id + " " + name);

            string paramstart = (today.ToString(format) + open);
            string paramend = (today.ToString(format) + close);

            Console.WriteLine("Queue open: " + paramstart);
            Console.WriteLine("Queue close: " + paramend);

            //var Open = TimeSpan.Parse(row["CSQOpen"].ToString());
            //string statsDate = DateTime.Now.ToShortDateString(); 
            //string newDateTime = statsDate + Open;
            //Console.WriteLine("Converted " + newDateTime);

            csqActivity(paramstart, paramend, procedure, name, conn, sql2k5conn);
        }
    }

    private static void csqActivity(string pstart, string pend, string procedure, string queueName, SqlConnection conn, SqlConnection sql2k5conn)
    {
        try
        {
            queueName = @"|" + queueName;
            SqlDataAdapter da = new SqlDataAdapter();
            da.SelectCommand = new SqlCommand(procedure, conn);
            da.SelectCommand.CommandType = CommandType.StoredProcedure;
            //da.SelectCommand.Parameters.Add(new SqlParameter("@starttime", "2012-11-08 08:30:00"));
            //da.SelectCommand.Parameters.Add(new SqlParameter("@endtime", "2012-11-08 17:15:00"));
            da.SelectCommand.Parameters.Add(new SqlParameter("@starttime", pstart));
            da.SelectCommand.Parameters.Add(new SqlParameter("@endtime", pend));
            da.SelectCommand.Parameters.Add(new SqlParameter("@csqlist", queueName));

            DataSet ds = new DataSet();
            da.Fill(ds, "result_name");

            DataTable dt = ds.Tables["result_name"];

            foreach (DataRow row in dt.Rows)
            {
                //Console.WriteLine(row["CSQ_Name"]);
                Console.WriteLine("Queue: {0} - Presented: {1}", row["CSQ_Name"], row["Calls_Presented"]);
                LogFile.write("Queue: " + row["CSQ_Name"]);
                LogFile.write("Presented: " + row["Calls_Presented"]);
                InsertRecord(sql2k5conn, row, pstart, pend, queueName);
            }
        }
        catch (Exception e)
        {
            Console.WriteLine("Error: " + e);
            LogFile.write("Error: " + e);
        }
        finally
        {
            Console.WriteLine("Done");
            conn.Close();
        }
    }

    private static void InsertRecord(SqlConnection sql2k5conn, DataRow row, string start, string end, string queue)
    {
        try
        {
            DateTime startdate = Convert.ToDateTime(start);
            DateTime endDate = Convert.ToDateTime(end);

            sql2k5conn.Open();
            SqlCommand sql2k5comm = new SqlCommand("insert_csq_activity", sql2k5conn);
            sql2k5comm.CommandTimeout = 0;
            sql2k5comm.CommandType = CommandType.StoredProcedure;
            //sql2k5comm.Parameters.Add(new SqlParameter(/*PARAMNAME*/,/*PARAM*/);
            sql2k5comm.Parameters.Add(new SqlParameter("@CSQ_Name", row["CSQ_Name"]));
            //sql2k5comm.Parameters.Add(new SqlParameter("@CSQ_Name", queue));
            sql2k5comm.Parameters.Add(new SqlParameter("@Call_Skills", row["Call_Skills"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Calls_Presented", row["Calls_Presented"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Queue_Time", row["Avg_Queue_Time"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Max_Queue_Time", row["Max_Queue_Time"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Calls_Handled", row["Calls_Handled"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Speed_Answer", row["Avg_Speed_Answer"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Handle_Time", row["Avg_Handle_Time"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Max_Handle_Time", row["Max_Handle_Time"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Calls_Abandoned", row["Calls_Abandoned"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Time_Abandon", row["Avg_Time_Abandon"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Max_Time_Abandon", row["Max_Time_Abandon"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Calls_Abandoned", row["Avg_Calls_Abandoned"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Max_Calls_Abandoned", row["Max_Calls_Abandoned"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Calls_Dequeued", row["Calls_Dequeued"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Avg_Time_Dequeue", row["Avg_Time_Dequeue"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Max_Time_Dequeue", row["Max_Time_Dequeue"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@Calls_Handled_by_Other", row["Calls_Handled_by_Other"]));
            sql2k5comm.Parameters.Add(new SqlParameter("@CSQ_StartDateTime", startdate));
            sql2k5comm.Parameters.Add(new SqlParameter("@CSQ_EndDateTime", endDate));
            sql2k5comm.ExecuteNonQuery();
        }
        catch (Exception e)
        {
            Console.WriteLine("Error: " + e);
            LogFile.write("Error: " + e);
        }
        finally
        {
            sql2k5conn.Close();
        }
    }

    static DataTable getCSQTable(SqlConnection connection)
    {
        SqlDataAdapter da = new SqlDataAdapter();
        da.SelectCommand = new SqlCommand("get_csqnames", connection);
        da.SelectCommand.CommandType = CommandType.StoredProcedure;

        DataSet ds = new DataSet();
        da.Fill(ds, "csqTable");

        DataTable dt = ds.Tables["csqTable"];
        return dt;
    }


}
}
share|improve this question
1  
I've added the code from the URL. Don't want to seem as if I'm going off-topic :( –  carlhunt3r Nov 17 '12 at 23:23
add comment

1 Answer

up vote 1 down vote accepted

Your task falls into ETL (Extract-Tranform-Load) category, and SQL Server Integration Services (SSIS) is the service dedicated to ETL. You can actually implement all the processing described here using the SSIS package (next version of DTS), at it might result in faster and more reliable solution because SSIS uses stream-based processing and optimised interaction with SQL Server.

If you still want to use .NET app to do the transfer then suggestion will vary depending on how much data do you need to transfer, and how likely you would need to maintain this solution in the future.

  • If data volume is not large (i.e. less than a couple of thousands records) then you can keep using DataTables, otherwise it would be better to switch to streaming techniques (process the data while you load it)
  • If it's a one-time implementation then usually it's not worth investing lots of efforts into clean design as long as program does the job.

Since you've asked for suggestions to clean up the code, here is my list of what I would do with it:

  1. Apply .NET naming conventions (don't use Hungarian notation in particular).
  2. write method: use the using keyword for all disposable objects (StreamWriter) + you can initialise logWriter with a single line:

    public static void Write(string logMessage)
    {
        const string logFileLocation = @"C:\debug\UCCXtoSQL.log";
        using (var logWriter = new StreamWriter(logFileLocation, true))
        {
            logWriter.WriteLine(string.Format("{0}: {1}", DateTime.Now, logMessage));
        }
    }
    
  3. Replace DataSet/DataTable with Entity Framework - that's the biggest change since it will remove all the manual SqlCommand/SqlConnection/DataTable/DataSet/DataAdapter processing in favor of typed data objects. Read more on Entity Framework
  4. Replace your custom LogFile logging class with logging framework (log4net, NLog).
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.