Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here is the snippet of my code:

/Views/Emp/EditEmp.aspx

<% if(HttpContext.Current.User.IsInRole("Manager")) { %>
    <div class="small right" style="margin:5px 0 10px 0;">
        <a class="btn" href="/Emp/Edit"><i class="icon-double-angle-left"></i>Back to Emp Info</a>
    </div>
<% } %>

<div>. ..... .. .. ...</div>

<script type="text/javascript">
$(document).ready(function () {
    EmpWorkingDays = [<%= Model.SundayWorkingTime %>, <%= Model.MondayWorkingTime %>, <%= Model.TuesdayWorkingTime %>, <%= Model.WednesdayWorkingTime %>, <%= Model.ThursdayWorkingTime %>, <%= Model.FridayWorkingTime %>, <%= Model.SaturdayWorkingTime %>];
    <% if(Model != null && Model.Holidays != null){
        foreach(EmpHoliday eh in Model.Holidays.ToList()) { %>
            Holidays.push({date:new Date("<%: eh.Date.Year %>", "<%: eh.Date.Month - 1 %>", "<%: eh.Date.Day %>",0,0,0,0).getTime(), holiday:"<%: eh.Holiday %>"});
        <% }
    } %>
});
</script>


<% if(Model != null && Model.Holidays != null) { 
    foreach(EmpHoliday eh in Model.Holidays){ %>
        <div class="editor-label"><%= eh.Date.ToString("dd/MMM/yyyy") %></div>
        <div class="editor-field"><%= eh.Holiday %></div>
    <% } %>
    <div class="clear"></div>
<% } %>

Is it okay to use server side variables like this?

I was a PHP developer and I started writing a product in C# without studying the basic structure. Now, I have around 100+ views with similar code. After 1 year into development, I now realize that something is not right. How can I separate the server tags when they are so intertwined in the code?

PS: I am not using Razor in my product.

share|improve this question
add comment

2 Answers

This is how I see server variables and local variables interacting. There is not really any other way to get the values between the two that I'm aware of.

As for the code, I one minor point:change Emp to Employee for variable names. This will eliminate confusion in the future from you, or anybody else reading the code (what does Emp mean?)

Other than that, this is really easy code to read. Keep it up.

share|improve this answer
    
+1 for variable names –  dreza Mar 6 at 19:55
add comment

Yes, that is how I would typically do it.

As an alternative concept which I also like to consider at times is using JavascriptSerializer to convert a list to an array.

It would go something like:

<%
Model
.Holidays
.ToList()
.Select(eh => new {
   date: new Date(eh.Date.Year, eh.Date.Month - 1,eh.Date.Day,0,0,0,0),
   holiday: eh.Holiday
});

var serializer = new System.Web.Script.Serialization.JavaScriptSerializer();
%>

var Holidays = <%= serializer.Serialize(array) %>;

Following this line of thought you could create a method on your Model to accomplish the same thing for the working days variable. i.e.

<%
class MyModel {

  public IEnumerable<WorkingDays> GetWorkingDays() {
     yield return Model.SundayWorkingTime;
     yield return Model.MondayWorkingTime;
    // etc
  }
}

EmpWorkingDays = <%= serializer.Serialize(Model.GetWorkingDays())  %>;
%>

Using existing MVC helper methods

When creating links in the view I always like to use the built in MVC helper methods such as Url.Content or Url.Action. This means that if the site is installed in a sub directory the correct link will be created automatically for you. i.e. mysite.com/subdir/Emp/Edit.

If you currently deployed the solution you have your links would break with a 404 Not found.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.