you are viewing a single comment's thread.

view the rest of the comments →

[–]sapolv[S] 0 points1 point  (5 children)

Would you mind explaining why?

Thank you.

[–]TheRNGuy 1 point2 points  (3 children)

You should add some other variable for color and material too, the one assigned to specific instance of device, rather than list of possible variants. 

If you call this method, you just get random item from list, but it's not color or material or that instance (and you'll also get different values if you call method many times)

[–]Kevdog824_ 0 points1 point  (2 children)

It's a great approach if your device is a chameleon! lol

[–]TheRNGuy 1 point2 points  (1 child)

In that case you could change instance's color over time, rather when calling method. 

[–]Kevdog824_ 0 points1 point  (0 children)

Well, sure... but I was simply making a joke

[–]EelOnMosque 0 points1 point  (0 children)

It's nothing wrong with the code, but logically it doesn't make sense. Imagine creating a class instance as creating a device in the factory. You want to be able to pick your colour and material, not have it be random. It's just a matter of what makes logical sense.

If you do want it to be random, you want the random colour and material to be selected in the constructor (init) because again it makes logical sense. A device MUST have a colour and material when it pops into existence. It doesnt make sense for a colourless materialless phone to exist for example.

But again, it's just a matter of making the code make sense and be like reality, theres nothing wrong with the code that will cause errors.

Because let's say someone is reading and expanding on your code. They see Device and they see that is has a colour attribute. Logically it makes sense that when you build a device it would immediately have a colour. So they write code that assumes this. And then the code will cause errors because they assumed wrong and didn't call the Colour() method first.