SharePoint and JSON output project

Question

I'm working on a project that reads a series of SharePoint lists and outputs them into JSON file(s) which will later be uploaded via FTP to use on a website.

I'm aware that there's a lot I could be doing better, but I need some direction on what these areas are and what I can be doing to improve upon my development skills. Any feedback, good or bad, would be greatly appreciated.

Full project can be downloaded here

Program.cs

using System;
using System.Configuration;
using System.IO;
using System.Reflection;
using code.runner.intranet.Events;
using code.runner.intranet.JSON;
using NLog;
namespace code.runner
{
 class Program
 {
 // create our NLOG references
 private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
 // create our NLOG type
 private enum LogType
 {
 Debug,
 Info,
 Error
 };
 static void Main(string[] args)
 {
 CLog(LogType.Info, "Code Runner started with version " + Assembly.GetAssembly(typeof(Program)).GetName().Version);
 ProcessVolunteerOpportunities( ConfigurationManager.AppSettings["VCECurrentOpportunitiesOutputFile"],
 ConfigurationManager.AppSettings["VCECurrentOpportunitiesSPSite"],
 ConfigurationManager.AppSettings["VCECurrentOpportunitiesSPList"]);
 }
 private static void ProcessVolunteerOpportunities(string outputFile, string sharePointSite,
 string sharepointList)
 {
 try
 {
 var r = new VCECurrentOpportunities();
 // Attach VCE event handler
 r.OnEventHandler += SharePointEventHandler;
 try
 {
 // retrieve list and write to a new JSON file
 using (var jsonFile = new StreamWriter(outputFile))
 {
 jsonFile.Write(r.RetreiveOpportunities(sharePointSite, sharepointList));
 CLog(LogType.Info, "Successfully wrote local opportunities to: " + outputFile);
 }
 }
 catch (Exception ex)
 {
 CLog(LogType.Error, "Failed to write available opportunities to local JSON file: " + outputFile, ex);
 }
 }
 catch (Exception ex)
 {
 CLog(LogType.Error, "General error when processing volunteer opportunities", ex);
 }
 }
 /// <summary>
 /// Log messages to file and the console
 /// </summary>
 /// <param name="type">The type of log event to write (Debug, Info, Error)</param>
 /// <param name="message">Optional verbose message to be included in the logs</param>
 /// <param name="e">Optional, pass an exception to the stack</param>
 private static void CLog(LogType type, string message = "", Exception e = null)
 {
 while (true)
 {
 switch (type)
 {
 case LogType.Debug:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Logger.Debug(String.Format("[{0}] {1}", DateTime.Now.ToShortTimeString(), message));
 break;
 case LogType.Info:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Logger.Info(String.Format("[{0}] {1}", DateTime.Now.ToShortTimeString(), message));
 break;
 case LogType.Error:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Console.WriteLine("[{0}] Exception details: {1}", DateTime.Now.ToShortTimeString(), e);
 Logger.Error(String.Format("[{0}] {1}\n", DateTime.Now.ToShortTimeString(), message));
 Logger.Error(String.Format("[{0}] Exception details: {1}", DateTime.Now.ToShortTimeString(), e));
 break;
 default:
 type = LogType.Error;
 e = new Exception("Unknown logging type.");
 continue;
 }
 break;
 }
 }
 /// <summary>
 /// Method to handle events from our class and post them to NLOG and the Console
 /// </summary>
 /// <param name="sender"></param>
 /// <param name="e">SharePoint event object</param>
 static void SharePointEventHandler(object sender, SharePointEventArgs e)
 {
 switch (e.ExceptionType)
 {
 case SharePointEventArgs.ExceptionLevel.Debug:
 CLog(LogType.Debug, e.Message);
 break;
 case SharePointEventArgs.ExceptionLevel.Info:
 CLog(LogType.Info, e.Message);
 break;
 case SharePointEventArgs.ExceptionLevel.Error:
 CLog(LogType.Error, e.Message, e.Exception);
 break;
 }
 }
 }
}

VCECurrentOpportunities class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using code.runner.intranet.Dal;
using code.runner.intranet.Entity;
using code.runner.intranet.Events;
using Microsoft.SharePoint;
using Newtonsoft.Json.Linq;
namespace code.runner.intranet.JSON
{
 public class VCECurrentOpportunities
 {
 /// <summary>
 /// Retreive List of current volunteer opportunities
 /// </summary>
 public string RetreiveOpportunities(string siteName, string listUrl)
 {
 try
 {
 PostEvent("Loaded VCE Current Opportunities Module", SharePointEventArgs.ExceptionLevel.Info);
 using (var site = new SPSite(siteName))
 {
 using (var web = site.OpenWeb())
 {
 PostEvent("Successfully opened: " + web.Url, SharePointEventArgs.ExceptionLevel.Info);
 PostEvent("Loading list: " + listUrl, SharePointEventArgs.ExceptionLevel.Info);
 var coDal = new VCECurrentOpportunitiesDAL();
 SPList list = web.GetList(listUrl);
 List<VCECurrentOpportunitiesEntity> currentOpportunities = coDal.FetchItems(list);
 PostEvent("Preparing JSON output for " + listUrl, SharePointEventArgs.ExceptionLevel.Info);
 // initalize and specify that our return value (in JSON) contains an array
 var json = "[";
 foreach (var item in currentOpportunities)
 {
 // create a new JSON object for use within our array
 var o = new JObject();
 o["positionTitle"] = item.PositionTitle;
 o["region"] = item.Region;
 // split locations choice field from SharePoint format into a processable array
 string[] locationsArray = null;
 if (item.Location != null)
 locationsArray = item.Location.Split(new[] {";#"}, StringSplitOptions.RemoveEmptyEntries);
 var location = new JArray {locationsArray};
 o["location"] = location;
 o["jobDescription"] = item.JobDescription;
 o["fileName"] = item.FileName;
 o["fileLocation"] = item.FileLocation;
 o["department"] = item.Department;
 json += o.ToString();
 // only add a comma on our object if it isn't the last one in the array
 if (!item.Equals(currentOpportunities.Last())) json += ",";
 }
 // close our JSON array
 json += "]";
 return json;
 }
 }
 }
 catch (UnauthorizedAccessException ex)
 {
 PostEvent("Unable to access SharePoint list for Current Opportunities", SharePointEventArgs.ExceptionLevel.Error, ex);
 return ex.ToString();
 }
 catch (Exception ex)
 {
 PostEvent("General error when retreiving opportunities", SharePointEventArgs.ExceptionLevel.Error, ex);
 return ex.ToString();
 }
 }
 /// <summary>
 /// Internal event handler allowing for logging of events within the class
 /// </summary>
 private EventHandler<SharePointEventArgs> _onEvent;
 /// <summary>
 /// Public event handler allowing for accessibility outside of the class
 /// </summary>
 public event EventHandler<SharePointEventArgs> OnEventHandler
 {
 add { _onEvent += value; }
 remove { _onEvent += value; }
 }
 public void PostEvent(string message, SharePointEventArgs.ExceptionLevel exceptionLevel, Exception exception = null)
 {
 if (_onEvent == null) return;
 if (exception == null)
 {
 var e = new SharePointEventArgs(message, exceptionLevel);
 _onEvent(this, e);
 }
 else
 {
 var e = new SharePointEventArgs(message, exceptionLevel, exception);
 _onEvent(this, e);
 }
 }
 }
}

SharePointEventArgs class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace code.runner.intranet.Events
{
 public class SharePointEventArgs : EventArgs
 {
 public SharePointEventArgs(string message, ExceptionLevel exceptionType, Exception exception = null)
 {
 Message = message;
 ExceptionType = exceptionType;
 if (exception != null) Exception = exception;
 }
 /// <summary>
 /// Property to allow the storage of a more verbose and explainable error message
 /// </summary>
 public string Message { get; private set; }
 /// <summary>
 /// Object to store full exception information
 /// </summary>
 public Exception Exception { get; private set; }
 /// <summary>
 /// Stores the type of exception being sent
 /// </summary>
 public ExceptionLevel ExceptionType { get; private set; }
 /// <summary>
 /// Enum to store the types of exceptions that can be sent to the event
 /// </summary>
 public enum ExceptionLevel
 {
 Debug,
 Info,
 Error
 }
 }
}

VCECurrentOpportunitiesEntity class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Microsoft.SharePoint;
namespace code.runner.intranet.Entity
{
 /// <summary>
 /// Entity for tracking current opportunitiy data
 /// </summary>
 public class VCECurrentOpportunitiesEntity : ItemEntity
 {
 public string PositionTitle { get; set; }
 public string Department { get; set; }
 public string Region { get; set; }
 public string Location { get; set; }
 public string JobDescription { get; set; }
 public string FileName { get; set; }
 public string FileLocation { get; set; }
 }
}

VCECurrentOpportunitiesDAL class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Sockets;
using System.Text;
using code.runner.intranet.Entity;
using Microsoft.SharePoint;
namespace code.runner.intranet.Dal
{
 public class VCECurrentOpportunitiesDAL : ItemDal
 {
 /// <summary>
 /// Fetch items from SharePoint and populate them into our entity
 /// </summary>
 /// <param name="list"></param>
 /// <returns></returns>
 public List<VCECurrentOpportunitiesEntity> FetchItems(SPList list)
 {
 try
 {
 return (from SPListItem item in list.Items
 select LoadItems(
 item["Position Title"].ToString(),
 item["Department"].ToString(),
 item["Region"].ToString(),
 item["Location"].ToString(),
 item["Job Description"].ToString() ,
 item["File Name (Job Description)"].ToString() ,
 item["File Location (Job Description)"].ToString() ,
 item.ID.ToString(), // item id
 item.ContentType.Name, // content type
 item.DisplayName, // display name
 item.Name, // name
 "", // title
 item.Url, // url
 item["Created By"].ToString(), // author
 item["Modified By"].ToString(), // editor
 Convert.ToDateTime(item["Created"]), // date time modified
 item["Created By"].ToString(), // modified by
 Convert.ToDateTime(item["Created"]), // date time created
 item["Created By"].ToString() // created by
 )).ToList();
 }
 catch (Exception ex)
 {
 Console.WriteLine("Error fetching list items: {0}", ex);
 throw;
 }
 }
 public VCECurrentOpportunitiesEntity LoadItems(
 string positionTitle, string department, string region, string location,
 string jobDescription, string fileName, string fileLocation, string itemId, string contentType, 
 string displayName, string name, string title,
 string url, string author, string editor, DateTime modified, string modifiedBy,
 DateTime created, string createdBy)
 {
 var vceCurrentOpportunitiesEntity = new VCECurrentOpportunitiesEntity
 {
 // Current opportunities specific data
 PositionTitle = positionTitle,
 Department = department,
 Region = region,
 Location = location,
 JobDescription = jobDescription,
 FileName = fileName,
 FileLocation = fileLocation,
 // Common SharePoint List data
 ItemId = itemId,
 ContentType = contentType,
 DisplayName = displayName,
 Name = name,
 Title = title,
 Url = url,
 Author = author,
 Editor = editor,
 Modified = modified,
 ModifiedBy = modifiedBy,
 Created = created,
 CreatedBy = createdBy
 };
 return vceCurrentOpportunitiesEntity;
 }
 }
}

Answer 1

public enum ExceptionLevel
{
 Debug,
 Info,
 Error
}

And

private enum LogType
{
 Debug,
 Info,
 Error
};

Both are enums, and both have the same values. This tells me you should probably merge these into one enum.

---

while (true)
{
 switch (type)
 {
 case LogType.Debug:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Logger.Debug(String.Format("[{0}] {1}", DateTime.Now.ToShortTimeString(), message));
 break;

 case LogType.Info:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Logger.Info(String.Format("[{0}] {1}", DateTime.Now.ToShortTimeString(), message));
 break;
 case LogType.Error:
 Console.WriteLine("[{0}] {1}", DateTime.Now.ToShortTimeString(), message);
 Console.WriteLine("[{0}] Exception details: {1}", DateTime.Now.ToShortTimeString(), e);
 Logger.Error(String.Format("[{0}] {1}\n", DateTime.Now.ToShortTimeString(), message));
 Logger.Error(String.Format("[{0}] Exception details: {1}", DateTime.Now.ToShortTimeString(), e));
 break;
 default:
 type = LogType.Error;
 e = new Exception("Unknown logging type.");
 continue;
 }
 break;
}

If I read that correctly, it enters the loop, logs the error, and immediately exits the loop, unless the LogType is not entered there, upon which it sets the LogType to Error and loops again. I would probably do this with a guard clause rather than a loop which under all normal circumstances should never actually loop. Also, I expect R# would tell you the default case is never hit, and may be removed unless you have plans to add future values to LogType.

This is how I would do this:

var currentTime = DateTime.Now.ToShortTimeString();
if (type != LogType.Debug &&
 type != LogType.Info &&
 type != LogType.Error)
{
 type = LogType.Error;
 e = new Exception("Unknown logging type.");
}
switch (type)
{
 case LogType.Debug:
 Console.WriteLine("[{0}] {1}", currentTime, message);
 Logger.Debug(String.Format("[{0}] {1}", currentTime, message));
 break;
 case LogType.Info:
 Console.WriteLine("[{0}] {1}", currentTime, message);
 Logger.Info(String.Format("[{0}] {1}", currentTime, message));
 break;
 case LogType.Error:
 Console.WriteLine("[{0}] {1}", currentTime, message);
 Console.WriteLine("[{0}] Exception details: {1}", currentTime, e);
 Logger.Error(String.Format("[{0}] {1}\n", currentTime, message));
 Logger.Error(String.Format("[{0}] Exception details: {1}", currentTime, e));
 break;
}

---

Braces help prevent bugs. I would use braces here:

if (_onEvent == null) return;

---

Finally, since you are writing all the values into the log file, and are not removing any of them programmatically, I would probably not log to the screen. If you really want a notification, just set a Boolean value and write to the screen once after everything is done and logged.