Alternative way of implementation of a code

Question

The title is not exactly meaningful, but I am not share what else to name it. I wrote a TOC Generation code sometime later. Based on this I was writing code to check for duplicates also. The code is as follows

curNumber = getTOCReference(selItem.SNo, IsParent);
CheckForDuplicates(curNumber, IsParent,out realTOCRef);
curNumber = realTOCRef;

And the code for CheckForDuplicates is

        ListViewItem curItem = this.tlvItems.FindItemWithText(curNumber);

        if (curItem != null)
        {
            curNumber = this.getTOCReference(curNumber, !IsParent);
            CheckForDuplicates(curNumber, IsParent,out realTOCRef);
        }
        else
        {
            realTOCRef= curNumber;
        }

What this code does, it gets a TOC and tries it find if it already exists in the ObjectListView and gets a new TOC if there is a existing TOC. Once it determines that the generated TOC is not there in the list it stores it in realTOCRef and sends it back to the main calling code.

I have used "out" to return the last generated TOC, which is something I did not want to do. The reason why I did it was after the non duplicate TOC was generated the return was not going back to the calling code instead it looped through the previous instances then it returned back. When the looping back happened the TOC that was to be returned also got reset.

I would appreciate any help on this.

Answer 1

Did I miss some information in the above post ? I would be happy to provide more information if someone is not able to understand the above post.

Answer 2

I am not sure what are you looking for.

What I assume is that you are looking fore something like following.

Existing call:

CheckForDuplicates(curNumber, IsParent,out realTOCRef);
curNumber = realTOCRef;

Required call:

curNumber =CheckForDuplicates(curNumber, IsParent);

If yes, simple change the code to following it will work.

        ListViewItem curItem = this.tlvItems.FindItemWithText(curNumber);

        if (curItem != null)
        {
            curNumber = this.getTOCReference(curNumber, !IsParent);
            return  CheckForDuplicates(curNumber, IsParent);
        }
        else
        {
            return curNumber;
        }

Answer 3

When you have a single out parameter, it can often be removed by using that value as the return value for the method. Replacing the out param, with the return value looks like this:

String CheckForDuplicates(String curNumber, bool IsParent) 
{
    ListViewItem curItem = this.tlvItems.FindItemWithText(curNumber);

    if (curItem != null)
    {
        String newNumber = this.getTOCReference(curNumber, !IsParent);
        curNumber = CheckForDuplicates(newNumber, IsParent);
    }
    // else - curNumber is correct - not a duplicate
    return curNumber;
}

As to design, I feel this code should be part of your getTOCReference so that clients don't have to worry about deduplication - it makes sense to make deduplication part of the basic function of getTOCReference. Its nothing complex - just a little renaming and some glue:

• rename getTOCReference to getNextTOCReference since that's really what it does - finds the TOC value that would come next, without checking for duplicates.
• create a new method GetNextAvailableTOCRefernece. To me GetTOCRefernece implies retreiving some kind of saved state and that multiple calls would return the same value. That's not what it does, so the new name makes it clear. It looks like this:

(essentially the code you gave at the top of the page, refactored to use return value, wrapped in a method.)

String GetNextAvailableTOCReference(String curNumber, bool IsParent) 
{
   String newNumber = GetNextTOCRefrence(curNumber, IsParent);
   return CheckForDuplicates(newNumber, IsParent);   
}

With these changes, you call getNextAvailalbeTOCreference when you need a new reference and you can be sure it is unique (no duplicates.)

I've left the code-style as is - I agree you could be a little more consistent, and provide comments for some of the logic - particularly getTOCReference since that does 4 different things depending upon method arguments. And of course, unit tests so that you can be sure it does what you say it does!