4
\$\begingroup\$

Recently, I posted the following question on Code Review: Convert ES6 yield recursive method to a loop

At the beginning I post a question with actual code, asking if there is a way to write the same thing in a non-recursive way.

I made the error to simplify the code, as in my mind it became easier to understand and rewrite when it's not recursive. And so my question was flagged as pseudocode. I assume this is my fault, and have reverted my changes for now. I don't understand why this rule applies, and why you need all inconsistent parts of my code, but rules are rules so let's say ok. I should not have done this.

After this, my question received negatives votes, and I don't know why. Because when people ask me to clarify something, I did.

And the last edit changed the main purpose of my question. It says "anything that improves performance will be helpful". But that wasn't what I wanted originally. And the post title was changes to "Generate a set of combinations".

I may have explained something wrong; and maybe understood something wrong in the rules. I posted this question here and not on StackOverflow for example, because my code is running well.

So I let the question edited for now. But I want to discuss these changes. Why were these changes made? Was I difficult to understand? Should I have posted this question on StackOverflow or another community site, and so what have I not understood?

What should I do to avoid reproducing the same errors?

I assure that I do my best. I try to be as clear as possible and I try to do things following the help.

\$\endgroup\$
9
  • \$\begingroup\$ I down voted as you're asking a specific question - how to remove the recurssion from your code. Something I think should be off-topic, as you don't seem to want a code review. \$\endgroup\$
    – Peilonrayz Mod
    Jan 17, 2018 at 17:25
  • 1
    \$\begingroup\$ Not sure why revision 7 rolled back to rev.4, considering revision 6 "put my actual code" seems absolutely legit and exactly what you were supposed to do (example code being off-topic). @Peilonrayz OP is describing their working solution, alluding to an inefficiency problem, presumably relating to the recursiveness. The post explicitly states that any other idea to improve performance is welcome - I hardly see anything off-topic there. The rollback thoroughly baffles me. If OP puts back their actual code, I've no problem with reopening \$\endgroup\$ Jan 17, 2018 at 17:27
  • \$\begingroup\$ @Mat'sMug I fail to see "The post explicitly states that any other idea to improve performance is welcome". \$\endgroup\$
    – Peilonrayz Mod
    Jan 17, 2018 at 17:30
  • \$\begingroup\$ @Peilonrayz "I suspect that converting the code into a non-recursive algorithm, using loops, will speed it up, but anything that improves performance will be helpful." - improving code performance is [a rather large] part of what reviewers do on this site. And of course anything else reviewers can point out ought to be welcome feedback too. \$\endgroup\$ Jan 17, 2018 at 17:31
  • \$\begingroup\$ @Mat'sMug That was added by Snowbody, not NatNgs. And NatNgs explicitly said in this very question that thay didn't mean thit. "It became asking for "anything that improves performance will be helpful". But that wasn't what I want originally" \$\endgroup\$
    – Peilonrayz Mod
    Jan 17, 2018 at 17:32
  • 1
    \$\begingroup\$ @Peilonrayz (read fail) so this is where we answer this meta to guide this user into understanding what we do on this site, and why there's an important difference between "I think recursiveness is hurting performance" and "can you help me convert recursive to iterative". \$\endgroup\$ Jan 17, 2018 at 17:36
  • 1
    \$\begingroup\$ Ok so revision 7 was rolling back the "simplification/examplification" of the code, correct? If so then all is good. \$\endgroup\$ Jan 17, 2018 at 17:54
  • \$\begingroup\$ @Peilonrayz Ok, so reading the help I didn't get this point ! Thank you for this, I was misunderstanding how codereview was working. \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:13
  • \$\begingroup\$ @mats-mug Rolled back to revision 4 because there was already my actual code in fact (with some explanation of how it works). The revision 6 was a sort of "partial" rollback (only the code, not the explanations). - We can rewrite my question as like you proposed: "Can you help me convert recursive to iterative ? And if you find anything that can be improved, please explain also" \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:13

2 Answers 2

4
\$\begingroup\$

I downvoted as you limited answers to only ways to increase performance by not using recursion. I posted a comment to this effect, however you didn't answer my question, or change your question to remove my concern.

Do you want a review of any and all of your code, or do you want us to only say if there's a way to change from a generator to a loop?

Since code reviews should be able to point out any and all problems with your code, and you dis-allowed some points, I downvoted.


I'd also encourage you to think about what you really want.

  • Do you want to stop using generators and recursion? Or,
  • Do you want more performance?

From the sounds of it you want the latter, but you are asking the former. A classic XY problem.

And so all you need to do is ask what I think you really want. Which would make your question definitely on-topic, and would remove my downvote.

\$\endgroup\$
6
  • \$\begingroup\$ Ok, so I was not clear; sorry for that ! Lets say that what I want was mainly to break recursion, and a second point will be to improve overall performances of this algorithm. After, if you say that is not useful to break recursion the question become "How to improve performances". \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:04
  • \$\begingroup\$ @NatNgs I'm sorry, I don't understand your comment. But, why do you want to remove the recursion? \$\endgroup\$
    – Peilonrayz Mod
    Jan 18, 2018 at 9:08
  • \$\begingroup\$ Yeah, the main purpose of removing recursion is here to improve calculation speed and also to make the function support bigger inputs without consuming 4Go of ram. I'm possibly wrong when I suppose that this one of the better way to improve this algorithm; but for now I have no information about my judgment error :P ! \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:22
  • \$\begingroup\$ @NatNgs If you can "improve calculation speed and also to make the function support bigger inputs without consuming 4Go of ram" without removing recursion would you want that as an answer? Do you want an answer that may remove recursion, but not those problems? \$\endgroup\$
    – Peilonrayz Mod
    Jan 18, 2018 at 9:34
  • \$\begingroup\$ But the problem I think is recursion in fact ! But sure that if you find something else interesting, I will look at it ! Its the reason why I let the question "different from original question". I may have too focused on this problem (that for now nobody explain me why this should not be a problem :P). But I found crazy that reviewers want to answer the question another way; in that case, question owner will just repost his question on another site because he did not have received help. \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:42
  • \$\begingroup\$ Note, CR isn't a site to "get help". It's a site to "get peer reviewed" \$\endgroup\$ Jan 18, 2018 at 11:44
3
\$\begingroup\$

And the last edit change the main purpose of my question. It became asking for "anything that improves performance will be helpful". But that wasn't what I want originally; and the post title was changes to "Generate a set of combinations".

That last edit made your question on-topic.

Code Review is:

I've written this piece of code, it does XYZ, I have the concerns 1-2-3 with it presumably because ABC, here it is in its full glory, can you help me improve 1-2-3 or change ABC? Any other feedback is welcome.

Compare to:

I've written this piece of code, it does XYZ, I have concerns 1-2-3 with it presumably because ABC, here's an example snippet that shows what I mean, can you help me change ABC to DEF?

Per the help center, in order for a question to be on-topic, you need it to include your real, actual, working code, with as much context as possible; you must also be interested in feedback on any & all aspects of the code, because that's what reviewers do.

And that's why it has so much value: helping you change a recursive solution to an iterative one might help performance - it's actually very possible that a reviewer suggests an iterative approach... but it's also possible that a reviewer points out an inefficiency that you hadn't noticed, and that the best solution has nothing to do with what you think the answer is.

Let the reviewers do their job, don't tie their hands & feed with your assumptions: doing that is indeed asking a very specific question (albeit one that's too broad to be a good fit on Stack Overflow).

The edits indeed changed what the question was - the original question was off-topic, and this awesome community wants to help you improve your code, and helped you put the post back on track.

As for the title, know that CR post titles need to describe the purpose of the code, as opposed to what you'd want to achieve. We'll have a home page full of identical/similar titles if we don't do that.

\$\endgroup\$
2
  • \$\begingroup\$ I'm ok for that point ! Helping me founding anything that could improve my program that I can have not notice can be really helpfull ! But it wasn't really my main objective; that was to use a non recursive way for this code. In fact, I think if the question become explicitelly "Please find anything that can optimise my code", I will recieve multiple (and usefull !) answers; but because these will not answer to my main question, so I will never accept any proposed answer. \$\endgroup\$
    – NatNgs
    Jan 18, 2018 at 9:08
  • 1
    \$\begingroup\$ That's what we do on this site... \$\endgroup\$ Jan 18, 2018 at 11:42

You must log in to answer this question.

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