32

Inspired by this question.

How should I let a user know that the code they've written goes against Microsoft's recommended practice? In this case, instantiating an SPSite instance with a hardcoded URL, not disposing of SPSite when finished and getting an SPList instance with the hardcoded list name when it is available in the method parameters.

Normally I'd put it in a comment but that's difficult when it's several lines long. When it's not related to the answer, should I even point this out?

9
  • 4
    You could try to put it in the comments in short with a link to the source. And hope that they explain why in the guidelines themself.
    – Hayt
    Commented Sep 20, 2016 at 8:32
  • 4
    Well, it's stackoverflow, they're coming for a problem they need solved not for a best practices preach. If it was codereview I'd encourage you to point it out but it's not. Maybe add a comment "your code could use some review" and link them to it. Commented Sep 20, 2016 at 8:32
  • 3
    It helps if you can find a canonical q&a that has all the detail and just link to that.
    – DavidG
    Commented Sep 20, 2016 at 12:10
  • 13
    I do this on a regular basis to warn about SQL-Injection. You wouldn't believe how many people are still building queries by concatenating constant strings and unescaped user input. So I'd say yes - warn them with a comment even if it's a long one. Commented Sep 20, 2016 at 12:19
  • 5
    @GillBates - I'd argue that people coming here (for the most part) do want to know what they could do better - although I will concede that some flatly do not care. Commented Sep 21, 2016 at 14:23
  • 1
    I think this is a good question, and I often end up mentioning some of these kinds of things in my answers, but do be aware that sometimes in the process of turning something into an MVCE, best practices that might require more code are dropped in favor of making the question minimal. Commented Sep 21, 2016 at 15:00
  • Interesting point. Personally if I was abridging code, 9 times out of 10 I'd do so with comments, rather than post bad code to the public internet where it could be copied by some unsuspecting dev. In the SQL injection example I'd probably replace my escaping and parameterising code with a comment like // Building query here rather than replace it with quick-and-dirty string concanenation. It won't stop the laziest coders writing dangerous code but it'll force people to go looking for the answer, at least giving them a chance of finding the right one.
    – MorayM
    Commented Sep 21, 2016 at 15:13
  • 1
    @Mourndark: But then your code won't compile - and for an MCVE, it needs to. I would certainly encourage people to remove more than the most minimal error checking from an MCVE. Commented Sep 22, 2016 at 11:19
  • If you answer, and to not add a warning for poor coding practices even if unrelated to the problem, you could even get downvotes for not noticing them. Commented Sep 23, 2016 at 7:32

1 Answer 1

52

Yes, a comment would be appropriate. To give an example, there must be at least a million comments in the and tags along the lines of

Please note that mysql_ functions are unsafe and deprecated, rewrite your code to use mysqli_ or PDO instead.

Try to find a link where Microsoft's recommended practices are explained so they can read further, but make sure your comment is useful without the link.

1

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .