Checking if website exists and adding +1

Question

I am trying to find if a subwebsite exists in sharepoint, and if it does, then add +1.

I think the loop can be improved.

// subsiteName will be like "FruitCrateOfCompanyX"

public static string FormatMyHomieURL(string subsiteName, string mainSite)
{
    if (!mainSite.EndsWith("/"))
        mainSite = mainSite + "/";

    string temp = mainSite + subsiteName;
    int i = 0;

    while (SubwebExists(temp))
    {
        i++;
        temp = mainSite + subsiteName + i;

    }


    return subsiteName + i;    // I only need to return subSiteName which will be used later on by web services to create a unique subweb
}

public static bool SubwebExists(string url)
{
    bool exists = false;
    Uri u = new Uri(url);
    using (SPSite site = new SPSite(url))
    using (SPWeb web = site.OpenWeb(u.AbsolutePath.ToString(), true))
    {
        exists = web.Exists;
    }

    return exists;
}

Answer 1

Eliminate potential logic error in return value

I'm assuming the purpose of this code is to let you provide the name you want for your subsite, and the code will give you an available name based on whether the provided name already exists. It does this by appending/incrementing a suffix to the provided name whenever there is a naming collision. If this is the case, your FormatMyHomieURL() function should return temp sans the mainSite prefix, not subsiteName + i.

To walk through an example briefly, let's say you want to name a subsite AwesomeWeb. The code checks and AwesomeWeb doesn't exist, so it returns subsiteName + i which resolves to AwesomeWeb0. You then go ahead and create a subsite named AwesomeWeb0 and everyone is happy. One month later, you come along and decide to make a subsite named AwesomeWeb. The code checks and sure enough, AwesomeWeb doesn't exist (since you created AwesomeWeb0 last time), so the code returns subsiteName + i which again resolves to AwesomeWeb0. Now you're going to run into a naming collision.

Reduce the number of instantiated SPSite objects

You are only ever dealing with a single SPSite object in this code (the parent site that you are checking for subsites). Therefore, you don't really need to create and dispose of a new SPSite object in every iteration of your loop just to grab a subsite from it.

I recommend refactoring your code to instantiate the SPSite object outside of SubwebExists() and pass it in to SubwebExists() as a parameter, along with the name of the target subweb temp.

Answer 2

You asked about the loop.

while (SubwebExists(temp))
{
    i++;
    temp = mainSite + subsiteName + i;

}

Is this loop guaranteed to execute at least once? I suspect so, because if it doesn't, you return subsiteName + 0. I don't think that's what you want. So, the loop should be a do {} while loop, like this:

temp = mainSite + subsiteName;
do
{
    i++;
} while (SubsiteExists(temp + i));

Answer 3

It's quite late here, so just a quick answer but I can come back if this is still relevant to you.

public static string FormatMyHomieURL(string subsiteName, string mainSite)
{
    if (subsiteName == null) throw new ArgumentNullException(nameof(subsiteName));
    if (mainSite == null) throw new ArgumentNullException(nameof(mainSite));

    var url = AppendPaths(mainSite, subsiteName);
    var i = 0;

    try
    {
        using (var site = new SPSite(url))
        {
            while (SubWebExists(site, new Uri(url, UriKind.Absolute)))
            {
                url = $"{url}{i}";
                i++;
            }
        }
    }
    catch (UnauthorizedAccessException ex)
    {
        throw new UnauthorizedAccessException(nameof(mainSite));
    }

    return url;
}

public static bool SubWebExists(SPSite site, Uri url)
{
    if (site == null) throw new ArgumentNullException(nameof(site));
    if (url == null) throw new ArgumentNullException(nameof(url));

    using (var web = site.OpenWeb(url.AbsolutePath, true))
    {
        return web.Exists;
    }
}

public static string AppendPaths(string path1, string path2)
{
    if (path1 == null) throw new ArgumentNullException(nameof(path1));
    if (path2 == null) throw new ArgumentNullException(nameof(path2));

    return !path1.EndsWith("/") || !path2.StartsWith("/")
        ? (!path1.EndsWith("/") && !path2.StartsWith("/") ? path1 + "/" + path2 : path1 + path2)
        : path1 + path2.Substring(1);
}

This does need more refactoring (quite a bit of erroneous code) but the main points which I felt needed covering were:

• Although I haven't added anything within this code block I hope you're using try and catch blocks in your production code to handle Exceptions such as System.UnauthorizedAccessException - quite a common occurance with these kinds of solutions.

• The ability to handle instances where trailing and preceding "/"'s were added erroneously into the input. For that reason I've put in AppendPath's. There's likely better ways to do this and I suggest researching this a bit.

• Prevent multiple uses of SPSite. You should pass this into your method, not open a new site instance each time (a huge no, no). I think this could be further improved so you don't need the Uri but I've run out of time to look further. Let me know if you would like me to revisit.
• Web.Exists already returns a boolean value (documentation) so you can return it directly.