SharePoint class

Question

To help learn the SharePoint object model (primarily for SharePoint 2007), I've been working on a class library with a number of useful functions. I'm a junior developer and this is my first C# project of this nature.

With this in mind, how can I improve the class below? What bad habits do I have or what should I study so I can code better?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Microsoft.SharePoint;
using System.Diagnostics;

// todo: Lists
// MoveListItems
// MoveListItemsSiteToSite
// DeleteListItem
// WriteFileMetadata
// AddListEntry

// Throttling on copy methods

namespace DEVTEST.SharePoint
{
    /// <summary>
    /// Class to encapsulate methods that interact with SharePoint Lists and Libraries
    /// </summary>
    public class ListsAndItems
    {
        /// <summary>
        /// Access and modify items in SharePoint lists within the same site
        /// </summary>
        /// <param name="siteURL"></param>
        /// <param name="sourceList"></param>
        /// <param name="destinationList"></param>
        /// <param name="retainMeta"></param>
        public static void MoveListItems(string siteURL, string sourceList, string destinationList, bool retainMeta)
        {
            Console.WriteLine("[{0}] Opening site: {1}", DateTime.Now.ToShortTimeString(), siteURL);

            using (var site = new SPSite(siteURL))
            {
                Console.WriteLine("[{0}] Opened site: {1}", DateTime.Now.ToShortTimeString(), siteURL);

                using (var web = site.OpenWeb())
                {
                    Console.WriteLine("[{0}] Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(), web.ServerRelativeUrl);

                    try
                    {
                        // Get your source and destination libraries
                        var source = web.GetList(web.ServerRelativeUrl + sourceList);
                        var destination = web.GetList(web.ServerRelativeUrl + destinationList);

                        Console.WriteLine("[{0}] Source set to: {1}", DateTime.Now.ToShortTimeString(), source);
                        Console.WriteLine("[{0}] Destination set to: {1}", DateTime.Now.ToShortTimeString(), destination);

                        // Get the collection of items to move, use source.GetItems(SPQuery) if you want a subset
                        SPListItemCollection items = source.Items;

                        // Get the root folder of the destination we'll use this to add the files
                        SPFolder folder = web.GetFolder(destination.RootFolder.Url);

                        Console.WriteLine("[{0}] Moving {1} files from {2} to {3} - please wait...", DateTime.Now.ToShortTimeString(),
                                          items.Count, source, destination);

                        var fileCount = 0;
                        // Now to move the files and the metadata
                        foreach (SPListItem item in items)
                        {
                            //Get the file associated with the item
                            SPFile file = item.File;

                            // Create a new file in the destination library with the same properties
                            SPFile newFile = folder.Files.Add(folder.Url + "/" + file.Name, file.OpenBinary(), file.Properties, true);

                            // Optionally copy across the created/modified metadata                        
                            if (retainMeta)
                            {
                                SPListItem newItem = newFile.Item;
                                WriteFileMetaDataFiletoFile(item, newItem);
                            }

                            // Delete the original version of the file
                            // todo: make local backup before deleting?
                            file.Delete();

                            fileCount++;
                        }
                        Console.WriteLine("[{0}] Completed moving {1} files to {2}", DateTime.Now.ToShortTimeString(), fileCount,
                                          destination);

                    }
                    catch (System.IO.FileNotFoundException)
                    {
                        Console.WriteLine(
                            "[{0}] Unable to set a location. Please check that paths for source and destination libraries are correct and relative to the site collection.",
                            DateTime.Now.ToShortTimeString());

                        throw;
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine("[{0}] Exception: {1}", DateTime.Now.ToShortTimeString(), ex);

                        throw;
                    }
                }
            }
        }

        /// <summary>
        /// Access and modify items in SharePoint lists in differing sites
        /// </summary>
        /// <param name="sourceSiteURL"></param>
        /// <param name="sourceList"></param>
        /// <param name="destinationSiteURL"></param>
        /// <param name="destinationList"></param>
        /// <param name="retainMeta"></param>
        public static void MoveListItemsSiteToSite(string sourceSiteURL, string sourceList, string destinationSiteURL,
                                                   string destinationList, bool retainMeta)
        {
            Console.WriteLine("[{0}] Opening Source site: {1}", DateTime.Now.ToShortTimeString(), sourceSiteURL);

            using (var sourceSite = new SPSite(sourceSiteURL))
            {
                Console.WriteLine("[{0}] Opened Source site: {1}", DateTime.Now.ToShortTimeString(), sourceSiteURL);

                using (var sourceWeb = sourceSite.OpenWeb())
                {
                    Console.WriteLine("[{0}] Source Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(), sourceWeb.ServerRelativeUrl);

                    try
                    {
                        // Get your source library
                        var source = sourceWeb.GetList(sourceWeb.ServerRelativeUrl + sourceList);
                        Console.WriteLine("[{0}] Source set to: {1}", DateTime.Now.ToShortTimeString(), source);

                        // Get the collection of items to move, use source.GetItems(SPQuery) if you want a subset
                        SPListItemCollection items = source.Items;

                        var fileCount = 0;

                        Console.WriteLine("[{0}] Opening Destination site: {1}", DateTime.Now.ToShortTimeString(), destinationSiteURL);

                        using (var destSite = new SPSite(destinationSiteURL))
                        {
                            Console.WriteLine("[{0}] Opened Destination site: {1}", DateTime.Now.ToShortTimeString(), destSite);

                            using (var destinationWeb = destSite.OpenWeb())
                            {

                                Console.WriteLine("[{0}] Destination Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(),
                                                  destinationWeb.ServerRelativeUrl);

                                // get destination library
                                var destination = destinationWeb.GetList(destinationWeb.ServerRelativeUrl + destinationList);
                                Console.WriteLine("[{0}] Destination set to: {1}", DateTime.Now.ToShortTimeString(), destination);

                                // Get the root folder of the destination we'll use this to add the files
                                SPFolder destinationFolder = destinationWeb.GetFolder(destination.RootFolder.Url);


                                Console.WriteLine("[{0}] Moving {1} files from {2} to {3} - please wait...", DateTime.Now.ToShortTimeString(),
                                                  items.Count, source, destination);


                                // Now to move the files and the metadata
                                foreach (SPListItem item in items)
                                {
                                    //Get the file associated with the item
                                    SPFile file = item.File;

                                    // Create a new file in the destination library with the same properties
                                    SPFile newFile = destinationFolder.Files.Add(destinationFolder.Url + "/" + file.Name, file.OpenBinary(),
                                                                                 file.Properties, true);

                                    // Optionally copy across the created/modified metadata                        
                                    if (retainMeta)
                                    {
                                        SPListItem newItem = newFile.Item;
                                        WriteFileMetaDataFiletoFile(item, newItem);
                                    }

                                    // Delete the original version of the file
                                    // todo: make local backup before deleting?
                                    file.Delete();

                                    fileCount++;
                                }

                                Console.WriteLine("[{0}] Completed moving {1} files to {2}", DateTime.Now.ToShortTimeString(), fileCount,
                                                  destination);
                            }

                        }


                    }
                    catch (System.IO.FileNotFoundException)
                    {
                        Console.WriteLine(
                            "[{0}] Unable to set a location. Please check that paths for source and destination libraries are correct and relative to the site collection.",
                            DateTime.Now.ToShortTimeString());

                        throw;
                    }
                    catch (Exception ex)
                    {
                        Console.WriteLine("[{0}] Exception: {1}", DateTime.Now.ToShortTimeString(), ex);

                        throw;
                    }
                }
            }
        }
        /// <summary>
        /// Overwrite existing meta data for a file with meta data from another file
        /// </summary>
        /// <param name="sourceItem">Source file to take meta data from</param>
        /// <param name="destinationItem">Destination file to write meta data to</param>
        public static void WriteFileMetaDataFiletoFile(SPListItem sourceItem, SPListItem destinationItem)
            // overwrites a list items meta data with meta data from another file
        {
            //todo: change to write individual items instead of using source item

            destinationItem["Editor"] = sourceItem["Editor"];
            destinationItem["Modified"] = sourceItem["Modified"];
            destinationItem["Modified By"] = sourceItem["Modified By"];
            destinationItem["Author"] = sourceItem["Author"];
            destinationItem["Created"] = sourceItem["Created"];
            destinationItem["Created By"] = sourceItem["Created By"];

            // UpdateOverwriteVersion() will preserve the metadata added above. 
            destinationItem.UpdateOverwriteVersion();
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="item"></param>
        /// <param name="editor"></param>
        /// <param name="modified"></param>
        /// <param name="modifiedBy"></param>
        /// <param name="author"></param>
        /// <param name="created"></param>
        /// <param name="createdBy"></param>
        public static void WriteFileMetaData(SPList item, string editor, DateTime modified, string modifiedBy, string author, DateTime created, string createdBy)
        {
            // todo
        }
    }
}

toolic · Accepted Answer · 2025-07-10 13:34:22Z

Good job on the formatting and using using.

There are a few things I can see to clean this up.

Is there a reason you are not using var to declare obvious variables? I would suggest to use it whenever you can.

I would move the Console.WriteLine logging into a separate class, probably that had an interface, that is injected or set in your class. This will allow you to easily change where the log information is going. Say in production you want it to go into a text file??

Instead of rethrowing the exception in the catch statements, throw a new exception with the original one as the inner exception:

catch (System.IO.FileNotFoundException ex)
{
    _logger.Log(string.Format("[{0}] Unable to set a location. Please check that paths for source and destination libraries are correct and relative to the site collection.",
                        DateTime.Now.ToShortTimeString()));

    throw new UnableToProcessFileException(fileName, ex);
}

This will give a bit more meaning when the exception is finally caught and displayed.

You could create a function to deal with the code inside the ForEach loop. Then you could use the link ForEach(func<>) method:

private static void ProcessSPListItem(SPListItem item)
{
    SPFile file = item.File;

    SPFile newFile = folder.Files.Add(folder.Url + "/" + file.Name, file.OpenBinary(), file.Properties, true);

    if (retainMeta)
    {
         SPListItem newItem = newFile.Item;
         WriteFileMetaDataFiletoFile(item, newItem);
    }

    // todo: make local backup before deleting?
    file.Delete();

    fileCount++;
}

...

items.ForEach(ProcessSPListItem);

I also think there are too many unnecessary comments in your code. If you have to describe what the code is doing, you need to refactor to make the intent more clear.

e.g.

// Delete the original version of the file
// todo: make local backup before deleting?
file.Delete();

If it is important to portray the fact that the original version is being deleted, this would be much better:

DeleteOriginalVersion(file);

where

private static void DeleteOriginalVersion(File file)
{
    file.Delete();
}

but for this, I would say file.Delete() speaks for itself.

I also think you have too many parameters in your function calls. The max I ever have is 3, but I do my best to limit it to 2.

If you need more, create a class with like parameters.

internal class ListInformation
{   
    public string Url{ get; private set; }
    public string List{ get; private set; } 

    public ListInformation(string url, string list)
    {
        Url = url;
        List = list;
    }
}

internal class MoveSourceToDestinationParameters
{
    public ListInformation Source { get; private set; }
    public ListInformation Destination { get; private set; }

    public MoveSourceToDestinationParameters(ListInformation source, ListInformation destination)
    {
        Source = source;
        Destination = destination;
    }
}

I'd rename the variable retainMeta to copyMetaDataWithMove - this will allow removal of more unneeded comments.

Another thing is to check for duplicate code. It's a little late, my head is not fully functioning, and I'm doing this without VS open, but I am seeing similar patterns in MoveListItems and MoveListItemsSiteToSite. If you want, I'll check tomorrow and come up with a solution.

I think overall, its not bad, but like any code (including mine) there is a bit of room for improvement.

Let me know if you have any questions.

EDIT:

Here is the file as I see it.

    public class UnableToProcessFileException : Exception
    {
        public UnableToProcessFileException(string message, Exception innerException)
            : base(message, innerException)
        {
            
        }
    }

    public class ListInformation
    {
        public string Url { get; private set; }
        public string List { get; private set; }

        public ListInformation(string url, string list)
        {
            Url = url;
            List = list;
        }
    }

    public class MoveSourceToDestinationParameters
    {
        public ListInformation Source { get; private set; }
        public ListInformation Destination { get; private set; }

        public MoveSourceToDestinationParameters(ListInformation source, ListInformation destination)
        {
            Source = source;
            Destination = destination;
        }
    }

    /// <summary>
    /// Class to encapsulate methods that interact with SharePoint Lists and Libraries
    /// </summary>
    public class ListsAndItems
    {
        private bool _copyMetaDataWithMove;

        /// <summary>
        /// Access and modify items in SharePoint lists within the same site
        /// </summary>
        /// <param name="parameters"> </param>
        /// <param name="retainMeta"></param>
        public void MoveListItems(MoveSourceToDestinationParameters parameters, bool retainMeta)
        {
            _copyMetaDataWithMove = retainMeta;

            DisplayMessage(string.Format("[{0}] Opening site: {1}", DateTime.Now.ToShortTimeString(), parameters.Source.Url));

            using (var site = new SPSite(siteURL))
            {
                DisplayMessage(string.Format("[{0}] Opened site: {1}", DateTime.Now.ToShortTimeString(), parameters.Source.Url));

                using (var web = site.OpenWeb())
                {
                    DisplayMessage(string.Format("[{0}] Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(), web.ServerRelativeUrl));
                    
                    ProcessLists(parameters, web, web);
                }
            }
        }

        /// <summary>
        /// Access and modify items in SharePoint lists in differing sites
        /// </summary>
        /// <param name="parameters"> </param>
        /// <param name="retainMeta"></param>
        public void MoveListItemsSiteToSite(MoveSourceToDestinationParameters parameters, bool retainMeta)
        {
            _copyMetaDataWithMove = retainMeta;

            DisplayMessage(string.Format("[{0}] Opening Source site: {1}", DateTime.Now.ToShortTimeString(), parameters.Source.Url);

            using (var sourceSite = new SPSite(parameters.Source.Url))
            {
                DisplayMessage(string.Format("[{0}] Opened Source site: {1}", DateTime.Now.ToShortTimeString(), parameters.Source.Url);

                using (var sourceWeb = sourceSite.OpenWeb())
                {
                    DisplayMessage(string.Format("[{0}] Source Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(), sourceWeb.ServerRelativeUrl);
                    
                    DisplayMessage(string.Format("[{0}] Opening Destination site: {1}", DateTime.Now.ToShortTimeString(), parameters.Destination.Url));

                    using (var destinationSite = new SPSite(parameters.Destination.Url))
                    {
                        DisplayMessage(string.Format("[{0}] Opened Destination site: {1}", DateTime.Now.ToShortTimeString(), destinationSite));

                        using (var destinationWeb = destinationSite.OpenWeb())
                        {

                            ProcessLists(parameters, sourceWeb, destinationWeb);
                        }

                    }
                }
            }
        }

        private void ProcessLists(MoveSourceToDestinationParameters parameters, object sourceWeb, object destinationWeb)
        {
            try
            { 
                var sourceList = sourceWeb.GetList(sourceWeb.ServerRelativeUrl + parameters.Source.List);

                DisplayMessage(string.Format("[{0}] Source list set to: {1}", DateTime.Now.ToShortTimeString(), sourceList));

                DisplayMessage(string.Format("[{0}] Destination Web Relative URL is: {1}", DateTime.Now.ToShortTimeString(),
                                             destinationWeb.ServerRelativeUrl));
                
                var desinationList = destinationWeb.GetList(destinationWeb.ServerRelativeUrl + parameters.Destination.List);

                DisplayMessage(string.Format("[{0}] Destination list set to: {1}", DateTime.Now.ToShortTimeString(), desinationList));

                MoveFiles(desinationList, sourceList, destinationWeb.GetFolder(desinationList.RootFolder.Url));
            }
            catch (System.IO.FileNotFoundException ex)
            {
                throw new UnableToProcessFileException(
                    string.Format("Unable to set a location to {0}.", ex.FileName), ex);
            }
            catch (Exception ex)
            {
                throw new UnableToProcessFileException("Unknown error moving files.", ex);
            }
        }

        private void MoveFiles(object destination, object source, SPFolder destinationFolder)
        {
            var items = source.Items;

            DisplayMessage(string.Format("[{0}] Moving {1} files from {2} to {3} - please wait...",
                                         DateTime.Now.ToShortTimeString(),
                                         items.Count, source, destination));

            var fileCount = items.Count;

            items.ForEach(item => MoveSourceFileToDestinationFolder(item, destinationFolder));

            DisplayMessage(string.Format("[{0}] Completed moving {1} files to {2}", DateTime.Now.ToShortTimeString(), fileCount,
                                         destination));
        }

        private void MoveSourceFileToDestinationFolder(SPFile item, SPFolder destination)
        {
            SPFile file = item.File;

            SPFile newFile = destination.Files.Add(destination.Url + "/" + file.Name, file.OpenBinary(), file.Properties, true);

            if (_copyMetaDataWithMove)
            {
                SPListItem newItem = newFile.Item;
                WriteFileMetaDataFiletoFile(item, newItem);
            }

            // todo: make local backup before deleting?
            file.Delete();
        }

        private static void DisplayMessage(string msg)
        {
            Console.WriteLine(msg);
        }

    ...
}

That would be great. I tried to combine the two previously but I couldn't find a way to do it and not have two site objects when it wasn't needed (it's only needed when moving across sites). I was considering just making an override for the method - but maybe there's a better way? — Michael A
– Michael A, Commented Aug 29, 2012 at 4:32
This is excellent and provides me with some valuable insight into better using properties / managing parameters. Thank-you! If I can ever pay this forward I certainly will! — Michael A
– Michael A, Commented Aug 29, 2012 at 23:04

Stack Exchange Network

SharePoint class

1 Answer 1

You must log in to answer this question.

Hot Network Questions

SharePoint class

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions