2013-06-11 3 views
-1

Я написал следующий класс, чтобы вернуть случайное число, как прокатке кости:Понимание классов и случайный

using System; 
using System.Collections.Generic; 
using System.Linq; 
using System.Text; 

namespace GameTest 
{ 
    class Dice 
    { 
    public int publicMinNum 
    { 
     get { return _minNum; } 
     set { _minNum = value; } 
    } 

    public int publicMaxNum 
    { 
     get { return _maxNum; } 
     set { _maxNum = value; } 
    } 

    static int _minNum; 
    static int _maxNum; 

    static Random diceRoll = new Random(); 
    public int rolled = diceRoll.Next(_minNum, _maxNum); 
} 
} 

Этот класс называется пару раз в моей форме:

private void btnPushMe_Click(object sender, EventArgs e) 
    { 
     Dice myRoll = new Dice(); 
     myRoll.publicMinNum = 1; 
     myRoll.publicMaxNum = 7; 

     lblMain.Text = myRoll.rolled.ToString(); 

     Dice mySecondRoll = new Dice(); 
     mySecondRoll.publicMinNum = 1; 
     mySecondRoll.publicMaxNum = 13; 

     lblMain2.Text = mySecondRoll.rolled.ToString(); 
    } 

Как вы можете видеть, я дважды вызываю класс myRoll и mySecondRoll. Я думал, что, делая это, он будет создавать отдельные экземпляры класса и вывода два отдельных номера (один от 1 до 6, а другой 1 и 12)

Проблемы, которые я имею являются:

1) первый номер всегда равен 0.

2) два экземпляра класса мешают друг другу, т. е. число, которое должно быть от 1 до 6, - это не так.

Мне интересно, а не только, как исправить код, но также хотелось бы объяснить, что здесь происходит и почему, спасибо.

+3

Спросить учебники (или другие общие вопросы) не являются конструктивными , не соответствуют формату Q & A сайтов Stack Exchange и приводят к обсуждению. –

+0

С этим кодом возникает несколько проблем. Ваши целые поля min и max не должны быть статичными, ваше свернутое поле не должно быть общедоступным и не инициализировано так, как оно есть. Но в основном, вы правы, что вам нужен лучший учебник, но, к сожалению, это не тема для этого сайта. Я проголосовал за закрытие как не конструктивное. –

+0

http://www.blackwasp.co.uk/CSharpClassProperties.aspx – Learner

ответ

5

Проблема в том, что вы объявляете поля в классе Dice как static. Это означает, что будет только один экземпляр этой переменной, который будет использоваться для всех экземпляров класса в приложении.

Следующая строка:

public int rolled = diceRoll.Next(_minNum, _maxNum); 

... запускаемый в тот момент, вы создаете свой new Dice(), что означает, что вы еще не инициализированы свое _minNum и _maxNum значения еще: именно поэтому он дает вам 0 , Вы могли превратить это свойство, так что код будет ждать, чтобы работать, пока вы не просили об этом:

public int Rolled { get { return diceRoll.Next(_minNum, _maxNum); } } 

... но как правило, свойства не изменится, просто просят их стоимости. Этот тип кода имеет тенденцию создавать так называемые Heisenbugs, которые очень трудно отследить, потому что поведение системы изменяется просто, пытаясь его наблюдать.

Так вот один из способов вы можете переписать свой класс, используя метод Roll() на самом деле выполнить бросок, и это свойство, которое позволяет коду продолжать проверять на значение последнего валка при необходимости:

public class Die 
{ 

    // Using a constructor makes it obvious that you expect this 
    // class to be initialized with both minimum and maximum values. 
    public Die(int minNum, int maxNum) 
    { 
     // You may want to add error-checking here, to throw an exception 
     // in the event that minNum and maxNum values are incorrect. 

     // Initialize the values. 
     MinNum = minNum; 
     MaxNum = maxNum; 

     // Dice never start out with "no" value, right? 
     Roll(); 
    } 

    // These will presumably only be set by the constructor, but people can 
    // check to see what the min and max are at any time. 
    public int MinNum { get; private set; } 

    public int MaxNum { get; private set; } 

    // Keeps track of the most recent roll value. 
    private int _lastRoll; 

    // Creates a new _lastRoll value, and returns it. 
    public int Roll() { 
     _lastRoll = diceRoll.Next(MinNum, MaxNum); 
     return _lastRoll; 
    } 

    // Returns the result of the last roll, without rolling again. 
    public int LastRoll {get {return _lastRoll;}} 

    // This Random object will be reused by all instances, which helps 
    // make results of multiple dice somewhat less random. 
    private static readonly Random diceRoll = new Random(); 
} 

(обратите внимание, что «смерть» - это особая форма «кости»). Использование:

private void btnPushMe_Click(object sender, EventArgs e) 
{ 
    Die myRoll = new Die(1, 7); 
    lblMain.Text = myRoll.Roll().ToString(); 

    Die myRoll2 = new Die(1, 13); 
    lblMain2.Text = mySecondRoll.Roll().ToString(); 
} 
+0

Спасибо. У меня было ощущение, что это может быть так, но почему-то VS отчаянно хочет, чтобы эти переменные были статичными или они бомбили. Я предполагаю, что есть лучший способ построить его, который обойдет это? – Chris

+0

@Chris Вы называете это статическим методом или классом? –

+0

@ Крис: определить «бомбы». Скорее всего, это потому, что вы пытаетесь получить доступ к одному из этих полей из статического контекста. – StriplingWarrior

1

Я хотел бы изменить свой класс, чтобы выглядеть как следующее:

class Dice 
{ 
    // These are non-static fields. They are unique to each implementation of the 
    // class. (i.e. Each time you create a 'Dice', these will be "created" as well. 
    private int _minNum, _maxNum; 

    // Readonly means that we can't set _diceRand anywhere but the constructor. 
    // This way, we don't accidently mess with it later in the code. 
    // Per comment's suggestion, leave this as static... that way only one 
    // implementation is used and you get more random results. This means that 
    // each implementation of the Dice will use the same _diceRand 
    private static readonly Random _diceRand = new Random(); 

    // A constructor allows you to set the intial values. 
    // You would do this to FORCE the code to set it, instead 
    // of relying on the programmer to remember to set the values 
    // later. 
    public Dice(int min, int max) 
    { 
    _minNum = min; 
    _maxNum = max; 
    } 

    // Properties 
    public Int32 MinNum 
    { 
    get { return _minNum; } 
    set { _minNum = value; } 
    } 

    public Int32 MaxNum 
    { 
    get { return _maxNum; } 
    set { _maxNum = value; } 
    } 

    // Methods 
    // I would make your 'rolled' look more like a method instead of a public 
    // a variable. If you have it as a variable, then each time you call it, you 
    // do NOT get the next random value. It only initializes to that... so it would 
    // never change. Using a method will have it assign a new value each time. 
    public int NextRoll() 
    { 
    return _diceRand.Next(_minNum, _maxNum); 
    }  
} 
+2

На самом деле, 'Random()' static хорош. Обычно вам нужен только один случай случайности, чтобы получить лучшие случайные результаты. - В противном случае, когда несколько randoms создаются в (очень) близко к тому же времени, их выход станет предсказуемым –

+0

Согласен, и я обновил свой ответ. – Andrew

+0

Ваши обновления хорошо приняты. Голосование перевернуто. –

1

прибудет/сеттера поле отступающего помечено как «static». Если переменная объявлена ​​«static», значение сохраняется в приложении и делится между различными экземплярами того типа, в котором они живут.

См. here.

Кроме того,

так как ваши свойства класса не содержат никакой логики, я предлагаю использовать «automatic» свойства.

class Dice 
    { 
     public int publicMinNum { get; set; } 
     public int publicMaxNum { get; set; } 
     Random diceRoll = new Random(); 
     public int rolled = diceRoll.Next(publicMinNum , publicMaxNum); 
    } 

учебник по automatic propertieshere.

1

Ваша проблема связана с членами static.

Из документации MSDN по адресу static: «Хотя экземпляр класса содержит отдельную копию всех полей экземпляра класса, существует только одна копия каждого статического поля».

5

Вопрос Два уже aswered: поскольку переменные являются статическими:

static int _minNum; 
static int _maxNum; 

Вопрос Один с другой стороны, разве ответил еще, так что здесь идет:

public int rolled = diceRoll.Next(_minNum, _maxNum); 

это не какой-то динамический вызов , Это инициализация поля и будет установлена ​​еще до конструктора. Вы можете проверить это, отлаживая кубики в первый раз.

в этой точке, как _minNum и _maxNum по-прежнему 0, поэтому прокатке будет установлен в 0

это может быть исправлено путем поворота свернутый в собственность тоже:

public int rolled 
    { 
     get { return diceRoll.Next(_minNum, _maxNum); } 
    } 

В настоящее время _minNum и _maxNum устанавливаются в первый раз, потому что они статичны, поэтому, когда вы создаете вторую кубик, они уже установлены.

Edit, так как рекомендация был задан вопрос, это то, как я его создать:

Кости

class Dice 
{ 
    private static Random diceRoll = new Random(); 

    private int _min; 
    private int _max; 
    public int Rolled { get; private set; } 

    public Dice(int min, int max) 
    { 
     _min = min; 
     _max = max; 

     // initializes the dice 
     Rolled = diceRoll.Next(_min, _max); 
    } 

    public int ReRoll 
    { 
     get 
     { 
      Rolled = diceRoll.Next(_min, _max); 
      return Rolled; 
     } 
    } 
} 

Обратите внимание на кости имеет два свойства: Rolled и ReRoll. Поскольку ваше намерение неясно, я добавил оба, чтобы проиллюстрировать поведение.

Rolled устанавливается конструктором. Если вы хотите новый номер, вы можете ReRoll.

Если вы намеренно хотели, чтобы срок службы рулона составлял один кубик (но я так не думаю), вы удалите метод ReRoll.

Кости будет называться так:

private static void Main(string[] args) 
    { 
     Dice myRoll = new Dice(1, 7); 

     // All the same 
     var result1 = myRoll.Rolled.ToString(); 
     var result2 = myRoll.Rolled.ToString(); 
     var result3 = myRoll.Rolled.ToString(); 

     // something new 
     var result4 = myRoll.ReRoll.ToString(); 

     Dice mySecondRoll = new Dice(1, 13); 
     var result = mySecondRoll.ReRoll.ToString(); 
    } 
+0

Это работает над лучшим решением, чем другие ответы, поскольку он обращается к инициализатору 'rollled'. Тем не менее, должно быть более ясным и самодостаточным то, что вы хотите сделать с текущими статическими полями (явное указание вашей рекомендации), и я думаю, что лучшая рекомендация для получения свернутого значения в порядке (вероятно, вызов метода, чем свойство). –

+0

Это действительно отлично работает, даже если логика использования статического кода неверна и нуждается в исправлении. Спасибо за вашу помощь! – Chris

+0

Добро пожаловать. Я добавил пример по запросу пользователя @ user414076 –

1

Я думаю, что реальная проблема здесь состоит в том, что вы не совсем смоделировали Die правильно.

У штампа есть минимальное и максимальное значение (которые определяют начало и конец диапазона), но как только штамп был сделан, вы не можете изменить это, то есть шестисторонняя матрица не превращается в восьмистороннюю умереть. Таким образом, нет необходимости в публичных сеттерах.

Теперь не все умирают совместно использовать один и тот же диапазон, это то, что характерно для каждой матрицы, и поэтому эти свойства должны принадлежать экземпляру, а не быть static.

Снова каждый объект штампа имеет значение CurrentRoll, представляющее число, которое находится лицом вверх, и это действительно генерируется наугад. Однако для изменения CurrentRoll штампа вам нужно Roll.

Это оставляет реализацию Die смотрит на что-то вроде

class Die 
{ 
    private static Random _random; 

    public int CurrentRoll { get; private set; } 

    public int Min { get; private set; } 

    public int Max { get; private set; } 

    public Die(int min, int max) 
    { 
     Min = min; 
     Max = max; 
     Roll(); 
    } 

    public int Roll() 
    { 
     CurrentRoll = _random.Next(Min, Max+1); // note the upperbound is exlusive hence +1 
     return CurrentRoll; 
    } 
} 

и вы бы использовать его как

public static void Main() 
{ 
    Die d1 = new Die(1, 6); 
    Die d2 = new Die(1, 6); 

    Console.WriteLine(d1.Roll()); 
    Console.WriteLine(d2.Roll()); 
    //... 
} 

demo

Смежные вопросы