9
\$\begingroup\$

My code works in most scenarios, but in this specific scenario, it does not at all do what I intend for it to do. I was hoping to have it reviewed and see if anyone could spot a fix.

Is this sort of question on-topic for Code Review?

Now, having answered your gut reaction on whether or not that sort of question should be on-topic, let's take a closer look at this question and see if it falls in that category:

Thread-safe (generic) value wrapper

The question itself states:

Please keep in mind that my goal is to provide full and fully safe access to the value, including any operation you might want to do with it, as conveniently as possible.

And that emphasis is of the asker's, not mine.

Then immediately, he states:

if the generic type is a reference type (a class, for instance), then get() is not thread safe anymore if methods called on the class change its contents or properties.

So, his code is thread safe... unless he's wrapping a reference type... in which case it's not thread safe.

The question was originally closed as broken code. Then, without the code changing at all, the plain-English part of the question changed. The word "problem" was changed to "issue" and the word "solve" was changed to "improve upon". And then re-opened.

\$\endgroup\$

2 Answers 2

Reset to default
11
\$\begingroup\$

It is my opinion that this question has been viewed with a 'deficit mindset', looking for all the reasons to close it... when, instead, there are a lot of good reasons to keep it open.

The code is complete, works for the current use case, has usability issues which are managed by programmer discipline and procedure, nit is generally is well thought out, and neat.

Sure, it is not perfect code, and it does have problems.

The real issue appears to be the request that the code should be 'fixed' to handle situations which are currently handled manually by process and discipline.

Unfortunately the user has now deleted the question, probably as a result of the extended scrutiny it got.

But, the solution for questions like this is, as per usual:

  1. upvote those brilliant question we get.
  2. edit and/or comment on questions which, with some small effort, can be made brilliant.
  3. downvote (and comment?) questions which are not well presented, etc.
  4. vote to close those questions which are unsalvagable, or risk getting answers that would be invalidated by the fixes needed to make the question decent.

In this case, I believe answerers would have been fine to review the code in the question, "as is". Sure, the question asked for more than just a review, but a simple edit resolved that problem.

Note: in general, we cannot expect every question to be perfect. We should also expect many questions to be broken, and unsalvagable, and we want those to be closed and downvoted as soon as possible. But, when there is a question that is somewhere in between, we should endeavour to assist in whatever way we can in fixing those parts of the question that are concerning, by editing, commenting, or chatting.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ I think what you describe as a 'deficit mindset' is one (maybe the biggest) reason why StackExchange works so well. It is all about picking and chosing. We defined some questions are off topic. And we defined: everything not off-topic is on-topic. (Since that is the simplest way to define a group: We are not ...) which means we should strive to clearly outline what we are not instead of always looking at what we could be and deciding on that basis \$\endgroup\$
    – Vogel612
    Mar 1, 2015 at 7:35
  • 4
    \$\begingroup\$ @Vogel612 - I am not sure I agree with your assessment. SE is continually striving to eradicate the deficit mindset. There is reputation and badges to be earned by editing and fixing bad questions in to good ones. Although there is a strong deficit mindset on SOverflow, the SExchange staff and system are trying hard to prevent that. And for good reason. On code review it is particularly important to avoid that, because every question has problems, otherwise they mst likely would not have been asked. On CR the deficit mindset would close everything. \$\endgroup\$
    – rolfl
    Mar 1, 2015 at 12:08
  • \$\begingroup\$ @Vogel612 establishing everything in terms of a set of syllogisms may work for decidedly objective subject matter and questions that demand decidedly objective answers, but I don't believe that approach is a good fit for determining the suitability of Code Review questions. Due to the somewhat subjective nature of CR, I see that approach possibly leading to one or more of the following negative outcomes: 1) it takes an act of Congress and extended debate to approve a type of question not accounted for in the Constitution of Suitability, 2) quality code in answer is second to quality of answer \$\endgroup\$
    – moarboilerplate
    Mar 26, 2015 at 20:56
  • \$\begingroup\$ As for 2, I would say answers need to be of a certain quality, but the scenario I'm talking about is when dogged adherence to the protocols results in good code being discarded because of style points. \$\endgroup\$
    – moarboilerplate
    Mar 26, 2015 at 20:57
9
\$\begingroup\$

My opinion on the matter is.... it's complicated.

There is a precedent on CR that code is not considered to be broken just because it has some known limitation.

Here is my code. It does [X]. There is this edge case [Y], but I'll fix that in the next sprint. How is my current implementation, disregarding edge case [Y].

That is an acceptable question to ask, because we are not being asked to change the behavior of the code under review. We are being asked to give leniency on particular issue [Y], as it will be addressed in the future.

That's not what the OP of this particular question is asking though. OP is asking us to change the behavior of his code.

Please keep in mind that my goal is to provide full and fully safe access to the value, including any operation you might want to do with it, as conveniently as possible.

I can see so far one issue that I want to improve upon: if the generic type is a reference type (a class, for instance), then get() is not thread safe anymore...

Because setting the string property is no longer inside the dispatch block, it is no longer thread safe...

Any ideas on how to improve upon this issue...?

This is broken code and I have voted to close the question.

\$\endgroup\$

You must log in to answer this question.

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