Гость
Целевая тема:
Создать новую тему:
Автор:
Форумы / ASP.NET [игнор отключен] [закрыт для гостей] / Покритикуйте, пожалуйста, код. / 25 сообщений из 27, страница 1 из 2
08.05.2020, 17:05
    #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
08.05.2020, 20:05
    #39955458
Дмитрий Мух
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Покритикуйте, пожалуйста, код.
listtoview,

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

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


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

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


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

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

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

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


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

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

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

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

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

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

В программировании существует лишь два характерных затруднения: инвалидация кеша и наименование сущностей :)
...
Рейтинг: 0 / 0
11.05.2020, 08:59
    #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
11.05.2020, 11:24
    #39955831
love_bach
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Покритикуйте, пожалуйста, код.
Дмитрий Мух
listtoview,

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

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


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

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

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

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


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

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

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


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

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

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

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


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

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

Почему этот класс выглядит как структура с public полями?
...
Рейтинг: 0 / 0
12.05.2020, 09:56
    #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
12.05.2020, 11:06
    #39956154
listtoview
Участник
Скрыть профиль Поместить в игнор-лист Сообщения автора в теме
Покритикуйте, пожалуйста, код.
fkthat
crutchmaster
пропущено...

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

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

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

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

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

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


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

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

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

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

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

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

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

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


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


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