I've asked for a review on the way that I'm populating some comboboxes in a datagrid. One commenter asked what the purpose of the application was. Are those comments really relevant to a code review? In my particular case, I'm asking if my method is effective, or if there is a better way. The purpose of the overall application, I feel, has no bearing on the outcome of the question. I'm not trying to be a jerk, I just feel that comments like this aren't really relevant, and are non-conducive to what CR is supposed to be about.
2 Answers
We allow posters to bring a wide array of issues with respect to your code. They are not restricted to simply answering the problem as you see it. A good portion of the time, people come here with ugly code because they've taken the entirely wrong tack to solve their problem. As such, understanding what they are actually trying to do can actually be quite important. Often once that's understand we can suggest something way better.
In your case, the commentator is raising questions about the UI. That is straying a bit off-topic because we don't typically do UI reviews. But comments have more lee-way in being off-topic then answers. He's pointing out that your UI choices seem a bit odd. That's some feedback on your UI, feel free to take or leave it.
-
\$\begingroup\$ But in this case, I'm not asking for feedback on the UI design. I'm asking for feedback on the code that is populating my comboboxes. The UI design and purpose of the application have no bearing on either of those. \$\endgroup\$ Aug 8, 2013 at 16:03
-
5\$\begingroup\$ @MyCodeSucks, On Code Review, people are allowed to point out anything about the code they see a problem with, not just what you directly wanted help on. \$\endgroup\$ Aug 8, 2013 at 16:07
-
\$\begingroup\$ Yes, I saw that. But as you stated, he's raising questions about the UI itself, which is irrelevant to even your first paragraph, as it has nothing to do with code. And honestly, I don't think posters should be given the leeway to post about anything other than the question provided. If there's not truly enough info, then yes, ask for more. But we don't need UI design flaws pointed out to us when we're asking for help with code. In my particular instance, it is the way it is because that's what my boss wants. There's no need for me to have to explicitly state that. \$\endgroup\$ Aug 8, 2013 at 16:10
-
4\$\begingroup\$ @MyCodeSucks, our current policy is to give askers wide leeway in the questions they can ask. Of course, if the community agrees we can change the policy. However, I for one see no reason to object to people giving unsolicited advice on the UI or any other aspect of your code. \$\endgroup\$ Aug 8, 2013 at 16:19
-
\$\begingroup\$ The reason to object is precisly because it is unsolicited. If UI advice was wanted, that would have been the question. Askers have leeway in what they can ask, sure. Commenters and answerers should stick to the question at hand. If they have advice on another way, offer that AFTER they've addressed the question. "Here's the answer to your question. BTW, if you did X and Y, it would be much simpler." is much preferred to "Why not just do X and Y?" \$\endgroup\$ Aug 8, 2013 at 16:22
-
1\$\begingroup\$ @MyCodeSucks: If you want to explicitly limit the scope on advice, why not just state that in the post? By default, people will look at all facets of the code or whatever they see fit. So, if you just put up your code and some questions, expect to be asked/advised on anything in the post. As Winston said, posters on CR can address any issue, not just the ones specifically stated. A review is just that-- looking at what's given and providing feedback. \$\endgroup\$– JamalAug 8, 2013 at 16:32
-
-
\$\begingroup\$ @GarethRees, you learn something new every day! Thanks! \$\endgroup\$ Sep 25, 2013 at 15:26
Knowing the purpose of code is not just relevant, it is essential in order to properly review it.
Code is only a means to the end of solving someone's problem. If we don't know what that problem is, we can't say whether the code is going to be any good at solving it. All we can offer is a "generic" review which is much less useful to you and much less interesting for us.
The comments by users Nik and Malachi to your question are spot-on. Nik says in the first comment:
What is your problem exactly? Please add more context. Is there a reason you use strings? Is there a reason you use observable collections? Are those collections supposed to be unique per item? Stuff like that. It's hard to tell from your code what behaviour you are trying to achieve, so it's hard to give you proper advice.