What causes a question to get closed as example code?

Question

https://codereview.stackexchange.com/questions/78110/multiple-if-else-how-to-reduce-cyclomatic-complexity-of-code

Here's an example of something that was tagged as example code. I'm wondering what makes it example code as sometimes we have to change strings/variable names that exist in our production code.

I understand that several properties and variables are not defined. What if the code were submitted like this instead?

Vector<string> tableType1List ... /* holds table types */
Vector<string> tableType2List ... /* holds table types */

public string getTableType (string typeOfTable, string fileName)
{
    if (typeOfTable.equals("STRING1")) {
        for (String type1Table : tableType1List) {
            if (fileName.contains(type1Table)) {
                tableType= "STRING1";
                return tableType;
            }
        }
    } else if (typeOfTable.equals("STRING2")) {
        for (String type2Table : tableType2List) {
            if (fileName.contains(type2Table)) {
                tableType= "STRING2";
                return tableType;
            }
        }
    } else if (typeOfTable.equals("STRING3")) {
        if (fileName.contains("String3")) {
            tableType= "STRING3";
            return tableType;
        }
    } else if (typeOfTable.equals("STRING4")) {
        if (fileName.contains("String4")) {
            tableType= "STRING4";
            return tableType;
        }
    }
}

Is the difference between what was posted and my example enough to make it not be flagged as "example code"? Or would I also run into this issue? Personally I think my example should not be deemed example code as it can run by itself. I specifically did not add values to the vectors to show the irrelevance of the actual values in the method, but if that would be your sole reason for deeming it example code, please mention that but in the review context just assume that I'm doing that.

Answer 1

Vector<string> tableType1List ... /* holds table types */
Vector<string> tableType2List ... /* holds table types */

That's a pretty good start. That's broken because it's example code.

---

if (typeOfTable.equals("STRING1")) {

The "STRING1" part of this smells like a foobar string to me. Which, I could swallow if it was pointed out that it was sanitized a bit for proprietary reasons, but without some such disclaimer, this screams

I wrote this stripped down version just to have it reviewed.

As for why stripped down example code is off topic and why that's a problem, I'll refer you to the canonical answer.

Answer 2

Part of reviewing code is commenting on things like variable names. IF this is your code, all I can review is "come back when you have meaningful variable names so I can start making sense of what your code does."

Maybe that's your actual code, but it's hard to review the actual meat of the code with such pointlessly generic variable names.

It's kind of a big deal.

Answer 3

If the following code would have been included in the question:

Vector<string> tableType1List

I would first of all have said that Vector is deprecated in favor of ArrayList.

Secondly, that wouldn't have compiled as it's supposed to be Vector<String> (I understand, just an example here on meta, but if it would have been an actual code in the question it would have been very important! Not copying the exact code causes confusion).

And now, the most important things:

Whenever changes has to be made to variable names and stuff because it is company code, that should be mentioned. Mentioning that it is company code and the exact code and context cannot be revealed makes us a bit more understanding. However, this alone might not have changed things in this case.

Additionally, try to come up with an alternative context that makes sense.

The linked question additionally suffers from another problem:

I have the code from a method using Java 6, and the checkstyle complains about the Cyclomatic Complexity being too high (13 instead of the max 10), how can i reduce this?

There is absolutely no description about what your code does or what the purpose of doing that is. I often write the following comment for those cases:

To make life easier for reviewers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question

Answer 4

As a rule of thumb, the question needs to contain enough information so that reviewers can understand why the code exists.

Let's start with the title: Multiple if else: how to reduce Cyclomatic Complexity of code. It doesn't meet our guidelines, which call for the title to state the purpose of the code. If every question had such generic titles, then the site would be cluttered with questions unhelpfully entitled Help me clean up my mess of if-else statements.

Another concern is that we can't tell what is to be taken literally and what has been fabricated for the purpose of posting the question. For example, are the strings sequentially numbered, or are they purely arbitrary?

What about the tableType1List and tableType2List? How large are they? Is there any predictability to their contents which could help in simplifying the code? Why are there two cases that work one way, and two cases that work the other way?

As for your version of the question, it won't compile in Java, due to lowercase string and failure to return a value in all cases, and is therefore off-topic as broken code. It is only barely more informative than the original code, and I would classify it as hypothetical code as well.

Answer 5

Right, let's assume your code is real code, here's an example answer:

Your code has common aspects between the comparisons and the string constants. Because the last character of the constant is always a digit, you can extract and parse that, and do the logic based on a simple loop instead:

public string getTableType (String typeOfTable, String fileName)
{   
    Vector<Vector<string>> listOfLists = new ...
    listOfLists.add(tableType1List);
    listOfLists.add(tableType2List);
    listOfLists.add(new Vector<>("String3"));
    listOfLists.add(new Vector<>("String4"));
    if (typeOfTable.matches("String[123]"))
    {
        int id = Integer.parseInt(typeOfTable.substring(7, 1));
        for (String subtype : listOfLists[id - 1])
        {
            if (fileName.index(subtype) >= 0)
            {
                return "STRING" + id;
            }
        } 
    }

    ....
}

Note how, because the code is example code, I can abuse the system, and come up with a valid, but useless suggestion?

Note also that, in your assumptions you made in the question you broke the code. You have a method which has no return value... what's supposed to be returned when nothing matches?

Example code leads to situations where the asker has made assumptions that are not obvious to the reviewers.