C# 7 language feature updates

Controllers/AccountController:
Replace single line methods with expression bodied members.
Includes constructor, C# 7 feature.

Controllers/CatalogController:
Remove unnecessary ToString() call.
Replace single line methods with expression bodied members.

Extensions/HttpClientExtensions:
Replace single line methods with expression bodied members.

Extensions/SessionExtensions:
Replace single line methods with expression bodied members.

Services/BasketService:
Remove unnecessary ToString() calls.
Add ?? to simplify conditional initialization
Use TryGetValue and out variable initialization to simplify Quantity
calculation

Services/CatalogService:
Use Value<T> generic method instead of dynamic types.
There is a lot of overhead for dynamic and it seems overkill here.

Services/IdentityParser:
Use the pattern matching is expression.
Refactor the LINQ queries to enumerate the collection (and create an
enumerator)
only once. The previous code had 3 enumerations.

Services/Utilities/HttpApiClient:
Remove the 'async' modifier and 'await' statements from methods where
the only asynchronous statement is the final statement of the method,
and
the task from the underlying method can be returned.

Services/Utilities/HttpApiClientWrapper:
Use expression bodied members where applicable.
Remove the 'async' modifier and 'await' statements from methods where
the only asynchronous statement is the final statement of the method,
and
the task from the underlying method can be returned.

ViewComponents/Cart:
Use expression bodied members where applicable.

ViewComponents/CartList:
Use expression bodied members where applicable.
Remove the 'async' modifier and 'await' statements from methods where
the only asynchronous statement is the final statement of the method,
and
the task from the underlying method can be returned.

ViewModels/Annotations/CardExpiration:
Use the out variable initializer to simplify the validation of the
card expiration date.

ViewModels/Basket:
Use property initializer syntax instead of constructor body

ViewModels/CatalogViewModels/IndexViewModel:
Use expression bodied property to return the calculated 'Disabled'
property

ViewModels/Order:
Use property initializer syntax.
This commit is contained in:
Bill Wagner 2017-03-20 14:18:20 -04:00
parent aee1ac6a06
commit 00491910a2
15 changed files with 173 additions and 167 deletions

View File

@ -11,16 +11,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers
public class AccountController : Controller
{
private readonly IIdentityParser<ApplicationUser> _identityParser;
public AccountController(IIdentityParser<ApplicationUser> identityParser)
{
public AccountController(IIdentityParser<ApplicationUser> identityParser) =>
_identityParser = identityParser;
}
public ActionResult Index()
{
return View();
}
public ActionResult Index() => View();
[Authorize]
public IActionResult SignIn(string returnUrl)
{

View File

@ -14,10 +14,8 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers
{
private ICatalogService _catalogSvc;
public CatalogController(ICatalogService catalogSvc)
{
public CatalogController(ICatalogService catalogSvc) =>
_catalogSvc = catalogSvc;
}
public async Task<IActionResult> Index(int? BrandFilterApplied, int? TypesFilterApplied, int? page)
{
@ -35,7 +33,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers
ActualPage = page ?? 0,
ItemsPerPage = catalog.Data.Count,
TotalItems = catalog.Count,
TotalPages = int.Parse(Math.Ceiling(((decimal)catalog.Count / itemsPage)).ToString())
TotalPages = (int)Math.Ceiling(((decimal)catalog.Count / itemsPage))
}
};
@ -45,10 +43,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Controllers
return View(vm);
}
public IActionResult Error()
{
return View();
}
public IActionResult Error() => View();
}
}

View File

@ -11,20 +11,14 @@ namespace Microsoft.eShopOnContainers.WebMVC.Extensions
{
public static class HttpClientExtensions
{
public static void SetBasicAuthentication(this HttpClient client, string userName, string password)
{
public static void SetBasicAuthentication(this HttpClient client, string userName, string password) =>
client.DefaultRequestHeaders.Authorization = new BasicAuthenticationHeaderValue(userName, password);
}
public static void SetToken(this HttpClient client, string scheme, string token)
{
public static void SetToken(this HttpClient client, string scheme, string token) =>
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(scheme, token);
}
public static void SetBearerToken(this HttpClient client, string token)
{
public static void SetBearerToken(this HttpClient client, string token) =>
client.SetToken(JwtConstants.TokenType, token);
}
}
public class BasicAuthenticationHeaderValue : AuthenticationHeaderValue

View File

@ -8,10 +8,8 @@ using System.Threading.Tasks;
public static class SessionExtensions
{
public static void SetObject(this ISession session, string key, object value)
{
public static void SetObject(this ISession session, string key, object value) =>
session.SetString(key, JsonConvert.SerializeObject(value));
}
public static T GetObject<T>(this ISession session, string key)
{

View File

@ -33,16 +33,14 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
_apiClient.Inst.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token);
var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id.ToString()}";
var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id}";
var dataString = await _apiClient.GetStringAsync(basketUrl);
var response = JsonConvert.DeserializeObject<Basket>(dataString);
if (response == null)
{
response = new Basket()
// Use the ?? Null conditional operator to simplify the initialization of response
var response = JsonConvert.DeserializeObject<Basket>(dataString) ??
new Basket()
{
BuyerId = user.Id
};
}
return response;
}
@ -67,9 +65,12 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
basket.Items.ForEach(x =>
{
var quantity = quantities.Where(y => y.Key == x.Id).FirstOrDefault();
if (quantities.Where(y => y.Key == x.Id).Count() > 0)
x.Quantity = quantity.Value;
// Simplify this logic by using the
// new out variable initializer.
if (quantities.TryGetValue(x.Id, out var quantity))
{
x.Quantity = quantity;
}
});
return basket;
@ -119,7 +120,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
var token = await context.Authentication.GetTokenAsync("access_token");
_apiClient.Inst.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token);
var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id.ToString()}";
var basketUrl = $"{_remoteServiceBaseUrl}/{user.Id}";
var response = await _apiClient.DeleteAsync(basketUrl);
//CCE: response status code...

View File

@ -61,8 +61,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
JArray brands = JArray.Parse(dataString);
foreach (JObject brand in brands.Children<JObject>())
{
dynamic item = brand;
items.Add(new SelectListItem() { Value = item.id, Text = item.brand });
items.Add(new SelectListItem()
{
Value = brand.Value<string>("id"),
Text = brand.Value<string>("brand")
});
}
return items;
@ -79,10 +82,12 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
JArray brands = JArray.Parse(dataString);
foreach (JObject brand in brands.Children<JObject>())
{
dynamic item = brand;
items.Add(new SelectListItem() { Value = item.id, Text = item.type });
items.Add(new SelectListItem()
{
Value = brand.Value<string>("id"),
Text = brand.Value<string>("type")
});
}
return items;
}
}

View File

@ -12,26 +12,31 @@ namespace Microsoft.eShopOnContainers.WebMVC.Services
{
public ApplicationUser Parse(IPrincipal principal)
{
var user = new ApplicationUser();
var claims = (ClaimsPrincipal)principal;
// Pattern matching 'is' expression
// assigns "claims" if "principal" is a "ClaimsPrincipal"
if (principal is ClaimsPrincipal claims)
{
return new ApplicationUser
{
user.CardHolderName = (claims.Claims.Where(x => x.Type == "card_holder").Count() > 0) ? claims.Claims.First(x => x.Type == "card_holder").Value : "";
user.CardNumber = (claims.Claims.Where(x => x.Type == "card_number").Count() > 0) ? claims.Claims.First(x => x.Type == "card_number").Value : "";
user.Expiration = (claims.Claims.Where(x => x.Type == "card_expiration").Count() > 0) ? claims.Claims.First(x => x.Type == "card_expiration").Value : "";
user.CardType = (claims.Claims.Where(x => x.Type == "missing").Count() > 0) ? int.Parse(claims.Claims.First(x => x.Type == "missing").Value) : 0;
user.City = (claims.Claims.Where(x => x.Type == "address_city").Count() > 0) ? claims.Claims.First(x => x.Type == "address_city").Value : "";
user.Country = (claims.Claims.Where(x => x.Type == "address_country").Count() > 0) ? claims.Claims.First(x => x.Type == "address_country").Value : "";
user.Email = (claims.Claims.Where(x => x.Type == "email").Count() > 0) ? claims.Claims.First(x => x.Type == "email").Value : "";
user.Id = (claims.Claims.Where(x => x.Type == "sub").Count() > 0) ? claims.Claims.First(x => x.Type == "sub").Value : "";
user.LastName = (claims.Claims.Where(x => x.Type == "last_name").Count() > 0) ? claims.Claims.First(x => x.Type == "last_name").Value : "";
user.Name = (claims.Claims.Where(x => x.Type == "name").Count() > 0) ? claims.Claims.First(x => x.Type == "name").Value : "";
user.PhoneNumber = (claims.Claims.Where(x => x.Type == "phone_number").Count() > 0) ? claims.Claims.First(x => x.Type == "phone_number").Value : "";
user.SecurityNumber = (claims.Claims.Where(x => x.Type == "card_security_number").Count() > 0) ? claims.Claims.First(x => x.Type == "card_security_number").Value : "";
user.State = (claims.Claims.Where(x => x.Type == "address_state").Count() > 0) ? claims.Claims.First(x => x.Type == "address_state").Value : "";
user.Street = (claims.Claims.Where(x => x.Type == "address_street").Count() > 0) ? claims.Claims.First(x => x.Type == "address_street").Value : "";
user.ZipCode = (claims.Claims.Where(x => x.Type == "address_zip_code").Count() > 0) ? claims.Claims.First(x => x.Type == "address_zip_code").Value : "";
return user;
CardHolderName = claims.Claims.FirstOrDefault(x => x.Type == "card_holder")?.Value ?? "",
CardNumber = claims.Claims.FirstOrDefault(x => x.Type == "card_number")?.Value ?? "",
Expiration = claims.Claims.FirstOrDefault(x => x.Type == "card_expiration")?.Value ?? "",
CardType = int.Parse(claims.Claims.FirstOrDefault(x => x.Type == "missing")?.Value ?? "0"),
City = claims.Claims.FirstOrDefault(x => x.Type == "address_city")?.Value ?? "",
Country = claims.Claims.FirstOrDefault(x => x.Type == "address_country")?.Value ?? "",
Email = claims.Claims.FirstOrDefault(x => x.Type == "email")?.Value ?? "",
Id = claims.Claims.FirstOrDefault(x => x.Type == "sub")?.Value ?? "",
LastName = claims.Claims.FirstOrDefault(x => x.Type == "last_name")?.Value ?? "",
Name = claims.Claims.FirstOrDefault(x => x.Type == "name")?.Value ?? "",
PhoneNumber = claims.Claims.FirstOrDefault(x => x.Type == "phone_number")?.Value ?? "",
SecurityNumber = claims.Claims.FirstOrDefault(x => x.Type == "card_security_number")?.Value ?? "",
State = claims.Claims.FirstOrDefault(x => x.Type == "address_state")?.Value ?? "",
Street = claims.Claims.FirstOrDefault(x => x.Type == "address_street")?.Value ?? "",
ZipCode = claims.Claims.FirstOrDefault(x => x.Type == "address_zip_code")?.Value ?? ""
};
}
throw new ArgumentException(message: "The principal must be a ClaimsPrincipal", paramName: nameof(principal));
}
}
}

View File

@ -17,29 +17,29 @@ namespace WebMVC.Services.Utilities
_logger = new LoggerFactory().CreateLogger(nameof(HttpApiClientWrapper));
}
public async Task<string> GetStringAsync(string uri)
{
return await HttpInvoker(async () =>
await _client.GetStringAsync(uri));
}
// Notice that these (and other methods below) are Task
// returning asynchronous methods. But, they do not
// have the 'async' modifier, and do not contain
// any 'await statements. In each of these methods,
// the only asynchronous call is the last (or only)
// statement of the method. In those instances,
// a Task returning method that does not use the
// async modifier is preferred. The compiler generates
// synchronous code for this method, but returns the
// task from the underlying asynchronous method. The
// generated code does not contain the state machine
// generated for asynchronous methods.
public Task<string> GetStringAsync(string uri) =>
_client.GetStringAsync(uri);
public async Task<HttpResponseMessage> PostAsync<T>(string uri, T item)
public Task<HttpResponseMessage> PostAsync<T>(string uri, T item)
{
var contentString = new StringContent(JsonConvert.SerializeObject(item), System.Text.Encoding.UTF8, "application/json");
return await HttpInvoker(async () =>
await _client.PostAsync(uri, contentString));
return _client.PostAsync(uri, contentString);
}
public async Task<HttpResponseMessage> DeleteAsync(string uri)
{
return await HttpInvoker(async () =>
await _client.DeleteAsync(uri));
}
private async Task<T> HttpInvoker<T>(Func<Task<T>> action)
{
return await action();
}
public Task<HttpResponseMessage> DeleteAsync(string uri) =>
_client.DeleteAsync(uri);
}
}

View File

@ -24,76 +24,74 @@ namespace WebMVC.Services.Utilities
CreateRetryPolicy(),
CreateCircuitBreakerPolicy()
);
}
private Policy CreateCircuitBreakerPolicy()
{
return Policy
.Handle<HttpRequestException>()
.CircuitBreakerAsync(
// number of exceptions before breaking circuit
3,
// time circuit opened before retry
TimeSpan.FromMinutes(1),
(exception, duration) => {
// on circuit opened
_logger.LogTrace("Circuit breaker opened");
},
() => {
// on circuit closed
_logger.LogTrace("Circuit breaker reset");
}
);
}
private Policy CreateRetryPolicy()
{
return Policy
.Handle<HttpRequestException>()
.WaitAndRetryAsync(
// number of retries
3,
// exponential backofff
retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)),
// on retry
(exception, timeSpan, retryCount, context) =>
{
_logger.LogTrace($"Retry {retryCount} " +
$"of {context.PolicyKey} " +
$"at {context.ExecutionKey}, " +
$"due to: {exception}.");
});
}
private Policy CreateCircuitBreakerPolicy() =>
Policy.Handle<HttpRequestException>()
.CircuitBreakerAsync(
// number of exceptions before breaking circuit
3,
// time circuit opened before retry
TimeSpan.FromMinutes(1),
(exception, duration) =>
{
// on circuit opened
_logger.LogTrace("Circuit breaker opened");
},
() =>
{
// on circuit closed
_logger.LogTrace("Circuit breaker reset");
}
);
public async Task<string> GetStringAsync(string uri)
{
return await HttpInvoker(async () =>
await _client.GetStringAsync(uri));
}
private Policy CreateRetryPolicy() =>
Policy.Handle<HttpRequestException>()
.WaitAndRetryAsync(
// number of retries
3,
// exponential backofff
retryAttempt => TimeSpan.FromSeconds(Math.Pow(2, retryAttempt)),
// on retry
(exception, timeSpan, retryCount, context) =>
{
_logger.LogTrace($"Retry {retryCount} " +
$"of {context.PolicyKey} " +
$"at {context.ExecutionKey}, " +
$"due to: {exception}.");
}
);
public async Task<HttpResponseMessage> PostAsync<T>(string uri, T item)
{
// Notice that these (and other methods below) are Task
// returning asynchronous methods. But, they do not
// have the 'async' modifier, and do not contain
// any 'await statements. In each of these methods,
// the only asynchronous call is the last (or only)
// statement of the method. In those instances,
// a Task returning method that does not use the
// async modifier is preferred. The compiler generates
// synchronous code for this method, but returns the
// task from the underlying asynchronous method. The
// generated code does not contain the state machine
// generated for asynchronous methods.
public Task<string> GetStringAsync(string uri) =>
HttpInvoker(() => _client.GetStringAsync(uri));
public Task<HttpResponseMessage> PostAsync<T>(string uri, T item) =>
// a new StringContent must be created for each retry
// as it is disposed after each call
return await HttpInvoker(async () =>
await _client.PostAsync(uri,
new StringContent(JsonConvert.SerializeObject(item),
System.Text.Encoding.UTF8, "application/json")));
}
HttpInvoker(() =>_client.PostAsync(uri,
new StringContent(JsonConvert.SerializeObject(item),
System.Text.Encoding.UTF8, "application/json")));
public async Task<HttpResponseMessage> DeleteAsync(string uri)
{
return await HttpInvoker(async () =>
await _client.DeleteAsync(uri));
}
public Task<HttpResponseMessage> DeleteAsync(string uri) =>
HttpInvoker(() => _client.DeleteAsync(uri));
private async Task<T> HttpInvoker<T>(Func<Task<T>> action)
{
private Task<T> HttpInvoker<T>(Func<Task<T>> action) =>
// Executes the action applying all
// the policies defined in the wrapper
return await _policyWrapper
.ExecuteAsync(async () => await action());
}
_policyWrapper.ExecuteAsync(() => action());
}
}

View File

@ -13,10 +13,7 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewComponents
{
private readonly IBasketService _cartSvc;
public Cart(IBasketService cartSvc)
{
_cartSvc = cartSvc;
}
public Cart(IBasketService cartSvc) => _cartSvc = cartSvc;
public async Task<IViewComponentResult> InvokeAsync(ApplicationUser user)
{

View File

@ -12,19 +12,29 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewComponents
{
private readonly IBasketService _cartSvc;
public CartList(IBasketService cartSvc)
{
_cartSvc = cartSvc;
}
public CartList(IBasketService cartSvc) => _cartSvc = cartSvc;
public async Task<IViewComponentResult> InvokeAsync(ApplicationUser user)
{
var item = await GetItemsAsync(user);
return View(item);
}
private async Task<Basket> GetItemsAsync(ApplicationUser user)
{
return await _cartSvc.GetBasket(user);
}
// Notice that this method is a Task
// returning asynchronous method. But, it does not
// have the 'async' modifier, and does not contain
// any 'await statements.
// The only asynchronous call is the last (or only)
// statement of the method. In those instances,
// a Task returning method that does not use the
// async modifier is preferred. The compiler generates
// synchronous code for this method, but returns the
// task from the underlying asynchronous method. The
// generated code does not contain the state machine
// generated for asynchronous methods.
// Contrast that with the method above, which calls
// and awaits an asynchronous method, and then processes
// it further.
private Task<Basket> GetItemsAsync(ApplicationUser user) => _cartSvc.GetBasket(user);
}
}

View File

@ -14,11 +14,20 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels.Annotations
if (value == null)
return false;
var month = value.ToString().Split('/')[0];
var year = $"20{value.ToString().Split('/')[1]}";
DateTime d = new DateTime(int.Parse(year), int.Parse(month), 1);
var monthString = value.ToString().Split('/')[0];
var yearString = $"20{value.ToString().Split('/')[1]}";
// Use the 'out' variable initializer to simplify
// the logic of validating the expiration date
if ((int.TryParse(monthString, out var month)) &&
(int.TryParse(yearString, out var year)))
{
DateTime d = new DateTime(year, month, 1);
return d > DateTime.UtcNow;
return d > DateTime.UtcNow;
} else
{
return false;
}
}
}
}

View File

@ -7,11 +7,11 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels
{
public class Basket
{
public Basket()
{
Items = new List<BasketItem>();
}
public List<BasketItem> Items { get; set; }
// Use property initializer syntax.
// While this is often more useful for read only
// auto implemented properties, it can simplify logic
// for read/write properties.
public List<BasketItem> Items { get; set; } = new List<BasketItem>();
public string BuyerId { get; set; }
public decimal Total()

View File

@ -9,6 +9,6 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels.CartViewModels
public class CartComponentViewModel
{
public int ItemsCount { get; set; }
public string Disabled { get { return (ItemsCount == 0) ? "is-disabled" : ""; } }
public string Disabled => (ItemsCount == 0) ? "is-disabled" : "";
}
}

View File

@ -11,10 +11,6 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels
{
public class Order
{
public Order() {
OrderItems = new List<OrderItem>();
}
public string OrderNumber {get;set;}
public DateTime Date {get;set;}
@ -53,7 +49,10 @@ namespace Microsoft.eShopOnContainers.WebMVC.ViewModels
public string Buyer { get; set; }
public List<OrderItem> OrderItems { get; }
// See the property initializer syntax below. This
// initializes the compiler generated field for this
// auto-implemented property.
public List<OrderItem> OrderItems { get; } = new List<OrderItem>();
[Required]
public Guid RequestId { get; set; }