public void AddFarmCrop_InvalidAgricultureId_ThrowValidationException() { /* * CR-1 - structure test in Arrange-Act-Assert manner */ FarmCropDto enemy = new FarmCropDto() { AgricultureId = -1, FarmerId = 1, RegionId = 1, Area = 1, Gather = 1, Name = "abc" }; try { farmService.AddFarmCrop(enemy); } /* * CR-1 - * Use Asset.Throws<> instead of try-catch block, because if excation is not be throwed, the test will pass * Target code here: * var ex = Assert.Throws<ValidationException>(() => farmService.AddFarmCrop(enemy)); * Assert.AreEqual(ex.Property, "AgricultureId"); * * Use Assert.AreEqual istead of Assert.IsTrue with lambda, because of more meaningfull output */ catch (ValidationException ex) { Assert.IsTrue(ex.Property == "AgricultureId"); } }
public void AddFarmCrop(FarmCropDto farmCrop) { //TODO: ArgumentNullException if (farmCrop == null) { throw new ValidationException($"Параметр farmCorp равен null", ""); } //TODO: здесь и далее - все ошибки пользовательского ввода собрать в одну структуру, // которую можно будет удобно использовать в контроллерах, и бросить один ValidationException //TODO: здесь и далее - строки перенести в ресурсы //TODO: здесь и далее - проверка должна быть на допустимое значение if (farmCrop.AgricultureId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.AgricultureId }", "AgricultureId"); } if (farmCrop.FarmerId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.FarmerId }", "FarmerId"); } if (farmCrop.RegionId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.RegionId }", "RegionId"); } if (string.IsNullOrEmpty(farmCrop.Name)) { throw new ValidationException($"Не задано имя фермы", "Name"); } if (farmCrop.Gather < 0) { throw new ValidationException($"Значение урожайности не может быть отрицательным", "Gather"); } if (farmCrop.Area <= 0) { throw new ValidationException($"Значение площади должно быть больше нуля", "Area"); } try { //TODO: создание из одной структуры несколько реализовать явно, за счет new Some{} //TODO: здесь и далее - вход ф-й переименоать из ...Dto в, наприемр, ...Params var farm = mapper.Map <FarmCropDto, Farm>(farmCrop); var crop = mapper.Map <FarmCropDto, Crop>(farmCrop); crop.CropFarm = farm; database.Crops.Create(crop); database.Save(); } catch (Exception ex) { //TODO: здесь и далее - бессмысленный try-catch, убрать throw new Exception("Ошибка сохранения информации об урожае на ферме", ex); } }
public void AddFarmCrop_ValidateModelMappingWhenSave_IsValid_() { /* * CR-1 - structure test in Arrange-Act-Assert manner */ bool isValid = false; var cropRepo = new Mock <IRepository <Crop> >(); cropRepo.Setup(item => item.Create(It.IsAny <Crop>())).Callback <Crop>(arg => { isValid = arg.AgricultureId == 1 && arg.Gather == 5 && arg.CropFarm.Name == "abc" && arg.CropFarm.FarmerId == 2 && arg.CropFarm.RegionId == 3; }); var iow = new Mock <IUnitOfWork>(); iow.Setup(item => item.Crops).Returns(cropRepo.Object); farmService = new FarmService(iow.Object, AutoMapperConfig.GetMapper()); FarmCropDto enemy = new FarmCropDto() { AgricultureId = 1, FarmerId = 2, RegionId = 3, Area = 4, Gather = 5, Name = "abc" }; farmService.AddFarmCrop(enemy); Assert.IsTrue(isValid); }
//Нет описания метода (summary) //В задании не сказано про уникальность имен ферм, можно допустить, что это неважно //1. Хардкодные и повторяющиеся строки, нужно использовать ресурсы или константы //2. Передача в качестве строки имени свойства, когда есть nameof(), которому не страшно переименование //3. throw new Exception - лучше использовать собственный тип исключений, чтобы в случае надобности можно было по нему сделать фильтрацию //4. catch(Exception ex) - лучше использовать глобальный обработчик исключений, а текст сообщения об ошибке запроса писать на фронтэнде, т.к. там известно что за запрос был и известно, что он не выполнился. Более того, при выбросе нового исключения затрется StackTrace исходного исключения, а это важная информация public void AddFarmCrop(FarmCropDto farmCrop) { if (farmCrop == null) { throw new ValidationException($"Параметр farmCorp равен null", ""); } //тут два варианта: 1. создать метод ValidateFarmCrop или, раз уж есть DTO, в FarmCropDto создать метод Validate, в который поместить if'ы, а здесь его вызвать //интересная проверка связанных идентификаторов....вообще, и без этой проверки при сохранении возникнет исключение, и эта проверка не поможет, если связанной сущности с указанным идентификатором нет. //Я бы удалил первые 3 if'a if (farmCrop.AgricultureId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.AgricultureId }", "AgricultureId"); } if (farmCrop.FarmerId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.FarmerId }", "FarmerId"); } if (farmCrop.RegionId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.RegionId }", "RegionId"); } //заменить IsNullOrEmpty на IsNullOrWhiteSpace //эти if'ы вынести в метод, про который я выше написал if (string.IsNullOrEmpty(farmCrop.Name)) { throw new ValidationException($"Не задано имя фермы", "Name"); } if (farmCrop.Gather < 0) { throw new ValidationException($"Значение урожайности не может быть отрицательным", "Gather"); } if (farmCrop.Area <= 0) { throw new ValidationException($"Значение площади должно быть больше нуля", "Area"); } //try-catch убрать try { var farm = mapper.Map <FarmCropDto, Farm>(farmCrop); var crop = mapper.Map <FarmCropDto, Crop>(farmCrop); //в EF такой код сработает как нужно, а с другой ORM? //я бы сначала добавил farm, потом crop.FarmId = farm.Id; и потом добавил crop crop.CropFarm = farm; database.Crops.Create(crop); database.Save(); } catch (Exception ex) { throw new Exception("Ошибка сохранения информации об урожае на ферме", ex); } }
public void Mapping_FarmCropDtoToCrop_IsValid() { var from = new FarmCropDto() { AgricultureId = 1, Area = 2, FarmerId = 3, Gather = 4, Name = "abc", RegionId = 5 }; var to = mapper.Map <FarmCropDto, Crop>(from); Assert.IsTrue(to.AgricultureId == 1 && to.Gather == 4); }
public void Mapping_FarmCropDtoToFarm_IsValid() { var from = new FarmCropDto() { AgricultureId = 1, Area = 2, FarmerId = 3, Gather = 4, Name = "abc", RegionId = 5 }; var to = mapper.Map <FarmCropDto, Farm>(from); Assert.IsTrue(to.Name == "abc" && to.Area == 2 && to.FarmerId == 3 && to.RegionId == 5); }
public void Mapping_FarmCropDtoToFarm_IsValid() { var from = new FarmCropDto() { AgricultureId = 1, Area = 2, FarmerId = 3, Gather = 4, Name = "abc", RegionId = 5 }; var to = mapper.Map <FarmCropDto, Farm>(from); //TODO: здесь и далее сделать атомарно на каждое свойство, добавить сообщение, когда тест не проходит Assert.IsTrue(to.Name == "abc" && to.Area == 2 && to.FarmerId == 3 && to.RegionId == 5); }
public void AddFarmCrop_InvalidRegionId_ThrowValidationException() { FarmCropDto enemy = new FarmCropDto() { AgricultureId = 1, FarmerId = 1, RegionId = -1, Area = 1, Gather = 1, Name = "abc" }; try { farmService.AddFarmCrop(enemy); } catch (ValidationException ex) { Assert.IsTrue(ex.Property == "RegionId"); } }
public void Mapping_FarmCropDtoToCrop_IsValid() { /* * CR-1 - structure test in Arrange-Act-Assert manner */ var from = new FarmCropDto() { AgricultureId = 1, Area = 2, FarmerId = 3, Gather = 4, Name = "abc", RegionId = 5 }; var to = mapper.Map <FarmCropDto, Crop>(from); /* * CR-1 * Separate checkings into different calls * Target code here: * Assert.AreEqual(to.AgricultureId, 1); * Assert.AreEqual(to.Gather, 4); */ Assert.IsTrue(to.AgricultureId == 1 && to.Gather == 4); }
public void AddFarmCrop_InvalidArea_ThrowValidationException() { var cropRepo = new Mock <IRepository <Crop> >(); cropRepo.Setup(item => item.Create(It.IsAny <Crop>())).Callback(() => { }); var iow = new Mock <IUnitOfWork>(); iow.Setup(item => item.Crops).Returns(cropRepo.Object); var farmService = new FarmService(iow.Object, AutoMapperConfig.GetMapper()); FarmCropDto enemy = new FarmCropDto() { AgricultureId = 1, FarmerId = 1, RegionId = 1, Area = 0, Gather = 1, Name = "abc" }; try { farmService.AddFarmCrop(enemy); } catch (ValidationException ex) { Assert.IsTrue(ex.Property == "Area"); } }
/* * CR-1 - check comments to IFarmService regarding the naming and comments for methods AddFarmCrop(), DeleteFarm(), GetFarms(), GetFarm() */ public void AddFarmCrop(FarmCropDto farmCrop) { /* * CR-1 - check argument for null */ /* * CR-1 - move validation rules into class FarmCropViewModel */ if (farmCrop == null) { throw new ValidationException($"Параметр farmCorp равен null", ""); } if (farmCrop.AgricultureId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.AgricultureId }", "AgricultureId"); } if (farmCrop.FarmerId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.FarmerId }", "FarmerId"); } if (farmCrop.RegionId < 1) { throw new ValidationException($"Неправильное значение: {farmCrop.RegionId }", "RegionId"); } if (string.IsNullOrEmpty(farmCrop.Name)) { throw new ValidationException($"Не задано имя фермы", "Name"); } if (farmCrop.Gather < 0) { throw new ValidationException($"Значение урожайности не может быть отрицательным", "Gather"); } /* * CR-1 - change validation condtition from "<=" to "<" */ if (farmCrop.Area <= 0) { throw new ValidationException($"Значение площади должно быть больше нуля", "Area"); } try { var farm = mapper.Map <FarmCropDto, Farm>(farmCrop); /* * CR-1 - set up AutoMapper fill inner crops collection of farm instance, get rid of crop variable */ var crop = mapper.Map <FarmCropDto, Crop>(farmCrop); /* * CR-1 * Rewrite two lines below as adding farm, instead of adding crop * Target code here: * database.Farms.Create(farm); */ crop.CropFarm = farm; database.Crops.Create(crop); database.Save(); } /* * CR-1 - move handling of data access exceptions to repository class */ catch (Exception ex) { throw new Exception("Ошибка сохранения информации об урожае на ферме", ex); } }