powered by simpleCommunicator - 2.0.41     © 2025 Programmizd 02
Форумы / ASP.NET [игнор отключен] [закрыт для гостей] / Покритикуйте, пожалуйста, код.
25 сообщений из 27, страница 1 из 2
Покритикуйте, пожалуйста, код.
    #39955406
listtoview
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
DI пока прошу не учитывать.

Модель:

Код: c#
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
	public partial class RoleMapEmps
    {
        public int Id { get; set; }

        [Required(ErrorMessage ="Поле должно быть заполнено")]
        [Display(Name = "Роль")]
        public int RoleId { get; set; }

        [Required(ErrorMessage = "Поле должно быть заполнено")]
        [Display(Name = "Пользователь")]
        public string User { get; set; }
                
        [Display(Name = "Автор")]
        public string Owner { get; set; }
		
        [Display(Name = "Создано")]
        public System.DateTime Created { get; set; }

        [Display(Name = "Удалено")]
        public Nullable<System.DateTime> Deleted { get; set; }
    
        public virtual Roles Roles { get; set; }
    }
	
	public partial class Roles
    {
        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2214:DoNotCallOverridableMethodsInConstructors")]
        public Roles()
        {
            this.RoleMapEmps = new HashSet<RoleMapEmps>();
        }

        public int Id { get; set; }
        public int TypeId { get; set; }
        public Nullable<int> DomainId { get; set; }
        [Display(Name = "Роль")]
        public string Name { get; set; }
        public System.DateTime Created { get; set; }
        public Nullable<System.DateTime> Deleted { get; set; }

        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2227:CollectionPropertiesShouldBeReadOnly")]
        public virtual ICollection<RoleMapEmps> RoleMapEmps { get; set; }
    }


Контроллер:

Код: c#
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
47.
48.
49.
50.
51.
52.
53.
54.
55.
56.
57.
58.
59.
60.
61.
62.
63.
64.
65.
66.
67.
68.
69.
70.
71.
72.
73.
74.
75.
76.
77.
78.
79.
80.
81.
82.
83.
84.
85.
86.
87.
88.
89.
90.
91.
92.
93.
94.
95.
96.
97.
98.
99.
100.
101.
102.
using System;
using System.Linq;
using System.Web.Mvc;

namespace WebUI.Controllers
{
    public class RoleMapEmpsController : Controller
    {
        LapsadminContext db = new LapsadminContext();

        public ActionResult Index()
        {
            return View(db.RoleMapEmps);
        }

        [HttpGet]
        public ActionResult Create()
        {
            ViewBag.Roles = GetRolesList();
            return View();
        }


        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Create(RoleMapEmps rme, bool flagDeleted)
        {
            if (ModelState.IsValid)
            {
                var obj = new RoleMapEmps();

                obj.Owner = Models.Helpers.EmployeeHelper.GetCurrentUserLogin();
                obj.RoleId = rme.RoleId;
                obj.User = rme.User;
                obj.Created = DateTime.Now;
                obj.Deleted = !flagDeleted ? null : (DateTime?)DateTime.Now;

                db.RoleMapEmps.Add(obj);

                db.SaveChanges();
                return RedirectToAction("Index");
            }

            ViewBag.Roles = GetRolesList();

            return View(rme);
        }


        [HttpGet]
        public ActionResult Edit(int? id)
        {
            if (id == null)
            {
                return HttpNotFound();
            }
            var rme = db.RoleMapEmps.Find(id);
            if (rme != null)
            {
                ViewBag.Roles = GetRolesList();

                return View(rme);
            }
            return HttpNotFound();
        }


        [HttpPost]
        [ValidateAntiForgeryToken]
        public ActionResult Edit(RoleMapEmps rme, bool flagDeleted)
        {
            if (ModelState.IsValid)
            {
                var obj = db.RoleMapEmps.Find(rme.Id);

                obj.Owner = Models.Helpers.EmployeeHelper.GetCurrentUserLogin();
                obj.RoleId = rme.RoleId;
                obj.User = rme.User;
                obj.Deleted = !flagDeleted ? null : (DateTime?)DateTime.Now;

                db.SaveChanges();
                return RedirectToAction("Index");
            }

            ViewBag.Roles = GetRolesList();

            return View(rme);
        }


        protected SelectList GetRolesList()
        {
            return new SelectList(db.Roles.Where(r => r.Deleted == null).Select(r => new { Id = r.Id, Name = r.Name }).OrderBy(r => r.Name).ToList(), "Id", "Name");
        }

        protected override void Dispose(bool disposing)
        {
            db.Dispose();
            base.Dispose(disposing);
        }
    }
}


Index.cshtml:

Код: plaintext
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
47.
48.
49.
50.
@model IEnumerable<WebUI.RoleMapEmps>

<p>
    @Html.ActionLink("Create New", "Create")
</p>
<table class="table">
    <tr>
        <th>
            @Html.DisplayNameFor(model => model.Roles.Name)
        </th>
        <th>
            @Html.DisplayNameFor(model => model.User)
        </th>
        <th>
            @Html.DisplayNameFor(model => model.Owner)
        </th>
        <th>
            @Html.DisplayNameFor(model => model.Created)
        </th>
        <th>
            @Html.DisplayNameFor(model => model.Deleted)
        </th>
        <th></th>
    </tr>

    @foreach (var item in Model)
    {
        <tr>
            <td>
                @Html.DisplayFor(modelItem => item.Roles.Name)
            </td>
            <td>
                @Html.DisplayFor(modelItem => item.User)
            </td>
            <td>
                @Html.DisplayFor(modelItem => item.Owner)
            </td>
            <td>
                @Html.DisplayFor(modelItem => item.Created)
            </td>
            <td>
                @Html.DisplayFor(modelItem => item.Deleted)
            </td>
            <td>
                @Html.ActionLink("Edit", "Edit", new { id = item.Id })                
            </td>
        </tr>
    }

</table>


Create.cshtml:

Код: html
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
@model WebUI.RoleMapEmps

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken()

    <div class="form-horizontal">
        <h4>RoleMapEmps</h4>
        <hr />
        @Html.ValidationSummary(true, "", new { @class = "text-danger" })

        <div class="form-group">
            @Html.LabelFor(model => model.RoleId, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.DropDownListFor(model => model.RoleId, ViewBag.Roles as SelectList, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.RoleId, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            @Html.LabelFor(model => model.User, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.EditorFor(model => model.User, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.User, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            @Html.Label("flagDeleted", "Удалено", htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.CheckBox("flagDeleted", Model == null || Model.Deleted == null ? false : true, htmlAttributes: new { @class = "control-label" })
                @Html.ValidationMessageFor(model => model.Deleted, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            <div class="col-md-offset-2 col-md-10">
                <input type="submit" value="Save" class="btn btn-default" />
            </div>
        </div>
    </div>
}

<div>
    @Html.ActionLink("Back to List", "Index")
</div>


Edit.cshtml:

Код: html
1.
2.
3.
4.
5.
6.
7.
8.
9.
10.
11.
12.
13.
14.
15.
16.
17.
18.
19.
20.
21.
22.
23.
24.
25.
26.
27.
28.
29.
30.
31.
32.
33.
34.
35.
36.
37.
38.
39.
40.
41.
42.
43.
44.
45.
46.
47.
@model WebUI.RoleMapEmps

@using (Html.BeginForm())
{
    @Html.AntiForgeryToken()

    <div class="form-horizontal">
        <h4>RoleMapEmps</h4>
        <hr />
        @Html.ValidationSummary(true, "", new { @class = "text-danger" })
        @Html.HiddenFor(model => model.Id)
        
        <div class="form-group">
            @Html.LabelFor(model => model.RoleId, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.DropDownListFor(model => model.RoleId, ViewBag.Roles as SelectList, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.RoleId, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            @Html.LabelFor(model => model.User, htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.EditorFor(model => model.User, new { htmlAttributes = new { @class = "form-control" } })
                @Html.ValidationMessageFor(model => model.User, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            @Html.Label("flagDeleted", "Удалено", htmlAttributes: new { @class = "control-label col-md-2" })
            <div class="col-md-10">
                @Html.CheckBox("flagDeleted", Model == null || Model.Deleted == null ? false : true, htmlAttributes: new { @class = "control-label" })
                @Html.ValidationMessageFor(model => model.Deleted, "", new { @class = "text-danger" })
            </div>
        </div>

        <div class="form-group">
            <div class="col-md-offset-2 col-md-10">
                <input type="submit" value="Save" class="btn btn-default" />
            </div>
        </div>
    </div>
}

<div>
    @Html.ActionLink("Back to List", "Index")
</div>


Что меня смущает:
1) наличие ViewBag.Roles = GetRolesList();
2) наличие flagDeleted, из-за неполучается сделать например db.Entry(rme).State = EntityState.Modified; при редактировании.
3) поле Owner не помечено атрибутом Required но всегда заполняется. Наверное нужно устанавливать его на гет версиях, тогда валидация модели пройдет при сохранении.
4) Create.cshtml и Edit.cshtml очень похожи. Разница только отсутствия Id на форме создания. Может стоит оставить одну страницу Edit.cshtml, пусть id там нулем инициализируется?

как правильно поправить эти проблемы? спасибо
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955458
Дмитрий Мух
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview,

после первой строки уже скучно: public partial class RoleMapEmps...

"Роль, карта, апчхи" - так это выглядит для меня :)
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955467
Дмитрий Мух
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
в дотнете я не джун, 9 лет опыта
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955504
Фотография hVostt
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
как правильно поправить эти проблемы? спасибо


Видимо, перестать использовать Entity-классы в качестве вью-моделей.

Это касается всех пунктов.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955515
fkthat
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Я уже что-то конкретно подзабыл MVC но, ЕМНИП, нет смысла писать Edit(int? id) и тут же на пустой id возвращать 404. Он и так на пустом id вернет 404 - роут нужный не найдет просто.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955617
Фотография hVostt
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
fkthat
Я уже что-то конкретно подзабыл MVC но, ЕМНИП, нет смысла писать Edit(int? id) и тут же на пустой id возвращать 404. Он и так на пустом id вернет 404 - роут нужный не найдет просто.


Да, конструкция Edit(int? id) максимально глупая. Так делать не нужно, конечно.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955762
listtoview
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Дмитрий Мух
listtoview,

после первой строки уже скучно: public partial class RoleMapEmps...

"Роль, карта, апчхи" - так это выглядит для меня :)

EF так генерит, а что не так?
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955764
listtoview
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
hVostt
listtoview
как правильно поправить эти проблемы? спасибо


Видимо, перестать использовать Entity-классы в качестве вью-моделей.

Это касается всех пунктов.

да, спасибо за совет. вообщем то это решает все 3 пункта
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955769
Дмитрий Мух
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
Дмитрий Мух
listtoview,

после первой строки уже скучно: public partial class RoleMapEmps...

"Роль, карта, апчхи" - так это выглядит для меня :)

EF так генерит, а что не так?

В программировании существует лишь два характерных затруднения: инвалидация кеша и наименование сущностей :)
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955816
17-77
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview,

модель:
1. решительно непонятно, что за RoleMapEmps, если это типа таблицы соответствия ролей и пользователей, то почему там user string? это что логин? а если юзер логин поменяет, то все сломается?
2. с Owner аналогично
3. Created / Deleted повторяются, либо делать отдельную таблицу, где хранить когда что и кем было сделано или хотя бы наследоваться от общей модели с этими полями
4. почему нет даты обновления? обычно делают дату создания, дату последнего обновления, и если есть софт-делит - тогда флаг IsDeleted. тогда если он true, то дата обновления является датой удаления. а сейчас выходит - дата удаления есть, но запись будет удалена физически, смысл тогда в этом поле?
5. s на конце названия модели, выходит что virtual Roles Roles { get; set; } содержит Рол и , а она там одна, wtf?

контроллер:
1. bool flagDeleted - когда в модели будет нормальный флаг IsDeleted - все встанет на свои места
2. ViewBag.Roles = GetRolesList(); - это зависит от сложности и предпочтений, можно сделать ViewModel<TModel> и сложить туда все сопутствующие данные
3. и да, лучше ентити отдельно, модели отдельно

вьюхи:
1. Create.cshtml и Edit.cshtml очень похожи - зависит от, сейчас объединишь, а потом придется разъединять обратно
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955831
love_bach
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Дмитрий Мух
listtoview,

после первой строки уже скучно: public partial class RoleMapEmps...

"Роль, карта, апчхи" - так это выглядит для меня :)


Что такое "апчхи"?
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955832
love_bach
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
Дмитрий Мух
listtoview
пропущено...

EF так генерит, а что не так?

В программировании существует лишь два характерных затруднения: инвалидация кеша и наименование сущностей :)

+, какой-то уважаемый перец сформулировал
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955833
love_bach
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
hVostt
пропущено...


Видимо, перестать использовать Entity-классы в качестве вью-моделей.

Это касается всех пунктов.

да, спасибо за совет. вообщем то это решает все 3 пункта


нет, это решает только "перестать использовать Entity-классы в качестве вью-моделей"
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955834
love_bach
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
а вообщето, этот товарищ listtoview, просто пытается разжечь срачь. он давний мембер, и так развлекается
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955887
listtoview
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
love_bach
а вообщето, этот товарищ listtoview, просто пытается разжечь срачь. он давний мембер, и так развлекается

скажем так, я понимал что нужно использовать модель представления а не доменную модель для решения этой задачи
просто задача простейшая и обычно обходятся доменной моделью сгенерированной какой ниб ОРМ
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39955916
ViPRos
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview,

для домена "представление", "модель представления" является "доменной моделью".
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956029
Фотография hVostt
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
love_bach
а вообщето, этот товарищ listtoview, просто пытается разжечь срачь. он давний мембер, и так развлекается

скажем так, я понимал что нужно использовать модель представления а не доменную модель для решения этой задачи
просто задача простейшая и обычно обходятся доменной моделью сгенерированной какой ниб ОРМ


Это обманчивое впечателние, проще не будет.
Используйте вью-модели.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956068
Фотография crutchmaster
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview obj.RoleId = rme.RoleId;
obj.User = rme.User;

Если там будет 50 полей?
obj.Deleted = !flagDeleted ? null : (DateTime?)DateTime.Now;

Почему этот класс выглядит как структура с public полями?
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956123
fkthat
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
crutchmaster
listtoview obj.RoleId = rme.RoleId;
obj.User = rme.User;

Если там будет 50 полей?
obj.Deleted = !flagDeleted ? null : (DateTime?)DateTime.Now;

Почему этот класс выглядит как структура с public полями?
Потому что фактически это DTO, а они так и выглядят. Паттерн "anemic model". Классы сущностей тоже так обычно выглядят - никого же это не смущает.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956154
listtoview
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
fkthat
crutchmaster
пропущено...

Если там будет 50 полей?
пропущено...

Почему этот класс выглядит как структура с public полями?

Потому что фактически это DTO, а они так и выглядят. Паттерн "anemic model". Классы сущностей тоже так обычно выглядят - никого же это не смущает.

я если руками делаю, то стараюсь максимально закрыть, а сетить только чз конструктор
не знаю насколько это правильно, само получается
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956397
love_bach
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
listtoview
fkthat
пропущено...

Потому что фактически это DTO, а они так и выглядят. Паттерн "anemic model". Классы сущностей тоже так обычно выглядят - никого же это не смущает.

я если руками делаю, то стараюсь максимально закрыть, а сетить только чз конструктор
не знаю насколько это правильно, само получается


DTO чз конструктор?
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956400
ViPRos
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
love_bach

DTO чз конструктор?

да мозгов нет, вот понапридумали всякую ерунду
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956623
Фотография crutchmaster
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
fkthat
Классы сущностей тоже так обычно выглядят - никого же это не смущает.

Копипаста полей никого не смущает? Ну ок.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956626
fkthat
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
crutchmaster
fkthat
Классы сущностей тоже так обычно выглядят - никого же это не смущает.

Копипаста полей никого не смущает? Ну ок.

Не смущает.
...
Рейтинг: 0 / 0
Покритикуйте, пожалуйста, код.
    #39956635
Фотография hVostt
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Участник
ViPRos
love_bach

DTO чз конструктор?

да мозгов нет, вот понапридумали всякую ерунду


подходящего инструмента нет, DTO это такой костыль из-за отсутствия рекордов
...
Рейтинг: 0 / 0
25 сообщений из 27, страница 1 из 2
Форумы / ASP.NET [игнор отключен] [закрыт для гостей] / Покритикуйте, пожалуйста, код.
Целевая тема:
Создать новую тему:
Автор:
Закрыть
Цитировать
Найденые пользователи ...
Разблокировать пользователей ...
Читали форум (0):
Пользователи онлайн (0):
x
x
Закрыть


Просмотр
0 / 0
Close
Debug Console [Select Text]