storing a number and incrementing it ever time the method is called

Question

I'm trying to get my twist method to do the following:

store a number (result)
generate a random number and add it to result
The code I have so far is:

public int twist(int min, int max)
{
    int result = 0;
    Random random = new Random();

    int y = random.Next(min, max);

    result += y;

    System.Diagnostics.Debug.WriteLine(result);
    return result;
}

All it does is generate a new number each time because I'm not storing it, I don't think I need a loop, but I need to store result after each increment of y, and y should only be incremented each time the twist method is called.

Please review following answer to use Random properly: http://stackoverflow.com/questions/767999/random-number-generator-not-working-the-way-i-had-planned-c — Alexei Levenkov, Apr 16 '12 at 23:23

score 3 · Answer 1 · edited May 23 '17 at 11:43

3

Store the "result" outside of the method. I also tend to make the random generator static as well as it has a tendency to remember the number it generates on each call.

It's best to keep Random static because as stated by Marc Gravell "Every time you do new Random() it is initialized using the clock. This means that in a tight loop you get the same value lots of times. You should keep a single Random instance and keep using Next on the same instance."

Look at - Random number generator only generating one random number for more information and the source of the quote.

private int _result = 0;
private static Random _rand = new Random();

public int twist(int min, int max)
{
    int y = _rand.Next(min, max);

    _result += y;

    System.Diagnostics.Debug.WriteLine(_result);
    return _result;
}

edited May 23 '17 at 11:43

Community

1
1

answered Apr 16 '12 at 23:05

GJHix

522
10
18

+0. Would be complete answer if you have explanation for making Random static. Check out http://stackoverflow.com/questions/767999/random-number-generator-not-working-the-way-i-had-planned-c. – Alexei Levenkov Apr 16 '12 at 23:27
@AlexeiLevenkov Good point, thanks for reminding me though I never really looked into the reason why it did that. – GJHix Apr 16 '12 at 23:37

leviathanbadger · Answer 2 · 2013-03-06T23:14:50.493

2

Try:

private int result;
private Random random = new Random();

public int twist(int min, int max)
{
    int y = random.Next(min, max);
    result += y;

    System.Diagnostics.Debug.WriteLine(result);
    return result;
}

Because result needs to be accessed by multiple calls to the same method, you need to make it into a field instead of a variable. In your version, each individual call to twist created a new variable and set it to 0. If you extract it into a field, then it is initialized to 0 when the object is created and each call to twist increments it without resetting it each time.

edited Mar 06 '13 at 23:14

answered Apr 16 '12 at 22:46

leviathanbadger

1,682
15
23

result should be labeled _result since it's a class variable. – tsells Apr 16 '12 at 22:48
1

@tsells: Um, not really, no. Where did you get that naming convention? – Ry- Apr 16 '12 at 22:48
result needs to be assigned to either in constructor or inline. also if its a multithreaded application you should look into Interlocked.Increment – Manatherin Apr 16 '12 at 22:55
@Manatherin: No, it doesn't. Because it is a value type, it is initialized to 0 by default. If it were a reference type, it would be initialized to `null` by default. – leviathanbadger Apr 16 '12 at 22:56
2

@minitech - it's a common way to distinguish class level variables from local variables. It makes for cleaner code and is easier to read. It's not required - but suggested. – tsells Apr 16 '12 at 22:57
@tsells: Well, that depends who suggests it :) I personally use `_`-prefixed fields only as the backing fields for properties, whereas I use `this` for any other member. And other people prefix things with `m`, and other people use lowercase names... just a matter of preference. – Ry- Apr 16 '12 at 22:59
2

@minitech - did you come from the VB world? I hate seeing this.variable in code unless you are doing it in conjunction with a base class to distinguish between to values. – tsells Apr 16 '12 at 23:00
@tsells: VB.NET and JavaScript, yeah. In both languages, `this` is pretty much mandatory :) But I've honestly never seen `_` for every class field. – Ry- Apr 16 '12 at 23:02
Edit: Moved Random outside of method to properly create random numbers. – Alexei Levenkov Apr 16 '12 at 23:26
1

@tsells, Note that your suggestion to use underscore prefix is against [C# coding guidelines](http://msdn.microsoft.com/en-us/library/ms229012.aspx), so not be too surprised that not everyone does so. – Alexei Levenkov Apr 16 '12 at 23:31
1

@AlexeiLevenkov Where does it say to not use _? It says don't use prefix such as m_fieldname not _fieldname. Here is an example of why I use it this way (as do many others). http://vkreynin.wordpress.com/2009/04/09/resharper-45-complains-about-my-private-fields/ – tsells Apr 16 '12 at 23:46
1

This is a way to read it too I guess. It still far from "required to use _ for fields" for me. – Alexei Levenkov Apr 17 '12 at 00:14

Ron Warholic · Answer 3 · 2012-04-16T23:06:21.473

Here's a neat little method that will return the sum of any number of random numbers as long as you iterate the IEnumerable.

public IEnumerable<int> Twist(int min, int max)  {
  Random random = new Random();
  int result = 0;
  while (true) {
    result += random.Next(min, max);  // overflow pretty likely for large max
    System.Diagnostics.Debug.WriteLine(result);
    yield return result;
  }
}

// For a single element
int oneResult = Twist(1, 5).First();

// For the fifth
int fifth = Twist(1, 5).Skip(4).First();

And just for kicks if you want to defer iteration define yourself a nice extension method:

public static class EnumeratorExt {
    public static T Next<T> (this IEnumerator<T> seq) {
        if (seq.MoveNext()) {
            return seq.Current;
        }
        return default(T);
    }
}

// Now call it like so!
using (var generator = Twist(5, 10).GetEnumerator()) {
    Console.WriteLine(generator.Next());
    Console.WriteLine(generator.Next());
    Console.WriteLine(generator.Next());
    Console.WriteLine(generator.Next());
}

score 1 · Accepted Answer · answered Apr 17 '12 at 13:57

private static int result = 0;
private static Random random = new Random();

public int twist(int min, int max)
{
    int y = random.Next(min, max);
    result += y;

    System.Diagnostics.Debug.WriteLine(result);
    return result;
}

Basically, you need to declare result outside the method so it isn't created and set to zero every time.

Note: If you are working in a .aspx.cs file you may also need to declare it as static (or a Session variable) otherwise it will be reset on every action that occurs on your webpage.

Note 2: Random is static because each instance depends on the system clock, so you just have one instance and call .Next(min, max) on that instance to get another random number.

score 0 · Answer 5 · answered Apr 16 '12 at 22:49

0

depends what you mean by "storing it". Sorting it where? A global? A file? A database? Am guess you mean a global in which case, declare your result variable outside the scope of the twist method

answered Apr 16 '12 at 22:49

Gwyn Howell

5,365
2
31
48

storing a number and incrementing it ever time the method is called

5 Answers5