changeDieClasses --- #62
Merged
alexis.drai
merged 7 commits from changeDieClasses into main 3 years ago
alexis.drai
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'changeDieClasses'
Deleting a branch is permanent. It CANNOT be undone. Continue?
implemented chages:
-- Not tested yet --
changeDieClassesto changeDieClasses --- 3 years agoChanging the Dice class is a good idea, and I'm looking forward to these changes being merged into the main branch.
We'll proceed with the merge as soon as the problems mentionned in the review are solved
private readonly List<T> listFaces = new();public AbstractDie(string name, params T[] faces)From SonarQube issue https://codefirst.iut.uca.fr/sonar/project/issues?resolved=false&types=CODE_SMELL&id=dice-app&open=AYNrqm1m8oTwQX-NU-Nz
Since abstract classes can’t be instantiated, there’s no point in their having public or internal constructors. If there is basic initialization logic that should run when an extending class instance is created, you can by all means put it in a constructor, but make that constructor private, private protected or protected.
{}public override AbstractDieFace GetRandomFace()code duplication, this method could be implemented directly in the abstract class
public override AbstractDieFace GetRandomFace(){Random rnd = new();we should create only one instance of Random for the whole app.
I think the best way to do that is to create a static instance of Random as part of the AbstractDie class.
public ImageDie(string name, params ImageDieFace[] faces) : base(name, faces){}public override ImageDieFace GetRandomFace()code duplication, this method could be implemented directly in the abstract class
public NumberDie(string name, params NumberDieFace[] faces) : base(name, faces){}public override NumberDieFace GetRandomFace()code duplication, this method could be implemented directly in the abstract class
using Model;This whole file can be deleted for now, until unit tests are made
4fe026a094into main 3 years agoReviewers
4fe026a094.