Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic NumbersMagic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to usehow much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

Fixed 'exam' part of example code fragment, added spoiler for the deliberate bug to give the answer.
Source Link

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment?

I really would have to question the requirement to only use primitives... it would seem to be a rather arbitrary (and not especially useful) requirement even in an academic setting. The code would be a lot easier to write and test etc.

There are a relatively large number of Magic Numbers in your code. These can lead to problems later should the spec need to change, something which is guaranteed to happen in the real-world.

The array handling has magic numbers too. A good habit is to define an enum with values for the fields, though in this case you could use some public final int as constants to get around the primitives only requirement. This makes it clearer when you come back to the code in 3 years time (as would eventually happen in the real world). Take these two lines and decide which is clearer...

someArray[row][0] = sc.nextInt();
someArray[row][Marks.Fields.COURSEWORK_RATIO] = sc.nextInt();

...and yes, Java's all-caps convention isn't pretty.

You currently trust your user input and this is just asking for trouble in the real-world. You should as a minimum validate your input though knowing how much validation to use can require experience. Some basic bounds checking 100 >= x > 0 on the data entry certainly wouldn't go amiss.

Where you work with percentages (rather than them being raw data) I always find they're more intuitively represented if you store them as their actual value (where 50% is stored as 0.5 since it is 50/100.) This makes them far easier to use as working with them is always simple multiplication (and there's no repeated conversion between int to float and back).

Also, you're doing the rounding calculations manually and you should consider using Math.round() instead. It will compile to the same thing but it shows the intent of your code more clearly.

// original
int weighting = studentData[x][0];
int coursework = studentData[x][1];
int exam = studentData[x][2];
computedModuleMark = (int) ((((coursework * weighting) + (exam * (100 - weighting))) + 0.5) / 100);

// with weighting as a float
float weighting = studentData[x][Fields.WEIGHTING] / 100f;
int coursework = studentData[x][Fields.COURSEWORK];
int exam = studentData[x][Fields.EXAM];
computedModuleMark = Math.round(coursework * weighting + exam * (1.0 - weighting));

That should give you some more to work on.

PS: Did you spot the deliberate bug in the // original fragment? (answer below)

int coursework = studentData[x][1]; referred to the wrong array element.

This error would have been immediately obvious when using the more verbose notation which would have read int coursework = studentData[x][Fields.EXAM];

Source Link
Loading