# How to phrase questions about spotting design issues in programs

I like to give my students questions asking them to find and correct design issues in code. As an example, I gave them the following question:

/**
This class represents the user-system of the application.
There must be only one object of this class.
*/
public class UserSystem {
private Map<String,String> users;// maps user to password

public UserSystem() {
users = new HashMap<String, String>();
}

}

return false;
}
}


The question text was something like:

"The class compiles correctly but has a design problem. What is the design problem? Write a main program that demonstrates it. Use one of the design patterns we used in class to rewrite the class such that the problem is fixed. What design pattern did you use?"

The answer I aimed for was that, since the constructor is public, users of this class might create multiple objects of the class. The solution should be to convert this class into a singleton by making its constructor private (and adding a static member variable INSTANCE).

Indeed, some students got this correctly, but other students answered with all sorts of other design bugs, such as:

• The method "addUser" might be used by unauthorized users to enter their name into the system.
• The methods are not synchronized.

etc...

It is true that these issues can be problematic in some cases, however, this depends on the context and the usage of the class. I wanted them to focus on a specific issue which I deliberately wrote in the Javadoc to this class. However, many students probably did not notice this.

MY QUESTION IS: in the future, how can I phrase such questions such that the answers will be more focused?

• "There must be only one object of this class" can be interpreted as a warning to the class users to use one UserSystem in an instance of an Application. If a program only needs one UserSystem, the program could allocate and use a static one. Converting it to a singleton restricts its reuse. Feb 28 '18 at 11:19
• The singleton pattern you want as a solution to this problem is now consisted an antipattern. The preferred mechanism is to use a bean factory and scope the class as singleton. The correct fix would be to annotate the class with (at)Singleton or (at)Global (or whatever annotation your preferred bean factory prefers) Mar 2 '18 at 2:01
• I nearly go what you wanted. I read “There must be only one object of this class” and Identified that as a problem, I was then looking for where the code was not re-entrant. As @pojo-guy says singleton is considered harmful. What if I want two or more objects of this class, E.g. I am implementing authentication domains. It is not this classes responsibility. It is the responsibility of the factory. Mar 2 '18 at 14:31
• Singleton considered harmful aside. Don't make them guess. Tell them read the comment. Ask how do I implement the comment in code? So we can remove comment. Mar 2 '18 at 14:33
• @pojo-guy I agree only use when it makes the code simpler, consider first factory-pattern (so never). The only use is as a temporary kludge, on a class that does not work if there is more than one instance. (So not because we want one instance, but because we can not safely create more than one instance. Because of badly implemented class.) Mar 2 '18 at 23:39

My worry here is that there is no general solution other than to improve any question when you reuse it based on the answers you saw in the past. I have to admit that, even as a very experienced Java developer who is also very Pattern literate, I missed it entirely since I didn't actually read the Javadoc before I looked at the code. I came up with only minor quibbles.

The specific problem here is that I was looking at the Class in isolation, not as part of a larger whole. Also, I was reading the class, not writing it. I'm pretty sure that if I were writing it I'd have built a Singleton in some form instead.

But, you can't expect that every student will notice everything, and you can expect that some students (me, perhaps) will get lost in the detail before analyzing the bigger picture.

On the other hand, missing the point of your exercise can lead to a forehead slapping A-Ha moment for some students that is valuable in its own way, as long as they don't feel that they are punished in some way for missing it. It isn't a general solution, but had you here, or in general, emphasized to the students to RTFD (Read The Full Documentation) then more of the students would be likely to get it right. But that is a general prescription for how you teach about programming, not a specific recommendation for this sort of question. "Be sure you understand the intent of a class before you worry about its details" is a good lesson.

For this one, though, perhaps making the Javadoc more complete (longer) would have helped a bit. A longer explanation before the start of the actual code would be less likely to be missed.

Making it obvious would also help (A Singleton is required here) but likely defeat your purpose. It would, however, be essential in production code to be just that obvious. Even saying the key thing twice would help.

/**
This class represents the _unique_ user-system of the application.
There must be only one object of this class.
*/


I'd still have missed it, of course, since I didn't RTFD first. But if you can make such things lead to student A-Ha moments they are valuable. Forehead slapping may sting a bit, but is likely good for you.

But on an exam or quiz, especially a timed quiz, there is too much pressure to get to the answer quickly.

On the other hand, there are different sorts of quizzes. If you have a quickie quiz on most days, where each quiz counts very little toward grading, don't worry too much about making every question perfect. This is true even when the little quizzes collectively have a substantial contribution to grades. On major exams, of course you need to be much more careful. If you find that a substantial proportion of your group gets it wrong, or the folks you expect to get it right don't, then you might need to exclude that question from the computed totals, with an explanation and discussion soon after the quiz.

• The Java doc is almost always out of date irl, so novices might read it, but the more experienced the programmer, the less likely they are too read it or to give it credence. Mar 2 '18 at 2:03
• This is very interesting. Indeed my question is open-ended. It could be good as a low-weight homework exercise, but in exams, I should rephrase it more clearly. Mar 17 '18 at 19:04

It is true that these issues can be problematic in some cases, however, this depends on the context and the usage of the class.

Asking "what is wrong with the design of this class" is too broad. You need to provide more context: perhaps provide the UserSystem class and then another class that uses it incorrectly (by creating a new UserSystem instance for every user, for example), and then ask what the design problem is in that code and how it could be prevented in the future.

If you want a specific answer, you need to ask a specific question. All of the answers your students provided sound perfectly reasonable to me.

I would break the question into parts.

You can put a short paragraph describing what the code in front of them is supposed to do. When one (or more) writes code for something, the design is inseparable from the purpose of the code. Following the paragraph, put the code.

Also, I suggest changing the documentation of the code by adding a short description before each method. Phrasing the documentation can help with focusing the students on the correct solution (without explicitly giving the answer...).

Now, regarding breaking the question into parts:

Before asking "What's the bug?", I suggest putting a question\sub-question similar to:

Write an external (i.e. static) method that receives a person's details and checks whether they are in the system. The method can receive parameters of your choice.

That last sentence is a red light. Such a question hints about static, and about passing parameters. When using a singleton, you don't need to pass any parameters, except for the name of the person (not even the password).

After that, put in the question you wrote, but make it clear that the design flaw is in the code you gave them, and not in the one they wrote.

Phrasing the question in this manner and order makes it so that when they answer, they are already thinking about static and parameters and instances of the UserSystem class (if only subconsciously). All of this focuses them on Singleton.

### Simple demonstration:

Presenting this and asking, "What do you see?" is sure to get a variety of answers. Most might be along the line of a bird feeding baby birds. Not all will be, however. Possible answers include "A bunch of birds," "A birds' nest," "Tiny leaves," "A pretty picture," and even "A fuzzy background." None are wrong, and all will show what the answerer first focused on as an answer to the question. None of the answers reflect what else they might have noticed, given more time, or what else they could say, if asked in detail about it. The fact that you wanted to know the species of the bird is not in the question, so not necessarily in the answers.

Students have a particular mindset for taking tests, or other evaluations, that contributes to the problem. Students are driven to maximize their time, and the potential for high marks. They will do anything they can to find the answer in the shortest possible time-span. Once they have a possible solution, they will test it just enough to see if it is probably the answer, do whatever is required to demonstrate that answer, and move to the next question.

You can attempt to precondition the students to be thinking in the right mindset to "see" what you want them to see by ordering questions in a fashion that leads the student to the proper mindset. There is no certainty that such a tactic will succeed, however. Many students, especially in the higher levels of education have a test taking skill where the test is scanned first, then questions are answered. Shorter and/or easier questions are answered first, longer questions latter, and the hardest ones last. In that style of test taking the preconditioning questions can be somewhat removed in time from the final target and may have lost their "conditioning" value.

Unless a code segment has a compile-time error, or other syntactical error, a blanket statement of "find the bug" will lead to a range of answers, most of which are technically correct while not being what you had in mind. If it is a syntax issue, or an error which will cause a compile-time error, the question should still be explicit in that a syntax error, or compiler error is expected and you want the cause, or the correction to avoid it. Without that qualifier it is still often possible to find a "problem" with the code that is other than what you were expecting.

Since I don't know Java well enough to be sure, I'm only making a stab at a possible re-work of the question. The principles should carry over, even if the technical details do not, however.

In the question cover the facts that, first, the comments state, There must be only one object of this class, and second, some future coder using the class might not read the comments. State that you want them to make the needed changes "at the design level" which would force the future coder to only have one object of that class. "The best method will cause a compile-time error if the rule is violated." An alternative statement could be to rewrite the code such that "it is impossible for two objects of the class to be instantiated by the program."

That's the part I'm not solid on. I don't know that a compile-time error would be created in a program trying to create two objects of that class if you turn it into a singleton class using the changes you described.

If you have taught, and used, the singleton pattern, then they should have a decent change of coming to the conclusion that such is what you want. Still, other solutions are possible, such as a wrapper class that tests for how many there are already, or some kind of internal test for prior instances. The more specific you can be to eliminate alternative "fixes" the better.

My opinion is that in a testing environment it is unwise to ask for evaluation of designs in search of improvements. As I covered earlier, this will not offer any reliable insight into the abilities or thinking patterns of the tested students, but rather it might offer insights into their test-taking patterns. Since I doubt the latter is your objective, the tools used might be better replaced. It might be better to tell them to use a pattern to change (or even create) something, and the skeleton of that can even be bare bones. To determine that they understand the singleton pattern, as you taught it, you could instruct them to convert the following skeleton into a singleton class:

public class UserSystem {
private type var; // instance variables

public UserSystem() {
// instantiation code
}

public void methodOne(argList) {
// methodOne code
}
}


There are no "bugs" or design issues to correct. What the class is, or does, has no bearing on how it can be made into a singleton class, and is therefore, unimportant to the question, and only distracting to the student.

There is one other thing you can do before the tests, preferably at the beginning of the course, maybe even include in the sylubus. You can state that you will use certain, defined and provided, phrases to emphasize what elements of the material you are examining. This may, or may not, be possible in your situation, but at the very least you can develop and publish, a rubric, for the tests before they given to the students. This allows them to see what you're testing for, and evaluating against. They can then focus their attention on the details that matter. If you're not testing their ability to find syntax error in provided code, you can say so, and even tell them to ignore any they find and presume that it is correct syntax for the purposes of the question. After that instruction, any answer that is about the syntax is obviously wrong, even if they did find such an error and use it as the answer. You can even add such statements, where warranted, to the body of a given question. Though, if on the question itself, I'd make it a bold and separate line in the text of the question.