programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
• Campbell Ritchie
• Tim Cooke
• Devaka Cooray
• Ron McLeod
• Jeanne Boyarsky
Sheriffs:
• Liutauras Vilda
• paul wheaton
• Junilu Lacar
Saloon Keepers:
• Tim Moores
• Stephan van Hulst
• Piet Souris
• Carey Brown
• Tim Holloway
Bartenders:
• Martijn Verburg
• Frits Walraven
• Himai Minh

# Refactoring and testing

Greenhorn
Posts: 10
• Number of slices to send:
Optional 'thank-you' note:
Hello All.

I am stuck with a problem with creating unit test for refactoring for this code

Thanks

Rancher
Posts: 1059
27
• Number of slices to send:
Optional 'thank-you' note:
I have no hint on unit testing for you, as this is a fairly straight forward routine, but did you realize that you have 5 exactly the same statements:

That can be replace with:

BTW: where to you calculate "something"?

Marshal
Posts: 76867
366
• Number of slices to send:
Optional 'thank-you' note:
And it can be simplified by removing every instance of != false.
Also don't mess around with Math.random() and arithmetic. Use
1 + random.nextInt(6)
where random is an instance of the Random class.

Ranch Hand
Posts: 385
6
• Number of slices to send:
Optional 'thank-you' note:

Les Morgan wrote:
That can be replace with:

I am confused. Surely the if(m[j]!= false) should be inside the for loop, no?

I mean, in the above code, if j> 0, if m[j] is false, we will still set d[j] ... but the OP doesn't want to set d[j] if m[j] is false. So the OP doesn't want to, for example, set d[4] if m[4] is false.

Marshal
Posts: 5471
326
• Number of slices to send:
Optional 'thank-you' note:
Well it looks like you've already gotten a whole load of help at another forum where you posted the exact same question.

Was there something you weren't happy with from their help that compelled you to ask again here?

Les Morgan
Rancher
Posts: 1059
27
• Number of slices to send:
Optional 'thank-you' note:
Indeed! Good catch, it seems your mind is more awake this morning than mine.

Ahmed Bin S wrote:

Les Morgan wrote:
That can be replace with:

I am confused. Surely the if(m[j]!= false) should be inside the for loop, no?

I mean, in the above code, if j> 0, if m[j] is false, we will still set d[j] ... but the OP doesn't want to set d[j] if m[j] is false. So the OP doesn't want to, for example, set d[4] if m[4] is false.

Ahmed Bin S
Ranch Hand
Posts: 385
6
• Number of slices to send:
Optional 'thank-you' note:

Les Morgan wrote:Indeed! Good catch, it seems your mind is more awake this morning than mine.

Well it is late afternoon here, had it been morning my mind would probably have been as asleep as yours!

Les Morgan
Rancher
Posts: 1059
27
• 1
• Number of slices to send:
Optional 'thank-you' note:
So thanks to Ahmed's keen eyes and other suggestions:

Ahmed Bin S wrote:

Les Morgan wrote:Indeed! Good catch, it seems your mind is more awake this morning than mine.

Well it is late afternoon here, had it been morning my mind would probably have been as asleep as yours!

Campbell Ritchie
Marshal
Posts: 76867
366
• Number of slices to send:
Optional 'thank-you' note:

Les Morgan wrote:. . . . . .

No, no, no, no, no.

Apart from the poor formatting, there is a serious logical error in your translation from the original.

Mohamed Fareedh
Greenhorn
Posts: 10
• Number of slices to send:
Optional 'thank-you' note:

Campbell Ritchie wrote:

Les Morgan wrote:. . . . . .

No, no, no, no, no.

Apart from the poor formatting, there is a serious logical error in your translation from the original.

WHat is the problem witth the above suggestion?

Campbell Ritchie
Marshal
Posts: 76867
366
• Number of slices to send:
Optional 'thank-you' note:
It is a very little problem to see, and not easy to find, but it is a serious logical change. I shall give you a bit longer to find it.

Les Morgan
Rancher
Posts: 1059
27
• Number of slices to send:
Optional 'thank-you' note:
My mind is really not up yet: the "!=false" just drops out, so:

I mistakenly put a negation in front of the boolean... no "!" should have been translated over. Good catch Campbell.

Mohamed Fareedh wrote:

Campbell Ritchie wrote:

Les Morgan wrote:. . . . . .

No, no, no, no, no.

Apart from the poor formatting, there is a serious logical error in your translation from the original.

WHat is the problem witth the above suggestion?

Campbell Ritchie
Marshal
Posts: 76867
366
• 1
• Number of slices to send:
Optional 'thank-you' note:
That goes to show how error‑prone things like != false are. If the original code has simply said
if (m[i]) ...
which would have been correct style, there would have been no chance of such errors. Of course using single‑letter names for variables is error‑prone, too.

Bartender
Posts: 1845
10
• Number of slices to send:
Optional 'thank-you' note:
Just because nobody has mentioned the testing part of it yet...

One of the base things about a test is that it needs to be reproducible. i.e. have the same result every time.
The use of the random numbers in here is going to throw a spanner in the works for that.
As already suggested, the Random class is better than Math.Random because it gives you the nextInt() method, but it would also make your code testable, because the Random class can be told to always generate the same set of random numbers from a given seed - thus making your test repeatable with the same results.

Also with regards to testing, what do you want to assert?
My guess would be that you test only the 'dice' you have selected for 'reroll' get rerolled.
Using objects rather than primitives would potentially help there. If you had a Dice class with a reroll method, you could test for executions of "reroll".
All you can do here I think is maybe set up the array with a list of known values, and make sure that only the ones that you want to get changed.

for instance if you set d to be {0,0,0,0,0} and m to be {true, false, true, false, true}
you would expect to end up with {x, 0, y, 0, z} where x,y and z are your "random" numbers.

 There's a city wid manhunt for this tiny ad: the value of filler advertising in 2021 https://coderanch.com/t/730886/filler-advertising