Code Review - Discussion about help vs. review

Vogel612

15:00

@200_success depends on where you are..

ckuhn203

I understand Pimgd had to go, but does anyone have thoughts on what "working code" means? That seems to be the core of this debate.

Is an interface "working code"?

200_success

Pretend that you had to review the Electric example, with no GitHub and no unit tests. Could you do it? Could you do it well?

As I said, we can tailor our definition of "working" as needed.

The question is whether we want such questions.

ckuhn203

Ok, so the debate then is not whether these are currently on topic, but whether they should be on topic?

I obviously think that under our current rules, they are on topic, but I also don't much care for these questions.

200_success

Yes. Let's not be narrow-minded.

ckuhn203

I don't think they should be on topic.

It draws opinionated answers.

Simon André Forsberg

15:08

@ckuhn203 Doesn't all questions do that? For questions that looks too opinionated, there's a close reason for that

ckuhn203

For example:

opinions

1 hour ago, 5 minutes total – 19 messages, 2 users, 0 stars

Bookmarked 12 secs ago by ckuhn203

200_success

You could also say that all of our answers are opinionated.

2

ckuhn203

True @Simon. But If you go back to the bookmark, you'll see a widely contrasting opinions without much to go on.

That's what happens in the case of an interface only question.

Most of our opinions are based somewhere in fact, but without an implementation of some kind, or opinions are just opinions.

There's no fact to back them up.

but you know what, at the end of the day it's a cop out. We're saying it's too hard to review this.

Which, we've agreed before is not a reason for a question to be off topic.

Vogel612

@ckuhn203 Not necessarily...

In fact it could also mean, that it's not reviewable.

Pimgd

Buswifi~

Vogel612

15:15

(playing devil's advocate) An interface is just a better specification

Pimgd

I want to review interfaces because theyd be suitable targets in a larger review too

ckuhn203

Go on @Vogel612. I'm interested.

Vogel612

And per definition, is specification not code.

rolfl

0

A: Questions about interfaces, protocols, and APIs

Interface-only questions are off topic. This is why: Interfaces/Protocols define the interaction between two components. They describe a common mechanism for two otherwise independent 'systems' to communicate with each other. As a result: the primary purpose of an interface is not to 'do' thi...

Vogel612

and that makes it off-topic as missing code

Pimgd

15:17

Vogel that sounds like the "interface is design" argument that I made

ckuhn203

It actually sounds more like the "example code" argument. chat.stackexchange.com/transcript/message/17204160#17204160

Vogel612

@Pimgd no it isn't...

@rolfl put it quite nice in his answer:

> If the question supplies an interface only, then then it can only be considered to be example code, because the current implementing/interacting code does not matter, is meaningless, and has no value to the asker in the context of the question.

Pimgd

Yes good answer i like that

Vogel612

because It's more like a class with:

public void methodName(Object... params) {
     //implementation goes here
}

would like to star that quote myself because it's so awesome.

Pimgd

So when is it ontopic

Vogel612

15:23

As soon as there's implementation

Pimgd

Can that be example code?

rolfl

I just added a 'but' section to that answer.....

Vogel612

because everything else is example code...

200_success

The difference is that the author does not expect help with // missing code.

rolfl

A specific request to review just the style/names/etc. of an interface would be on topic, I think

Vogel612

15:24

@rolfl I'd prefer to have it black / white actually

--> interfaces only are off-topic

regardless of question intent

rolfl

I would too, but I don't think we can be that lucky.....

Pimgd

Rolfl "I have a class that does xyz, what do i call it?"

rolfl

although, it is true that you can critique the naming on the implementation just as easily as on the interface.....

let me change the 'but'.

Vogel612

@rolfl exactly. Especially when the implementation can be anything as long as it adheres to the contract

Pimgd

Switching buses

Vogel612

15:26

the name of the interface method is irrelevant to the implementation...

so if the implementation does something completely different, is the interface still well named?

Pimgd

Switch complete~

Vogel612

soo we're all fine with interface-only questions being off-topic?

Pimgd

Yeah they lack context

But when are they ontopic

Vogel612

@Pimgd never.

That simple ;)

Pimgd

Thats not okay

Vogel612

15:33

@rolfl elaborates in his answer, that they are example code for the purpose of the question

Pimgd

If my code contains interfaces its oftopic

Why

Vogel612

nooo,.

interface-only

if you use interfaces that's a whole different story.

Pimgd

Okay so how much do i have to add to make it on topic

Vogel612

an implementation of the interface

and the review then is not about the interface only actually..

Pimgd

Can that be example code?

Vogel612

15:35

but whether the implementation adheres to the interface

@Pimgd well then it's example code.

Pimgd

Ah i see

Vogel612

(as are interface-only questions for that matter)

Pimgd

Hmmm

rolfl

@Pimgd - in my mind, the 'negative space' of an interface is the most important part.

Pimgd

Negative space?

rolfl

15:37

the primary purpose of the interface is to describe what's not there.

and it's the stuff that's not there, that is reviewable.

2

Pimgd

Okay new question

ckuhn203

@rolfl I think that might have just convinced me.

Also, that this is an example code issue, not a working code issue.

200_success

We have reviewed DB schemas without queries, right?

Pimgd

Is the interface reviewable if you have the usage the interface and the implementation

rolfl

@Pimgd - yes, but, the interface review "follows" from the implementation

as in,:

"This implementation could be better if the interface was structured differently".

Pimgd

15:40

Yeah like that

Hmmm

200_success

But even without the implementation, you could say "Your interface makes it impossible to do X."

rolfl

but that would be a design review

(a good one, perhaps, but off-topic, and too broad).

200_success

Sorry, going on standby for a while. (breakfast)

rolfl

enjoy.

I was formulating an answer to the DB schema thing.

Pimgd

I suppose its because you cant own usage and interface but not implementation

Thatd be silly

rolfl

15:43

For a DB Schema, presented as DDL, you can review the style, indentation, names, etc., but a design-specific request would be off-topic.

Pimgd

Constructs like that end up with someone having to comply to your arbitrary rules

rolfl

I have this SQL Script to create a database, is it a good script? <---- on topic

I have this SQL Script that creates a database. Is the design of the database good? <---- off topic

ckuhn203

Fine line, but yes. That ^

Pimgd

Err the first onw sounds really basd

rolfl

It would be like saying I have this program that plays tic-tac-toe, did I do it well.

Pimgd

15:45

Is this X good sounds so vague

rolfl

the second option is I have this game that plays tic-tac-toe, but, should it be playing checkers instead?

Pimgd

Yes tictactoe is boring

=D

rolfl

You never played UTTT .... did you.

21

A: Weekend-Challenge Reboot

The Ultimate Tic-Tac-Toetm Tic-Tac-Toe is boring. Let's code The Ultimate Tic-Tac-Toe, a whole different story. Uh, what? Each turn, you mark one of the small squares. When you get three in a row on a small board, you’ve won that board. To win the game, you need to win three smal...

Pimgd

Dunno whatsthssat

Fking wobbly buds

Vogel612

/me laughing loud now.

Pimgd

15:49

How so

Nobody

Hm the discussion seems to drift off?

Vogel612

because of the orthographic errors.

@Nobody yes kinda..

Pimgd

Cause we have a sokutuion

Vogel612

I think everyone's accepted the solution provided by rolfl.

Pimgd

*solution

Pfff

Nobody

15:50

Just one quick radical idea: We take "review" more literally and just say what can be improved and not how ^^

Vogel612

@Nobody glad I know you're just positively trolling on that one ;)

Nobody

what do you understand under positively trolling?

Vogel612

definitely defintive trolling to lighten the mood ;)

ckuhn203

Well, it sounds like we've reached consensus. Lets get some upvotes on this answer.

3

A: Questions about interfaces, protocols, and APIs

Interface-only questions are off topic. This is why: Interfaces/Protocols define the interaction between two components. They describe a common mechanism for two otherwise independent 'systems' to communicate with each other. As a result: the primary purpose of an interface is not to 'do' thi...

Vogel612

@ckuhn203 no sockpuppets here :(

I got a feeling that nhgrif may add some points against it though..

ckuhn203

15:58

Sockpuppet @Vogel612?

Vogel612

Fake account I control.

used to garner additional rep

--> I can only upvote once ;)

with sockpuppets I could change that. I'd probably get a ban for it though.

ckuhn203

Ahhhhh. Ok, well, there's lots of people in the room and only 3 upvotes on it.

200_success

Opinions may have changed since the Meta post, and you can't change votes easily. Should post a fresh question.

ckuhn203

And I am absolutely sure @nhgrif will have something to say about all of this.

rolfl

^^^ that

200_success

16:00

Perhaps with four poll responses to vote on:

1 Allow interface questions. Define "working" accordingly.

2 Ban interface questions. Define "working" accordingly.

3 Dunno. Let's run an experiment.

4 Dunno. Need time to think.

Simon André Forsberg

@rolfl in what way is an interface without implementation not reviewable?

ckuhn203

Can we get a volunteer? I need to go get some work done.

Vogel612

@200_success we ruled out "working code" for reasoning..

Simon André Forsberg

@ckuhn203 I don't agree about that one. Yet.

ckuhn203

Fair enough @SimonAndréForsberg.

Vogel612

16:03

@SimonAndréForsberg we were arguing:

interface ~= class with method bodies stripped --> example code

rolfl

Anything reviewable on the interface can be reviewed on the implementation.

Simon André Forsberg

@200_success That should not be underestimated. It is hard to be updated with all the meta-activity we have around here.

rolfl

anything interface-only, is design-related.

the design is reviewable, but, it's off-topic as a question

Nobody

What is so bad about status quo? There is no official statement on this and the community decides on a per case basis by not answering/voting

Simon André Forsberg

@Vogel612 I don't normally upload example code to Github. What makes this interface example code?

@rolfl Why is it off-topic if it is reviewable?

Nobody

16:07

we have a working meta were people can argue their case if they feel they were closed without good reason

Vogel612

@SimonAndréForsberg in itself nothing.

Simon André Forsberg

The interface is code. If the interface is reviewable, why should we not review it? Aren't we Code Review

Vogel612

but as soon as you want it reviewed, it could be anything

200_success

I'm sensing that reconvening after some time to think might be a good idea.

Vogel612

while interface typeof code, !code typeof reviewable per se

Simon André Forsberg

16:08

@Vogel612 what do you mean "it could be anything"?

Nobody

I am off, have a nice day and see you later

rolfl

@Simon, from my answer, there are only three things that can be wrong with an interface: won't compile, bad style, bad design.

he primary purpose of the interface is to express the design.

if the design review is requested, the user may as well supply a flowchart.

the interface is a proxy for that.

if the style review is requested, it may as well be based on an implementation

Vogel612

and if you'd take naming into that, the names are meaningless without the implementation behind them,

Simon André Forsberg

@rolfl that is the best argument I've heard so far for considering interface questions off-topic.

Vogel612

because (using your example), As long as I return a List<T> i could write anything:

public  List<T> getFields() {
     return new ArrayList<T>(myService.getPlayers());
}

could be a perfectly "valid" implementation

--> the interface itself isn't off-topic, the missing implementation is.

200_success

16:14

Interfaces don't express flow charts, though.

Vogel612

16:26

@200_success make it an UML then. The problem stays the same...

2

Simon André Forsberg

16:42

Here's my opinion about this:

0

A: Questions about interfaces, protocols, and APIs

Are questions about interfaces/protocols and APIs on-topic? Yes. But that does not mean that the question itself is on-topic. There are still other factors to consider, such as: Is there enough context in the question? Is it clear how these interfaces will be used? I see no reason whatsoever to...

rolfl

@Simon - can you think of a way to ask a question containing just an interface, and still be on topic?

Vogel612

@SimonAndréForsberg the big question is then where do we draw the line?

rolfl

or, alternatively, is there an example already?

Vogel612

or is it again something like (quoting rolfl):

> I don't know where the line is, but I know when it was crossed.

Simon André Forsberg

@rolfl Does this question count? codereview.stackexchange.com/questions/46334/…

@Vogel612 I think it all boils down to: Do I think this is reviewable, given the description, code and the questions asked?

rolfl

16:50

7

Q: Interface for a tree node in Qt

I have the following C++ interface (in Qt) for a tree node, inspired somewhat by Valve's implementation of a culling octree: /** * @brief Interface for a recursive tree node. */ class ITreeNode { public: /** * @brief Virtual destructor. */ virtual ~ITreeNode() {} /** ...

Vogel612

@SimonAndréForsberg but is it reviewable when you could hide absolutely anything in the implementation??

38 mins ago, by Vogel612

public  List<T> getFields() {
     return new ArrayList<T>(myService.getPlayers());
}

rolfl

That question is asking: how do I write a good system for manipulating tree data structures, and is this interface close?

> In order to improve my understanding of how interfaces should be designed I'm looking for a critique, if possible, of the functionality of this interface. My general target was to include a useful set of basic tree-related functions - obviously the bare minimum would be simply adding/removing/getting children,

> but I also felt as though automating potentially useful processes such as removing all children (pruneSubtree() - thinking recursive deletion) and removing a node from a given tree structure to leave a forest might help make future tasks easier.

Simon André Forsberg

@Vogel612 You should probably assume that the implementation reflects the intentions of the interface.

rolfl

It is asking: Is my design good?

it is a crappy, off-topic question, that has been horribly up-voted, and answered as if it was on programmers.

(and, apparently, I upvoted it at some point, sorry).

3

It does not even know what use cases it is trying to solve.

Simon André Forsberg

@rolfl very tempted to pin that

I'm not room owner. Can't pin even if I tried...

Vogel612

16:53

@SimonAndréForsberg but for an interface-only review, you can't assume that.

rolfl

mod-super-power.

Vogel612

and that's IMO the core of the problem

Simon André Forsberg

@rolfl How would closing the question as off-topic help anyone?

rolfl

codereview.stackexchange.com/review/first-posts/18999

^^^ I first-post reviewed it.....

Simon André Forsberg

@Vogel612 Why not?

Vogel612

16:54

you don't know what hides behind the imaginary { /* insert implementation here */}

which is the point of an interface

rolfl

@SimonAndréForsberg At this point, it may not.... but, if I paid more attention the first time.....

Vogel612

you can hide anything behind it.

rolfl

... it should have been closed immediately, and migrated to programmers, and asked as a "How do people use tree data structures, and what methods do people run on them?"

where it would have been closed as too broad.

Simon André Forsberg

@Vogel612 the point of an interface is to make a contract. Sometimes the point is to not care about how it is implemented and sometimes to push you in a direction for how to implement it.

@rolfl And again, how would that help?

rolfl

......

It would not have helped the asker.

it would have helped Code Review.

Vogel612

16:57

@SimonAndréForsberg well then you are reviewing the contract, and that's probably better off on {insert code-design site here}

@rolfl because we wouldn't have it come back and bite us now?

rolfl

^^^ that too ;-)

Code Review is not a place for designing your application.

it is for revieing the implementation

That question is asking: how do I design this interface?

Simon André Forsberg

@Vogel612 @Nobody knows what is on-topic at programmers.

rolfl

and the answer is not a code review, it's a design discussion, and it's too broad for any SE site.

it does not fit the Q&A format of SE

Simon André Forsberg

@rolfl Generally, neither does Code Review

rolfl

far too many 'what if' type considerations.

Simon André Forsberg

16:59

You can spend your whole life thinking "what if..."

Vogel612

@SimonAndréForsberg nobody said it should go to programmers.

Simon André Forsberg

@Vogel612 Are there other code-design sites?

Vogel612

no, the question is, is there any code-design site?

rolfl

not code-design, but 'application design'.

Simon André Forsberg

@rolfl Ah, so now we're Implementation Review?

Vogel612

17:01

because last time I checked, programmers wasn't

@SimonAndréForsberg maybe a little?

Simon André Forsberg

@Vogel612 last time I checked, programmers was. But then again... I don't know much about that site

Vogel612

@SimonAndréForsberg maybe we read the help center differently, but there's that cool infographic they got..

I think it would fall into the section "just you"

rolfl

Simon: I have to implement a system that solves all project euler problems. Is this interface good?

public interface EulerSolver {
    public void solve(int number);
}

Review please? & thanks!

Simon André Forsberg

And I think that for that "just you" section is where Code Review comes in, and what makes us unique. Many questions that are for "just you" fits here.

Vogel612

@SimonAndréForsberg exactly, but does that mean we should allow Questions about "just you and your design"

IMO we shouldn't

@rolfl did I compliment you on your "low rep gimmetehcodez-user" acting skills yet?

Simon André Forsberg

17:05

@rolfl No it is not because most Euler problems return a number. However, not having looked at all Euler problem descriptions I believe that some of them have different requirements for what to return, such as "all numbers that fulfills xxx". I do not consider that question off-topic

Vogel612

@SimonAndréForsberg well but then again it could be:

> Yes this interface is awesome, as long as you put in the choice, solving and outputting of a challenge behind that method

Simon André Forsberg

I would however edit out the "& thanks!" part, if Jamal hadn't already done so

Vogel612

which is reviewing the same interface...

but coming to a completely different end

Simon André Forsberg

I would probably have given the question a -1 though. But that's not the same as an off-topic vote.

rolfl

@Simon - my suggestion, is that an answer to that qeustion would need to make many assumptions, and the question itself is too broad.

Simon André Forsberg

17:08

@rolfl Then close it as too broad, not for "non-working code".

rolfl

The review by necessity, would jave to be about the design, and that's not code.

Vogel612

You'd then have to implement a ConsoleEulerSolver and a GuiEulerSolver and maybe an AbstractEulerSolver..

rolfl

OK, so then, I will change my statement, and say: All interface-only questions should be closed as too-broad

Simon André Forsberg

@rolfl Is this not code?

5 mins ago, by rolfl

public interface EulerSolver {
    public void solve(int number);
}

rolfl

@Simon - yes, it is, but, if you read my answer, I even say it is reviewable....

... unfortunately, the point of the question is not to review the code style, but the design it represents

Simon André Forsberg

17:09

@rolfl I am not willing to say that "All" should be closed as too-broad, but I can imagine that many of them can be closed as such

I think however we are making a monkey of dust here (or whatever the expression is, if such an expression even exists in English), we aren't getting that many interface-only questions at all.

rolfl

If the point of the question is to review the design, and design-only reviews are off topic, then the question should be off-topic.

if the point is to review the style, then there should be no problem with including the implementation as well.

Simon André Forsberg

@rolfl in addition to what I said before, I would remove that public in public void as it's implicit

rolfl

You are proving my point.... I think.

Simon André Forsberg

Either way, I have no plans in asking an interface-only question in the near future

rolfl

The 'value' requested by an interface-only asker, is feedback on the protocol/API/design.

As a consequence, the interface itself is merely pseudo-code.

it is a 'tool to describe the design'.

Vogel612

17:14

@rolfl actually the interface isn't pseudocode...

it can be actually workign code, taken directly from the production code.

rolfl

it can be, but, it is a description of the design.

Vogel612

but that doesn't matter, as the lack of implementation behind it makes it pseudocode.

rolfl

If you take java.lang.Iterator, and ask for a code review, by necessity you are reviewing the design/strategy/concept, and not the actual implementation.

While there is no doubt that it can be reviewed, we know that design rviews are too broad for Code Review, and SE in general.

if we let interfaces get reviewed, we may as well let white-board photos get reviewed.

I have some code that implements the above....^^^^

and then comment that blue-ink and red-ink are commonly confused by colour-blind people.

(or is that red, and green?).

meh, this is a diversion, and off-topic.

;-)

The point is, that we can, as intelligent, insightful people, offer a very good sounding board for all sorts of concepts, systems, and protocols..... we can do it.

But, should we do it?

the answer is NO, because it implies that we are people who can help review designs, and that design reviews and discussions are on-topic, in general

If you can express your design as an interface, then feel free to ask it here.

^^^ we do not want that happening.

Let me tell you about a system where you can build nodes for use in tree data structures. It will be able to do all these things: a, b, c, and d. Is that a good design?

Actually, that's a good word.... will

My code will be able to do xxx.

it is not currently able.....

Vogel612

aaaand we got our next reason ;)

Simon André Forsberg

@rolfl Do we really know that? citation-needed

Vogel612

17:24

so we're closing interface only questions as: non-working code, too broad, example/stub code and not implemented yet!

@SimonAndréForsberg what else does an interface express?

Simon André Forsberg

@rolfl No you don't. You never participated in that community-challenge ;)

rolfl

@SimonAndréForsberg Citation: codereview.stackexchange.com/help/dont-ask

Vogel612

It just says: Anything that implements me, will be able to [..]

Simon André Forsberg

@Vogel612 "we know that design rviews are too broad for Code Review, and SE in general."

rolfl

> you are asking an open-ended, hypothetical question: “What if ______ happened?”

> every answer is equally valid: “What’s your favorite ______?”

> Your questions should be reasonably scoped. If you can imagine an entire book that answers your question, you’re asking too much.

> Chatty, open-ended questions diminish the usefulness of our site and push other questions off the front page

From help/on-topic

Simon André Forsberg

17:26

> What types of questions should I avoid asking?

First, make sure that your question is on-topic for this site.

^^ That's a funny way to start that page

rolfl

> Best practices in general (that is, it's okay to ask "Does this code follow common best practices?", but not "What is the best practice regarding X?"

> How to add a feature to or solve a problem with your code

> Higher-level architecture and design of software systems

Vogel612

it's just coming more and more ;)

rolfl

On-topic rule:

> To the best of my knowledge, does the code work?

just because it compiles, does not mean it works.

you cannot 'run' an interface

Simon André Forsberg

And we haven't exactly defined what working code is. So please don't use that argument

work != runnable

@rolfl Just to make the whole thing a bit worse, what about Java 8 default methods in interfaces? Let's say you have an interface with excessive use of default methods, would that be on-topic?

Vogel612

@SimonAndréForsberg well is it actually "runnable"?

just for the sake of the argument?

Simon André Forsberg

17:32

@Vogel612 default methods are runnable, yes.

rolfl

@SimonAndréForsberg The design/concept represented by the interface is not on-topic, but, the implementation of it is.... and the default methods are implementations.

Vogel612

Well where's the problem then.

What makes default method different from a "default" implementation?

Simon André Forsberg

@rolfl Perhaps you should post an answer to this meta question?

18

Q: Reviewing "design"

It has been said in various comments that on Code Review we review code and not design. I find this statement not very clear though. Obviously, if someone has an UML and not any written code, that's off-topic on Code Review. However, if there is a question with a finished implementation, such a...

discussion scope

@Vogel612 good point

Vogel612

Either way ima go home now..

its's way beyond TTQW..

rolfl

17:47

@SimonAndréForsberg I will.

rolfl

18:14

0

A: Reviewing "design"

Code Reviews in general (not specifically at Code Review Stack Exchange) will often review the design. The people doing the review also have influence on the design too, so it is natural. Off-Topic The design itself though, is a very broad, and unsubstantial concept. On Code Review (and Stack E...

Simon André Forsberg

19:22

@rolfl What do you mean by "stable design"?

Is it design that is likely to change, or is it design that is not very functional?

> additionally, if the design is not stable, then the code is off-topic because one of the on-topic questions is: To the best of my knowledge, does the code work?

This is probably one of the silliest things I have ever heard on CR Meta:

> design reviews are only on-topic when the question does not ask for one

opinions

Transcript for

Code Review - Discussion about help v…