A program that uses three methods to reverse and print an array

Question

I have this code that uses three methods to do certain tasks. One creates an array, another reverses it and the other one prints it out(it was required to use three methods). A person recommended me to upload my code here as there are a few things that need to be improved.

They said that I do not need to return arrays if I modify them in-place, and GenerateNum could allocate the array itself instead of requiring an array with the correct size to be passed. However as a beginner I didn't quite catch what they were trying to tell me to change in the code.

using System;

class Program
{
    static void Main(string[] args)
    {
        string getnums = Console.ReadLine();
        int getnum = Convert.ToInt32(getnums);
        int[] array1 = new int[getnum];

        GenerateNum(getnum,array1);
        RevArray(array1);
        PrintArray(array1);
    }
    static int[] GenerateNum(int getnums, int[] arrayy)
    {

        for (int index = 0; index < getnums; index++)
        {
            arrayy[index] = index;
        }
        return arrayy;
    }
    static int[] RevArray(int[] arrayy)
    {
        for (int index = 0; index < (arrayy.Length)/2; index++)
        {
            int a = arrayy[index];
            arrayy[index] = arrayy[arrayy.Length - index-1];
            arrayy[arrayy.Length-index-1] = a;
        }
        return arrayy;
    }
    static void PrintArray(int[] arrayy)
    {
        Console.WriteLine(string.Join(",",arrayy));
    }
}

Answer 1

Let's review your code line by line

string getnums = Console.ReadLine();

• The name getnums is not really a good name for a string variable. When you name a variable please try to use noun(s) (w/o adjective). Methods should start with verbs.

int getnum = Convert.ToInt32(getnums);

• This is quite error-prone. What if the user types three? Your application will crash with a FormatException.
• Please prefer int.TryParse and remember that all input is evil (until proven otherwise).

int[] array1 = new int[getnum];

• Yet again array1 is not really a good name. Someone who reads your code will not know what was your intent with this variable. Try to write code that is understandable for others as well.
• This is quite error-prone again, because getnum can contain negative number. The array allocation attempt might result in a OverflowException.
• Consider to use ArrayPool if you try to allocate an quite big array.

static int[] GenerateNum(int getnums, int[] arrayy)

• The arrayy is really a bad name here, you are using Hungarian notation which should be avoided.
• You don't need to pass getnums because you can get the Length of the array.
• With the current implementation the PopulateArray might be a better name.
• Depending on your design the responsibility to allocate memory can be moved over here as well.

for (int index = 0; index < getnums; index++)

• Feel free to use array.Length instead of getnums

arrayy[index] = index;

• If you design your application in the way that this method should allocate the memory as well then you could rewrite this whole method with the following command:
  return Enumerable.Range(0, getnums).ToArray();

int[] RevArray(int[] arrayy)

• As it was stated by others as well abbreviation does not add anything here. On the contrary it might make confusion because it could mean other things as well not just reserve, like reveal, revive, review, revisit, etc...

for (int index = 0; index < (arrayy.Length)/2; index++)

• The Array class defines a Reverse method, which can do the whole in-place replacement on your behalf without reinventing the wheel.
• Here you are using the Length property which is good. I suspect that this code is based on this SO answer. I strongly encourage you to add there a comment where you mention the source of your code.

Answer 2

The person who said that you do not need to return the array if you modify it in place is correct. An implementation of RevArray() could be.

static void RevArray(int[] arrayy)
{
    for (int index = 0; index < (arrayy.Length)/2; index++)
    {
        int a = arrayy[index];
        arrayy[index] = arrayy[arrayy.Length - index-1];
        arrayy[arrayy.Length-index-1] = a;
    }
}

And the calling code would work just the same - currently, the return value of RevArray() is not used.

However, in different situation, where we wanted to use the reversed array, returning it could be useful to return the array.

Say, we change main() to

static void Main(string[] args)
{
    string getnums = Console.ReadLine();
    int getnum = Convert.ToInt32(getnums);
    int[] array1 = new int[getnum];

    GenerateNum(getnum,array1);
    PrintArray(RevArray(array1));
}

Now we are using the return value.

It depends on what is are trying to practice. If we are simply looking at 'modifying an array contents', then not returning the array is simplest. General rule: If we are not going to use the return value, don't return it.

GenerateNum()
This can be rewritten without passing in the array.

The code currently goes like this

Main
  Read in a size (n)
  Create an array of size n
  Populate the array with the numbers from 0 to n-1
  Reverse the array
  Display the array

GenerateNum() can be rewritten as 'Create and Populate the array', rather than 'Populate the array'.

So we could restructure it as

Main
  Read in a size (n)
  Create and Populate an array of size n with the numbers from 0 to n-1
  Reverse the array
  Display the array

and we'd use

static int[] CreateAndPopulateArray(int n)
{
    int[] ret = new int[n];
    for(int index = 0; index < n; index++)
    {
        ret[index] = index;
    }
    return ret;
}

to create and populate the array.

Our main() now becomes

static void Main(string[] args)
{
    string getnums = Console.ReadLine();
    int getnum = Convert.ToInt32(getnums);
    
    int[] array1 = CreateAndPopulateArray(getnum);
    RevArray(array1);
    PrintArray(array1);
}

Other points
Naming
This is only a practice app and one is unlikely to forget what it is intended to do but naming variables and functions/methods well is a good habit to cultivate.

In the original code arraySize would be a better choice than getnum, and PopulateArray() would be a better choice than GenerateNum() - the method doesn't really generate anything, the array already exists. This may seem like nit-picking (and it is, which is why it comes in Other Points), but good naming, and making code easy to read/comprehend by others, is important and is a good mindset adopt.

Answer 3

Some quick remarks:

• getnums is a bad variable name. Ditto getnum.

• What if the user provides an invalid input in getnums? Why don't you check for that?

• Do not pointlessly abbreviate: "Num" and "Rev" do not make your code run faster.

• Why do you spell it arrayy?

• What is the point of the first argument of GenerateNum? You know the size of the array, so just use that. (Also, the namechange from getnum to getnums is really confusing and only reinforces the need for properly named variables.)