Code Review - Discussion about help vs. review - 2014-08-21

« first day (2 days earlier) ← previous day next day → last day (57 days later) »

nhgrif

00:04

As a sample of the library/protocol/api/etc questions:

1

Q: API for SQLConnect Library

I've designed a library for connecting Objective-C (and now Swift) apps to Microsoft SQL Server 2005+. (I believe there are other databases it works with, but I've only tested with MSSQL.) The project is available on Github here, and while users are fully capable of downloading the uncompiled p...

objective-c api library

ckuhn203

00:20

Interesting question. I wish I understood Obj-C @nhgrif. I have a feeling there's more implementation there than I think.

nhgrif

01:02

Hmm, how much implementation do you think there is?

I posted the execute: method.

ckuhn203

01:51

After a second look. None.

TazMan

02:11

Hey Guys

I would like my code to be reviewed

rolfl

02:31

@TazMan - you may want to head over to the 2nd monitor - chat.stackexchange.com/rooms/8595/the-2nd-monitor

also, you may want to be a bit more circumspect about demanding a review like that

13 hours later…

rolfl

15:07

codereview.stackexchange.com/questions/60722/…

Now deleted.

Now here! ;-)

Simon André Forsberg

First of all, I don't know why the question has -1

especially when it does not have a comment

rolfl

Possibly this:

Simon André Forsberg

> is there a better way to do that, without instantiate each time a new DecimalFormat object?

rolfl

in The 2nd Monitor, 9 mins ago, by Vogel612

@CaptainObvious and self-deleted after I VTC'd as unclear..

Simon André Forsberg

> 0,024849302 and i have to format it into -> 0,024 and then multiply * 1000 so the result is 24

So the goal of the code is clear.

There is working code

The code really does need a review. I can see several things to say

I'm saying on-topic

rolfl

15:19

Me too... I was preparing to put up an answer when itwas deleted.

But, this:

Simon André Forsberg

And there is a much better way to do it

rolfl

in The 2nd Monitor, 11 mins ago, by Vogel612

it was a better fit for SO anyways..

Simon André Forsberg

I don't agree with that

rolfl

in The 2nd Monitor, 7 mins ago, by Vogel612

@rolfl feature-request not-wanting-a-review

Simon André Forsberg

@Vogel612 Why would you say that?

> is there a better way to do that, without instantiate each time a new DecimalFormat object?

rolfl

15:21

As it happens, there is one thing that is unclear about it.

Simon André Forsberg

That's not a feature-request, it is a request for doing the same thing in a better way

rolfl

but not what @Vogel612 says ;-)

Simon André Forsberg

Perhaps the question doesn't specifically ask for a review, but it sure does need one. And making reviews is what we do around here.

rolfl

> DecimalFormat provides rounding modes defined in RoundingMode for formatting. By default, it uses RoundingMode.HALF_EVEN.

yet, he claims that:

> 0,024849302 and i have to format it into -> 0,024

Vogel612

@SimonAndréForsberg it's not ready to accept anything else but "how can I change this one thing"

Heslacher

15:22

@SimonAndréForsberg The code itself didn't do what the OP thought it does. Just printing 0.0

rolfl

and, as far as I can tell, the format #,##0 will format to 0,025

No, it won't..

Simon André Forsberg

I guess @Vogel612 VTC'd because it wanted a completely different implementation of the same thing, instead of specifically asking for a review. But I would not say that the question is off-topic because of that. The user is here, sure he wants a better way to do it, but a simple review of his code + a suggestion (without code) to simply parse it first and then multiply by 1000 and round down would accomplish that well

rolfl

@Heslacher is right.

code does not work as designed.

Vogel612

@SimonAndréForsberg FWIW I VTC'd as unclear...

Also IMO the way he asks is SO and not CR.

Simon André Forsberg

@Vogel612 That is true, and that is acceptable.

Vogel612

15:27

he limits the scope of reviews to a single aspect, and thus this question is off-topic as of question 6

Simon André Forsberg

@Vogel612 I think those questions "Is there a better way?" are both SO and CR. It might be on-topic on SO, but I would not say it is off-topic here.

Vogel612

@SimonAndréForsberg but then our Question 6 isn't placed well.

rolfl

OK, I am satisfied that this question should be closed, and am happy with it deleted too. The reason is different to what I expected, but there's not much point in using it as a test case for the review&help debate.

Vogel612

@rolfl agreed

Simon André Forsberg

@Vogel612 That's not the first time that happens, and yet we still manage to keep such questions open anyway. Even if they "break" question 6 (Do I want reviews about anything?) answers can still review the code.

@Vogel612 That might be true

Why on earth did he have a number, formatted it as a string, and then converted it back to a double?

Vogel612

15:29

@SimonAndréForsberg While It is acceptable to try to direct reviewers to a single aspect of the code, it's not acceptable to limit them to it

rolfl

@SimonAndréForsberg because double v = (long)(input * 1000.0) is too hard?

(assuming truncation as the OP says in the text, rather than rounding which he implemented in the format).

Heslacher

or
Distance *=1000;
System.out.println(Distance.intValue());

Vogel612

@rolfl or because some ****** decided to just giving him strings from somewhere.

and he didn't have a double to begin with.

Simon André Forsberg

@Vogel612 If he didn't have a double, why would he use String s = df.format(Distance); ?

8

A: Can we ask reviewers to *not* focus on something?

Yes. It is fine to ask reviewers to preferably avoid commenting on something. Of course the reviewers can comment about it anyway, and then it will be up to the community - and the original poster - to determine if the review is helpful or not. If you're not interested in comments about naming,...

Vogel612

@SimonAndréForsberg I'll go kill some baby unicorns and take a shower :(

rolfl

15:34

I find the unicorn blood is rejuvenating for the skin, and leaves a fresh lustre in my hair!

Vogel612

or not... why can't I delete my own message???

rolfl

2-minute grace period expired.

Vogel612

@rolfl hrmph.

@SimonAndréForsberg That's the other direction...

I'd rather quote Mat here:

> Now, if I say I have a performance concern and I don't want to hear about anything other than performance... then I'm not asking for a code review, I'm asking for a performance tune-up.

IMO that's more fitting of what that java question ~~is.~~ was.

Simon André Forsberg

@Vogel612 It is interesting though that there's not much difference in voting between my and Mat's answer

(Which is why I still haven't accepted an answer)

Vogel612

@SimonAndréForsberg Well the core of both answers suggest the same, namely:

> You can ask them to exclude some aspects, because of xy, but they can still review it.

Your answer doesn't even touch upon that aspect.

Simon André Forsberg

15:39

@Vogel612 Isn't that precisely what my answer is saying?

RubberDuck

@rolfl I try not to be star happy in this room, but I couldn't resist.

Vogel612

@SimonAndréForsberg careful.

> ignore (xy, ab) != focusOn(cd,ef)

especially when ignore is optional.

rolfl

@SimonAndréForsberg - as it happens, i had upvoted Mat's answer, and not upvoted yours.

Vogel612

well I upvoted the top 3 answers, as the in core state the same.

rolfl

I just read them through both again, and I don't think I will change my mind (here I go disagreeing with you, agian ;-) .... but the reason is similar to what I have with this help vs. review.

Asker intent (and priority).

Vogel612

15:42

the only answer that highlights the "limiting to" aspect is the Mug's though

rolfl

If the asker asks for helpt to fix something, and the review is secondary, then I think it is off topic.

If the asker asks for a performance tune up, and explicitly says that's all they want, then any review is secondary.

if the asker does not want a review as their top priority, then why should we think it is on topic?

Vogel612

there's that close reason on SO / MSO / MSE: blatantly off-topic (it's not about ....)

there should be something similar here IMO.

though blatantly would definitely be wrong.

Simon André Forsberg

@rolfl Fix something = Code doesn't produce expected results / Work as intended = Off topic

rolfl

Agreed.

Simon André Forsberg

@rolfl Because top priority or not, it's still on the agenda

rolfl

15:46

Take the Enumerable.intersect(setA,setB) thing, the priority is to make that decision, not to get a review. They need help deciding.

Bah, actually, I don't have time for this right now.

Simon André Forsberg

@rolfl "the Enumerable.intersect(setA,setB) thing" doesn't say much as I think we had quite some example questions there.

rolfl

Agreed. There's better examples.

RubberDuck

16:10

Grey area question. I don't think it's example code like the comment suggests, but it does seem to be a design question.

1

Q: Ruby: What is the best way to declare mutex and mutex.synchronize shortcut?

I am using mutex in my ruby code across many files. to deal with that I created a singleton class with one mutex object that I use all the time: class LockBlock include Singleton def initialize @lock = Mutex.new end def lock @lock end end I have two questions: Is this the...

Also, seemingly relevant Meta SE question.

6

Q: What does it mean for a question to be "answerable?"

The "don't ask" page of the Help Center contains one sentence that I've always felt captures the essential ingredients for a truly great question on the network: You should only ask practical, answerable questions based on actual problems that you face. I find myself quoting this line a l...

discussion asking-questions

Simon André Forsberg

@RubberDuck Not real code in my opinion. Lacking usage example. Unclear. I don't know Ruby though.

Vogel612

@SimonAndréForsberg usage example is clear.

Application wide synchronized lock

Simon André Forsberg

Perhaps I should have just said "I don't know Ruby though." :)

Vogel612

it's something like synchronized (Singleton.getInstance().getLock()) {

at least if I understand it correctly..

RubberDuck

16:30

I think it's lacking context (unclear) so probably not a good test case for design. Okay.

The meta though... I think that strikes to the core of all this. What is answerable and what is not.

Touches on the unsettling idea of "I don't know, but I know it when I see it."

Vogel612

16:59

> Although this is compressing a twelve-hundred-and-some-odd-word long post into a single sentence, I think the message is rather clear: The difference between a question and a discussion or poll is that a real question is seeking to discover some particular truth. The other type of "question" is merely seeking to learn what other people think.

5 hours later…

nhgrif

21:42

I don't like that we're putting so much weight on the way users are phrasing the plain-English parts of their questions. All other reasons aside, it puts non-native-English-speakers at a significant disadvantage.

2

Simon André Forsberg

22:02

@nhgrif I totally agree.

« first day (2 days earlier) ← previous day next day → last day (57 days later) »

Transcript for

Aug '1421

Code Review - Discussion about help v…

Imported from a comment discussion on meta.codereview.stackexc...

discussion scope

join this room
about this room

00:00

06:00

12:00

18:00

all times are UTC

site design / logo © 2025 Stack Exchange Inc; legal

mobile