Improving a method to guarantee uniqueness

I have just written something to guarantee a unique URL when creating pages within an old legacy system (sounds odd doing this retrospectively but we've just changed how the routing works). I don't often end up using while and I think this may be why I feel it can be improved.

private string GetUniqueUrl(Page parentPage, Page newPage)
{
    var isNotDuplicatateUrl = false;
    var version = 0;
    var baseUrl = parentPage != null ? parentPage.FullUrl.Trim() + "/" : string.Empty;

    while (isNotDuplicatateUrl == false)
    {
        var friendlyUrl = version > 0 ? (newPage.FriendlyURLName + "-" + version) : newPage.FriendlyURLName;
        var fullUrl = baseUrl + friendlyUrl;

        var duplicateUrlPage =
            Site.Pages.FirstOrDefault(x => x.FullUrl != null && string.Equals(x.FullUrl.Trim(), fullUrl.Trim(), StringComparison.CurrentCultureIgnoreCase));

        if (duplicateUrlPage == null)
        {
            isNotDuplicatateUrl = true;
        }
        else
        {
            version++;
        }
    }

    return newPage.FriendlyURLName + "-" + version;
}

NB: I know Page.FriendlyURLName should be Page.FriendlyUrlName but I'm only looking for improvements within the scope of this method.

Having a loop to check each version of the URL could become a problem if there end up being a lot of URL's. There many be a better way to do it using a hash, or some other means of persisting data, and retrieving the most recently used version.

Still, even with your loop code, there's a way to do it in a simpler fashion, using a helper method to check for a duplicate URL is a start.....

private bool PageFound(string baseUrl, string friendlyURL)
{
    var fullUrl = baseUrl + friendlyUrl;
    var duplicateUrlPage = Site.Pages.FirstOrDefault(
                 x => x.FullUrl != null
              && string.Equals(x.FullUrl.Trim(), fullUrl.Trim(),
                       StringComparison.CurrentCultureIgnoreCase));
    return duplicateUrlPage != null;
}

private string GetUniqueUrl(Page parentPage, Page newPage)
{
    var version = 0;
    var baseUrl = parentPage != null ? parentPage.FullUrl.Trim() + "/" : string.Empty;
    var friendlyUrl = newPage.FriendlyURLName;
    while (PageFound(baseUrl, friendlyURL))
    {
        version++;
        friendlyUrl = newPage.friendlyURLName + "-" + version;
    }
    return newPage.FriendlyURLName + "-" + version;
}

There are two things to pay attention to here.

• Note how the first time the duplicate is checked, there's no 'version' on the URL? The version only gets added for version >=1. This is 'managed' by doing the first URL construction outside the loop, and the rest inside the loop. This saves the conditional on the version == 0

• I believe there's a bug in your code (which I have reproduced in my suggestion). If you search for the page without the version (version = 0) and you do not find one, you return the URL with the version anyway (you append "-0"). Are you sure this is what you want to do? If it is not what you want, you can just return friendlyUrl instead of newPage.FriendlyURLName + "-" + version

Using a helper function to check for the duplicate is a good way to put a code block in to a conditional clause for a loop. In essence, by creating a function, we have also created a powerful while loop check. The 'problem' with your code is that it was trying to do condition-loop logic inside the execution block, instead of inside the loop clause
2.

StackExchange Code Review Q#71759, answer score: 5