שלום רב,

עליי לכתוב תוכנית שבכל לחיצה על כפתור, יתווספו לLISTBOX שישה מספרים אקראיים ושונים זה מזה. בנוסף, יש למצוא את המספר הגבוה ביותר בLISTBOX. על פניו מדובר בתרגיל פשוט אך מכיוון שהשפה חדשה לי, נתקלתי במספר בעיות. אשמח לקבל עזרה.

בעיה ראשונה:

כאשר אני לוחץ על הכפתור של הוספת המספרים, התוכנית לעיתים קופאת.

        private void button1_Click(object sender, EventArgs e)
        {
            int[] box = new int[6];
            Random rnd = new Random();
            for (i = 0; i < box.Length; i++)
                box[i] = rnd.Next(1, 38);
                for (i = 0; i < box.Length; i++)
                {
                    for (j = 0; j < i; j++)
                        if (box[j] == box[i])
                            i--;
                    listBox1.Items.Add(box[i]);
                }
        }

בעיה שנייה:

אין לי מושג מה צריך לכתוב בתנאי ובהשמה איפה שיש סימני שאלה.

        private void button2_Click(object sender, EventArgs e)
        {

            int max=1;
            for (i = 0; i < listBox1.Items.Count ; i++)
                if(max < ?)
                   max = ?;
            MessageBox.Show(max.ToString());
        }

תודה מראש לעוזרים

נערך 2013 בנובמבר 2711 שנים על-ידי apu500

אני לא מומחה גדול ב#c אבל נראה לי שככה זה יותר טוב

אני לא חושב שצריך את כל הקוד הזה לפי דעתי הקוד הזה יספיק בשביל לאתחל 6 items בסמפר ראנדומלי:

Random random = new Random();
            for (int i = 0;i!=6;i++ )
            {
                listBox1.Items.Add(random.Next(1, 38));
            }

ככה לא צריך ליצור משתנים בכלל פשוט יוצרים item חדש ואתחלים אותו למספר ראנדומלי.

מה שכן אני רואה שלפעמים הוא מביא לי את אותו מספר כמה פעמים, תכף נראה איך נסדר את זה.

נערך 2013 בנובמבר 2711 שנים על-ידי eido300

עדכון קצרצר, הצלחתי לפתור את הבעיה הראשונה.

איך?

        private void btn_start_Click(object sender, EventArgs e)
        {
            int i, j;
            int[] box = new int[6];
            Random rnd = new Random();
            for (i = 0; i < box.Length; i++)
            {
                box[i] = rnd.Next(1, 38);
                for (j = 0; j < i; j++)
                    if (box[j] == box[i])
                    {
                        i--;
                        break;
                    }
                listBox1.Items.Add(box[i]);
            }
        }

נערך 2013 בנובמבר 2711 שנים על-ידי apu500

זה הפתרון לבעיה השניה.

 int max=1;
            for (int i = 0; i < listBox1.Items.Count ; i++)
                if(max < (int)listBox1.Items[i])
                    max = (int)listBox1.Items[i];
            MessageBox.Show(max.ToString());

תודה רבה!

נערך 2013 בנובמבר 2711 שנים על-ידי apu500

הוא עדיין מציג מספרים כפולים.

נערך 2013 בנובמבר 2711 שנים על-ידי eido300

עדיין לא פתרת את הבעיה שהוא לא מציג מספרים שונים.

צודק, שמתי לב אחרי שרשמתי כבר את ההודעה אך לפחות התוכנית כבר לא קופאת.

הוספתי דגל והבעיה של הכפילויות נעלמה:

        private void button1_Click(object sender, EventArgs e)
        {
            int[] box = new int[6];
            Random rnd = new Random();
            for (int i = 0; i < box.Length; i++)
            {
                int flag = 1;
                box[i] = rnd.Next(1,38);
                for (int j = 0; j < i; j++)
                    if (box[j] == box[i])
                    {
                        flag = 0;
                        i--;
                    }
                    if(flag == 1)
                       listBox1.Items.Add(box[i]);  
            }
        }

כמה דברים:

א. אל תיצור את ה-Random בתוך הפונקציה. שמור אותו כשדה של המחלקה וצור אותו רק פעם אחת.

ב. למה flag הוא לא bool? ולמה קוראים לו בשם הזה, ולא בשם יותר אינפורמטיבי?

ג. תמיד תמיד תמיד תמיד תמיד תשים סוגריים מסולסלים אחרים if/for/while. זה הופך את הקוד ליותר קריא ופחות מועד לטעויות. לדוגמה כשקראתי את הקוד שלך עכשיו ניסיתי להבין למה את שתי השורות האחרונות (אלו שמוסיפות את האיבר לרשימה אם אפשר) עשית בתוך הלולאה, ורק אז הבנתי שלא שמת סוגריים מסולסלים אחרי ה-for.

ד. לא מומלץ להתעסק עם משתני לולאה באמצע (אני מדבר על איפה שאתה עושה --i), כי זה יכול לבלבל את הקורא ומונע אופטימיזציות. הדבר הנכון הוא להוסיף עוד לולאת while בתוך לולאת ה-for.

רציתי להעיר על הבעיה השנייה: דרך המימוש שלך היא אומנם נכונה אבל לא יעילה מכיוון שבכל פעם שאתה מפעיל את קטע הקוד הזה (של button2) אתה עובר על כל איברי המערך מחדש ובודק עבור כולם מי הכי גדול. כלומר אם היו לך 100,000 איברים במערך אז פעולה זו הייתה לוקחת קצת זמן..

ניתן לפתור את זה בדרך מאוד פשוטה ויעילה אם היית מצרף את משתנה max לקטע הקוד הראשון שלך והיית בודק עבור כל הכנסה של מספר חדש ורנדומלי ל listbox האם המספר החדש שהכנסת הוא יותר גדול מ max, במידה וכן אז max יקבל את המספר הזה.

כעת משתנה max תמיד יחזיק לך את המספר הכי גדול וכשתרצה להוציא את המספר הכי גדול בשלב מסוים (לחיצה על button2) אתה יכול פשוט להחזיר את max ולא לעבור על כל האיברים ב listbox.

יכול להיות שעבור התרגיל שלך להתעסק עם זמני ריצה ואופטימיזציה זה לא רלוונטי אבל זה תמיד טוב לחשוב על הפתרונות האלה עבור עבודות בעתיד וגם כדי לפתח את החשיבה.