Removing a link from a linked list

Question

I have written a method for removing a link from a linked list for a phonebook project I'm currently working on and it works perfectly. However, I want to know whether or not my code is acceptable from a programmer's perspective. I tried to systematically structure it in a way such that it's understandable, but by doing so, this results in multiple if statements being nested inside one another.

Is this considered bad coding? Also, should I have used a recursive implementation instead? What do you think? Are there any ways to improve this code? Any constructive criticism is certainly welcome.

public Link removeLink(String surname, String firstName)
{
    Link currentLink = firstLink;
    Link previousLink = firstLink;
    if(isEmpty())
    {
        return null;    // Name was not found
    }
    // customer to delete is first element in the list
    else if((currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName))
    {
        firstLink = firstLink.next;
    }
    // customer is not the first element in the list
    else
    {
        // search until either the end of list is reached or a match is found
        while(currentLink.next!=null && !((currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName)))
        {
            previousLink = currentLink;
            currentLink = currentLink.next;
        }
        // if end of list is reached
        if(currentLink.next == null)
        {
            // check if there is a match with last element in list
            if((currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName))
            {
                previousLink.next = currentLink.next;
            }
            else
            {
                return null;
            }
        }
        // match is found somewhere in the middle of list
        else
        {
            previousLink.next = currentLink.next;
        }
    }
    numEntries--; // number of entries decrements each time a customer is deleted
    return currentLink; // return the entry that was deleted
}

Answer 1

Code is fine I think, although according to Robert C. Martin, generally, if you need to add a comment then you have failed to write readable code.

For example, this could be made a little easier to read and understand by extracting the block

else //customer is not the first element in the list 

into a separate method. Also the long tests such as

if ((currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName))
while (currentLink.next!=null && !((currentLink.surname).equals(surname) && ((currentLink.firstName).equals(firstName)))
if (currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName))

...contain duplicated code, namely

(currentLink.surname).equals(surname) && (currentLink.firstName).equals(firstName)

which would be better extracted into, say testForMatch()

In Summary

• Long methods are frowned upon
• Repeated code is hard to understand and harder to maintain
• Extracting into well named methods beats good comments any day (comments can be left behind during updates and cause more confusion than is necessary with just code)