The Stack Overflow podcast is back! Listen to an interview with our new CEO.

Hot answers tagged

11

I would change your method to use generics (so that the return value is meaningful) and not call .ToString() internally. This will enable you to use it in more scenarios. public static T GetValueTernary<T>(this HtmlHelper html, object a, object b, T valueIfEqual, T valueIfNotEqual) { return object.Equals(a, b) ? valueIfEqual : valueIfNotEqual; } ...


10

Your code will throw an exception when a is null. A safe way to compare values is the static version of object.Equals(): object.Equals(a, b)


8

In my opinion, your view is doing too much work. This is happening because your model is missing a property. If you add an IsSelected property to your model, I think we can simplify the view a bit. public class DeliveryAddress() { public int ID { get; set; } public string Location { get; set; } public bool IsSelected { get; set; } } Now this ...


6

It's a bad idea to modify the session state from inside a view or a partial view. Instead, I would create an alert view model public class AlertViewModel { public AlertType AlertType { get; set; } public string Message { get; set; } } public enum AlertType { None, Success, Error } and use that from inside the view/partial view, for ...


6

You don't need to enumerate twice here var temp = _context.Users.AsEnumerable().ToList(); You can just call .ToList(). SingleOrDefault() also accepts predicates so the first .Where() call is unnecessary, you can just do it like this : Your code : var temp = _context.Users.Where(x => x.id == id).SingleOrDefault(); Can become : var temp = ...


6

As you have already suspected, your code is trying to do too much. Consider separating functionality out into their own distinct concerns. (SRP/SoC -Single Responsibility Principle / Separation of Concerns) Your abstract Notification acts more like a base model and should be refactor to reflect that public abstract class Notification { public string ...


5

With the danger of not understanding your question entirely as it stands now, you have MyEntitiesViewModel with a enabled field. Why don't you use to trigger a CSS class for the entire entity which disables that entity? Why do you reiterate over the attributes, and create loads of helpers which I don't understand why you need. I'm not entirely sure about ...


5

ASP.NET MVC will produce a controller instance for each request, so you need a bit of help from the framework to assist you with caching - because anything you cache at controller level will only live for a single request's lifetime. As always, Stack Overflow has answers for you :) // GET: Surveys // GET: Surveys/Details?name=SomeSurveyName These comments ...


5

Semantically, HTML button tags are supposed to represent a user's action, whereas the link tags are supposed to take care of the navigation between pages. That said, as @user1320170 pointed out in comments, your tag should be a link (< a > tag.) You'll notice it's much clearer this way. Apart from that, your code is good, using @Url.Action("Index", "...


5

This is not an ideal way to do things. The model passed to your view should only have what you need for the view. There are a few different things going on here. One is searching for the information and two is displaying it in your view. The solution will depend on the context in which it is happening. One option is that you can change your service to be ...


4

I'm not sure if this is any better and it might depend on how many if statements or different categories you have whether it's worth it. But an alternative could be something like: var categoryLabelValues = new Dictionary<string, string>() { {"11", "Branche"}, {"22", "Udviklingsstadie"} }; foreach(var interest in InterstList ...


4

I would use a Partial View for this code. Anytime I have a menu for a page I put it in a partial view even though it may not be used on another page. <tr>@Html.Partial("_Actions")</tr> <tr>@Html.Partial("_OrderStatus")</tr> <tr>@Html.Partial("_OrderDetails")</tr> <tr>@Html.Partial("_OrderDate")</tr> inside ...


4

Basically, 95% of your code is the same between the two sections, so I'd prefer to use just a single instance with a bool on the page. @{ var error = ViewModel.ModelState["StorateError"]; var hasErrors = error != null && error.Errors.Any(); } <div class="a" title="@( hasErrors ? error.Errors[0].ErrorMessage : string.Empty" ) data-...


4

Talking about public async Task<ActionResult> Index() While looking at this while (pagedCollection != null) { var usersList = pagedCollection.CurrentPage.ToList(); userList.AddRange(usersList.Cast<Microsoft.Azure.ActiveDirectory.GraphClient.User>()); pagedCollection = await pagedCollection.GetNextPageAsync(); } I thought what ...


4

I would do this a little differently... /// <summary> /// Uses <see cref="EqualityComparer{TValue}"/> to determine if <paramref name="left"/> is /// equal to <paramref name="right"/>. If it is, <paramref name="returnIfEqual"/> is returned, /// otherwise, <paramref name="returnIfNotEqual" /> is returned. /// </summary&...


4

You might want to call your variable temp something like productToRemove Single / SingleOrDefault both have an overload which accepts predicate you can get rid of the .Where() call with a single Single call: var productToRemove = _context.Auction.SingleOrDefault(x => x.Id == id); You are using SingleOrDefault but you are 100% sure that it will find a ...


3

I'm new to MVC (relatively), but I don't think I'll ever plan on doing too much in a view. In my understanding, it's merely a template for an object (view model) you throw at it. There shouldn't be much server-side functionality in there at all. Getting data from a database and selecting a SelectList from this collection is too much. There isn't enough ...


3

This looks like a typo to me: <dd> <a href="@Html.DisplayFor(model => model.Url)">@Html.DisplayFor(model => model.Url)</a> </dd> Shouldn't it just be: <dd> <a href="@Model.Url">@Html.DisplayFor(model => model.Url)</a> </dd> One thing I find really useful when dealing with projecting to model ...


3

Having a chain of if...elseif s it is a good place to start having a registry/resolver container instead. Please take a look at my answer here. After refactoring the code and removing the nested if....else s you will have all the logic and actions in one place to look at and you can refactor them in order to be DRY.


3

Couple of items that might need a re-look: The now deprecated AzMan has very similar concepts. The conceptual equivalent for the 'Feature' in your application is an 'Operation' in AzMan. I'd suggest taking a look at it to understand how an application can use role based access to get the granularity that is desired. A Custom Role Provider can help you ...


3

You could just use <td>@user.created_at.ToString("MMM dd yyyy")</td> instead of <td>@user.created_at.ToString("MMM") @user.created_at.ToString("dd") @user.created_at.ToString("yyyy")</td>


3

Is this secure? No. It isn't secure because you can change anyone's email address on the last step. To do so, all I need to do is change the emailId stored in the hidden field (for example, using my browser tools). In the final step you must verify the token. You don't need step 2 at all. There are two requests you need once you have your reset token. ...


2

None. However, it has several issues. Here it is, indented differently to avoid horizontal scroll: <div data-role="fieldcontain" class="inline-toggle"> <label for="IsAuto">Auto <a href="#"> <img class="smallInfo" src="/sites/dev-demo/mobile/assets/images/info@2x.png"> </a>: </label> ...


2

By calling $(".RequestTrigger").on("click", ...); you are creating a jQuery collection of all elements matching that selector, then attaching click handlers directly to those elements. Instead, you can use event delegation: $(document).on("click", ".RequestTrigger", function () { // ... }); If you dynamically add new .RequestTrigger elements to the ...


2

I would suggest to: I would recommend to use camelCase naming style, so instead of ApprovalTable you could use approvalTable you are using a function to define a handler for an event, the name is not very consistent with what this function does, you could rename it to enableRequestValidationHandler It's often a good idea to cache selectors, if you plan to ...


2

I would use the razor boolean attributes: <td> <input type="checkbox" id="canRead_@item.SecureFolderID" class="CHKreader" onchange="changePermissions(@item.SecureFolderID)" checked="@(singleItem != null && singleItem.FolderRead)" /> </td> <td> <input type="checkbox" id="canWrite_@item.SecureFolderID" class="CHKwriter"...


2

Razor supports collections from model. Examples You can connect all information to the database, and maintain the data properly. Explanation: The table must be in database, in model, not in view. The correct usage of MVC: <table class="table table-bordered" id="pendingTable"> <thead> <tr> @foreach (var columnName in Model....


2

There's some duplication in the SurveysController. Instead of creating a new instance of the SurveyMonkeyApi in each action, create it in the constructor instead. namespace SurveyMonkeyPlayground.Controllers { public class SurveysController : Controller { private SurveyMonkeyApi _surveyMonkey; public SurveysController(...


2

You already have got a nice and simple solution from @GavinCoates so I only would like to add a couple of comments about your code for your future coding experience. You should consequently use either full types for your variables or the var keyword. In your code the var would greatly improve the readabilty as all types are obvious here and don't need to ...


2

Just do a string replace to swap the html breaks for commas, then trim the end to remove any extra commas: retailBreaks.Replace("<br/>",",").TrimEnd(',')


Only top voted, non community-wiki answers of a minimum length are eligible