4
\$\begingroup\$

I have been asked to evaluate the close reasons for this question here:

Has been closed "off-topic - example code".

Is the question actually on-topic?

If it is off-topic, is the 'example-code' the right close reason?

| |
\$\endgroup\$
10
\$\begingroup\$

The question is off-topic, and it is example-code.

There are other reasons to close it too, but let me go through my reasoning first:

The code consists of:

static void Main(String[] args) {
    //get list of files from source directory
    getListOfSourceFiles();

    //now filter those files acording to some criterion
    filterSourceFiles();

    //remove all files from the destination directory
    removeAllFilesFromDestination();

    //move filtered files from source directory to destination
    moveSourceFilesToDestination();

    //extract moved files
    extractFiles();

    //remove archives
    removeArchives();

    //do something with extracted files
    doSomething();
}

The last method doSomething(); speaks to being example code. The description in the question says: "One the archives have been copied, extract them. Then delete the .zip files and keep only the extracted files." ... so, once they are removed, what else is there to do? What does the doSomething() do?

That, in itself, is probably not enough to be a close-reason, but what about the following:

    //get list of files from source directory
    getListOfSourceFiles();

What source directory? How is the list of source files returned? Is it returned, or do we assume that those details are not relevant?

Then:

    //now filter those files acording to some criterion
    filterSourceFiles();

What criteria? What is the input, where's the output?

Then:

    //remove all files from the destination directory
    removeAllFilesFromDestination();

What destination? Hard-coded?

    //move filtered files from source directory to destination
    moveSourceFilesToDestination();

Now, what filtered files? Either the filtered list is a static variable somewhere, or the files have been 'deleted' from the source, or 'something'. We can only guess how you know which files are filtered, and which are not. Also, there's that Destination again... where does that get set?

Finally

    //extract moved files
    extractFiles();

    //remove archives
    removeArchives();

These have the same flaws as previous methods... extract from what files? What archives?

Your program reads more like elaborate pseuodocode. Your 'program' has no inputs, no outputs, no variables, no implemented functions (except Main), nothing.

In my assessment, the code could be closed for numerous reasons:

  1. too broad
  2. too opinionated
  3. unclear what you're asking
  4. off-topic - example code
  5. off-topic - broken code
  6. off-topic - must include the code to be reviewed
| |
\$\endgroup\$
  • \$\begingroup\$ 1. Really? 2. Huh? 3. The purpose of the question is explicitly written. You're right about 4, 5 and 6. I avoided writing code that would have distracted/didn't want reviewed. \$\endgroup\$ – async Oct 12 '14 at 22:39
  • 7
    \$\begingroup\$ @user16547 - What's left is no longer reviewable, and that's the problem: you've removed so much that what's left is... 'hypothetical' People would have to guess too much to make sense of things. \$\endgroup\$ – rolfl Oct 12 '14 at 22:41
6
\$\begingroup\$

I won't provide the actual contents of the methods. They are fairly straightforward. I also removed method parameters and local variables for brevity. Assume relevant and descriptive method names and parameters.

As I often write in comments:

Unfortunately, it's a bit hard to review code like this because the code you are showing is example code rather than code that you're actually using in a real project, which means that a lot of context has been stripped away. Unlike Stack Overflow, we prefer to look at real code instead of example code. Please see the meta question: Why is hypothetical code off-topic for Code Review?

Big problem in this case is that the reason for removing the context (method parameters and local variables) is for brevity. That is not a reason to strip out the context. In this case it leaves a lot of guesswork on the user. I might expect getListOfSourceFiles to store the results in a static variable and take no parameter as input, someone else might expect the same method to return a IEnumerable<File> and take string path as a parameter. In other words, it is unclear.


I have a small program that does a fairly trivial task. I am unhappy about the fact that I have everything in Main or called from Main. As a result, all my methods are static. I'd like to improve on my high level structure - assuming my Main does too much.

So... What is the problem? Has any alternatives been considered? Again, unclear in my opinion. Although not unclear enough for closing by itself, when together with the other problem stated above, it is very unclear. How can we help improving high level structure when all we have is... umm... a couple of method names inside a Main function? (And some completely useless comments to go along with them)


Conclusion

This question is definitely off-topic, and "example code" is absolutely a good close reason for it.

| |
\$\endgroup\$

You must log in to answer this question.

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