8
\$\begingroup\$

We generally don't allow design-only questions.

A question has been asked that provided review-able code, and depending on your POV an additional question is asked about 'maintainable modular code' in general. Now, that is definitely more design-related than code-related, and in my answer I have opted to focus only on the coding.

There is now a design-only answer for the 'maintainable modular code' part, but nothing on the code itself. This is different from providing a non-code answer that still focuses on the code aspects (e.g. when one isn't familiar with the target language, and prefers to outline in words/pseudo-code).

How should we handle this? Leave a comment for the answerer to see if they want to edit their answer to also talk about the code parts? Flag it? Or is the question itself off-topic in the first place, and we should get OP to clarify the intention first?

\$\endgroup\$
  • \$\begingroup\$ FWIW, the (accepted) answer-in-question has been updated to make it address a tiny bit of the code parts of the question. \$\endgroup\$ – h.j.k. Oct 17 '15 at 6:29
10
\$\begingroup\$

I think we have a XY problem here.

The answer stated exactly what the OP wanted to know and is therefore a good answer. However, while OP did post re-viewable code, an essential part of the question wasn't a good fit for CR (emphasis mine):

It occurred to me I should probably be asking here how maintainable code is actually written. I'm kind of ashamed I haven't done this sort of thing before, but as a self-taught programmer I presume this sort of obstacle is common.

Given the above sample of my idea of how to make maintainable modular code, what sort of pitfalls should I look out for while making the excursion into a legitimate program of larger than two or three files?

That's a general question not specific to the code at hand. While it doesn't make the question off-topic, it opens the door for exactly the kind of answer he got and wanted. A general answer instead of a review.

\$\endgroup\$
9
\$\begingroup\$

Don't over-think the formulation of the question.

I believe Donald Mc.Lean's answer says it well:

I would say that if the question contains code, and the asker wants the design of that code reviewed, there's absolutely nothing wrong with that, but if the asker wants something that is not code reviewed, it's off topic.

"Design-Only Questions" generally means UML-diagrams or pure English description of the design and no code. As long as there is real (non-examplish) reviewable code, there's nothing wrong with reviewing the design. I mean, how often haven't someone said something like "What you have done here makes no sense, scratch that and start over and do something like this instead...", those kind of answers are, to me, design answers. And there's nothing wrong with that.

\$\endgroup\$
4
\$\begingroup\$

IMO, the answer is not a good one because it doesn't actually reference the OP's code at all.

If, for the points it made, it highlighted part of the OP's code that did/did not follow them with a suggestion of how it could be done better, it would be an awesome answer. But, without that, I think it's not the kind of answer we want CR to attract.

\$\endgroup\$
3
\$\begingroup\$

It's important to tackle the highest-priority problems first. There's no point in worrying about the lack of whitespace around operators if the program doesn't work: the bugs need to be fixed first.

In particular, if the whole design of the program is wrong — for example, if it uses an algorithm with poor performance — then it's the design that should be discussed in the review. It's a waste of time to review code that is going to have to be completely rewritten.

A couple of examples:

  • This question from today uses an \$ Ω(n!) \$ algorithm. Until that's fixed, it's not worth worrying about the use of global variables.
  • This question tries to do code transformation at the token level without parsing. This is never going to work and so it's not worth commenting on the code.
\$\endgroup\$
2
\$\begingroup\$

IMO this answer is valid. The question did state

Given the above sample of my idea of how to make maintainable modular code, what sort of pitfalls should I look out for while making the excursion into a legitimate program of larger than two or three files?

Although the answerer didn't refer to the code itself, he/she clearly described how maintainable code should look like.

I don't really see that much of a difference wether the answerer tells

You have a variable run with the same name as a method run(). I would recommend renaming the variable to something else

or

Well-named variables, functions and classes

Often we see answers like

To help the maintainer of the code you really should name your variables, methods and classes in a meaningful name....

which sometimes is having an example of the code like

So because s doesn't tell anything what this variable is about it would be better to change it to x.

IMO this is a nice to have, but it shouldn't be mandatory.

\$\endgroup\$

You must log in to answer this question.

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