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);
}
}
