Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here is the project which currently I'm working.

http://www.megaupload.com/?d=NGJWS9C3

Sorry guys if the code is a mess. I'm a beginner, I'm still learning from you, and for that reason, I need a little of help to improve this code. It's not finished.

I'm creating a Math quiz system. And there are so many questions, as you will be able to see in it they're so different and I have a difficult momment to create the system.

The quiz is generated randomly, many problems of them are differently. For example:

1 + 3 = X, X is an integer.

0.5 = X / Y, X and Y are integers, and they're stored in a vector.

X <= Y <= Z, X, Y, and Z are doubles, and they're also stored in an array.

I need to store each student's answer. I've got a Question class and it has a Result property where contains his answer, but its datatype is object, and I think that's not correct.

This is the second problem: And I'm using several user controls (I don't know if this is the best way to do it) to show the questions or problems.

Please, check it out the solution. Any question you have, please let me know. That's what I have.


Ok, this will be the first question for this project.

This is a Math quiz system. Always must generate different problems for each problem, but at the same time, I have to generate the same exam for each one (using the same seed in random class).

Many problems are different. There are times tables, addition and subtraction of signed numbers, comparing integers, comparing fractions, convert fraction to decimal, and others I can tell while I advance to explain you.

The problems must be between in a range and there mustn't be repeat problems.

I was using several jagged arrays. The code for me is awful because sometimes I don't even understand it.

For example: There are problems that using the same structure ( NUMBER1 OPERATOR NUMBER 2 = RESULT )

This is the pattern for it:

        int[][] arrayLimits1 = new int[][]
        {
            new int[] { 6, 10, 3, 10 }, new int[] { 6, 10, 3, 10 },
            new int[] { 6, 10, 3, 10 }, new int[] { 6, 10, 3, 10 },
            new int[] { 6, 10, 3, 10 }, new int[] { 6, 10, 3, 10 },
            new int[] { 6, 10, 3, 10 }, new int[] { 6, 10, 3, 10 },
            new int[] { 6, 10, 3, 10 }, new int[] { 6, 10, 3, 10 },
            new int[] { 15, 50, -95, -50 }, new int[] { -50, -15, -95, -50 }, new int[] { -50, -15, -95, -50 },
            new int[] { 15, 50, -95, -50 }, new int[] { -50, -15, -95, -50 }, new int[] { -50, -15, -95, -50 },
            new int[] { 6, 10, -10, -3 }, new int[] { -10, -6, 3, 10 }, new int[] { -10, -6, -10, -3 },
            new int[] { 6, 10, -10, -6, 1 }, new int[] { -10, -6, 6, 10, 1 }, new int[] { -10, -6, -10, -6, 1 }, 
            new int[] { 300, 900, 15, 90 }, new int[] { 300, 900, 25, 50 }, new int[] { 2000, 10000, 105, 900 }, new int[] { 12, 20, 25, 50, 1},
            new int[] { 10, 50, -50, -10 }, new int[] { -100, -50, -100, -50 }, new int[] { -10, 10, -10, 10 }, new int[] { -100, 0, -100, 0  }   
        };

        int[][] problems1 = GeneratePattern1(arrayLimits1);

The generate function, it receives the array. The first index in the array means min and max of NUMBER1, and others two for NUMBER2.

    private int[][] GeneratePattern1(int[][] arrayLimits)
    {
        int[][] sel = new int[arrayLimits.Length][];
        for (int i = 0; i < arrayLimits.Length; i++)
        {
            int[] temp;
            do
            {
                temp = Mathematics.GeneratePairOfIntegers(arrayLimits[i][0], arrayLimits[i][1], arrayLimits[i][2], arrayLimits[i][3]);

                if (arrayLimits[i].Length > 4)
                    temp[0] *= temp[1];

            } while (AuxiliaryMethods.Exists(sel, temp, i));

            sel[i] = temp;
        }

        return sel;
    }

    public static int[] GeneratePairOfIntegers(int li1, int ls1, int li2, int ls2)
    {
        int[] pair = new int[2];
        pair[0] = randomizer.Next(li1, ls1);
        pair[1] = randomizer.Next(li2, ls2);

        return pair;
    }

    public static bool Exists<T>(T[] array, T elem, int capacity)
    {
        for (int i = 0; i < capacity; i++)
        {
            if (array[i].Equals(elem))
                return true;
        }
        return false;
    }

All the array has Count=4, except some because has a number "1". This means that the problem is a division and I want to generate divisions with a remainder equals to zero.

I had to create another function for each type of problem. Last function was for integer problems but if I want doubles..

    private double[][] GeneratePattern2(double[][] arrayLimits)
    {
        double[][] sel = new double[arrayLimits.Length][];
        for (int i = 0; i < arrayLimits.Length; i++)
        {
            double[] temp;
            do
            {
                temp = Mathematics.GeneratePairOfDoubles(arrayLimits[i][0], arrayLimits[i][4], (int)arrayLimits[i][5],
                                                         arrayLimits[i][6], arrayLimits[i][7], (int)arrayLimits[i][5]);

                if (arrayLimits[i].Length > 4)
                    temp[0] = Convert.ToDouble(Convert.ToDecimal(temp[1]) * Convert.ToDecimal(temp[0]));

            } while (AuxiliaryMethods.Exists(sel, temp, i));

            sel[i] = temp;
        }

        return sel;
    }

    public static double[] GeneratePairOfDoubles(double li1, double ls1, int decimals1, double li2, double ls2, int decimals2)
    {
        double[] pair = new double[2];
        pair[0] = randomizer.NextDouble(li1, ls1, decimals1);
        pair[1] = randomizer.NextDouble(li2, ls2, decimals2);

        return pair;
    }

I think I'm being redundant. I need to generate the problems, how can I generate a class to store each one? Because I'm using several jagged arrays and I think that's bad.

9 x 9 = __ <-- Only an integer

49 + -89 = __ <-- Only an integer

0.75 (decimal) = _ (fraction) <-- Two integers

3/4 (fraction) = _ (decimal) <-- A double

share|improve this question
6  
Great that you've come forward to improve yourself! All code seems messy and unfinished after a few years of becoming a different kind of a beginner. To help us help you, you could pick some key parts of the code you want to get review and paste it to this question - it's a lot friendlier than having to go through megaupload and whatever comes from there. If there are several different things you want to get reviewed then you might create a separate question for each of them. If you want your classes and their relationships to be reviewed then a simple class diagram may help. –  Aleksi Yrttiaho Aug 2 '11 at 6:35
4  
Just to add to @Aleksi, posting the code here also helps to preserve the questions and answers. If the code is ever removed from megaupload, none of the answers will make much sense. –  Levinaris Aug 2 '11 at 12:40
add comment

1 Answer

Without attempting to rewrite the entire code, I think you need to think about keeping your functions with your data. For instance, let's say you have an IBounds interface.

interface IBounds {
  Tuple<double, double> GeneratePairsOfDoubles();
}

you can then declare implementations

class MultiplicationBounds : IBounds {}
class DivisionBounds : IBounds {}

Each of these "Bounds" objects would correspond to one of the int arrays, but would have named fields saying what each element did.

This is obviously an over-simplification of what you're trying to do, but it you break it down like this, you'll eventually find the class structure that works for your project. Even in dynamically typed languages, you don't keep behaviour too separated from the data.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.