Проведите ревью кода?

@olegall · Регистрация: 27.01.2014

Студворк — интернет-сервис помощи студентам

Посмотрите, пожалуйста, код - не прошёл испытательный срок - жалуются на качество. Всё ли так плохо?

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
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
using StecPoint.ES.Infrastructure.Integration.Interfaces.Abstract;
using StecPoint.ES.SendSay.Sdk.Model.Response;
 
namespace StecPoint.ES.Infrastructure.Integration.SendSay
{
    class SendSayList : ServiceListAbstract
    {
        public SendSayList()
        {
 
        }
 
        public SendSayList(List x)
        {
            Id = x.Id;
            Name = x.Name;
        }
    }
}
 
using StecPoint.ES.Infrastructure.Integration.Interfaces.Abstract;
using StecPoint.ES.Infrastructure.Integration.Interfaces.Dto;
using StecPoint.ES.SendSay.Sdk.Model.Request;
 
namespace StecPoint.ES.Infrastructure.Integration.SendSay
{
    public class SendSayMember : ServiceListMemberAbstract
    {
        public SendSayMember()
        {
 
        }
 
        public SendSayMember(MemberDto x)
        {
            ListId = x.ListId;
            Email = x.Email;
            PhoneNumber = x.PhoneNumber;
            Params = x.Params;
            FirstName = x.FirstName;
            LastName = x.LastName;
        }
 
        public Subscriber GetSubscriber()
        {
            var subs = new Subscriber
            {
                Email = Email,
                GroupId = "" // !
            };
            return subs;
        }
    }
}
 
using StecPoint.ES.Infrastructure.Integration.Interfaces.Abstract;
using StecPoint.ES.SendSay.Sdk.Model.Response;
 
namespace StecPoint.ES.Infrastructure.Integration.SendSay
{
    public class SendSayProperty : ServiceParamAbstract
    {
        public SendSayProperty()
        {
            
        }
 
        public SendSayProperty(Quest x)
        {
            Id = x.Id;
            Name = x.Name;
        }
    }
}
 
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using StecPoint.ES.Infrastructure.Integration.Interfaces;
using StecPoint.ES.Infrastructure.Integration.Interfaces.Abstract;
using StecPoint.ES.Infrastructure.Integration.Interfaces.Dto;
using StecPoint.ES.SendSay.Sdk.Manager;
using StecPoint.ES.SendSay.Sdk.Model;
using StecPoint.ES.SendSay.Sdk.Model.Response;
 
namespace StecPoint.ES.Infrastructure.Integration.SendSay
{
    public class SendSayServiceProvider : IPartnerServiceProvider
    {
        private readonly SendSayManager _manager;
 
        public SendSayServiceProvider(AuthConfigAbstract config)
        {
            var apiConfig = new ApiConfig { UserName = config.UserName,
                                            Password = config.Password};
            _manager = new SendSayManager(apiConfig);
        }
 
        public async Task<ServerResponseDto> ValidateApiKeyAsync()
        {
            var status = await _manager.Subscribers.ValidateRequestAsync();
            return new ServerResponseDto() { Status = status };
        }
 
        public async Task<IEnumerable<ServiceListAbstract>> GetListsAsync()
        {
            var res = new List<SendSayList>();
            try
            {
                var lists = await _manager.Lists.GetAllAsync();
                res = lists
                    .Select(x => new SendSayList(x))
                    .ToList();
 
                var listParams = (await GetListParamsAsync(string.Empty, null)).ToList();
 
                foreach (var item in res)
                {
                    item.ServiceListParams = listParams;
                }
            }
            catch
            {
                // ignore
            }
            return res;
        }
 
 
 
        public async Task<ServiceListAbstract> GetListByIdAsync(string id)
        {
            //return await Task.Run(() => new WebHookList());
            return null; // !
        }
 
        public async Task<IEnumerable<ServiceParamAbstract>> GetListParamsAsync(string listId, string[] standartParams)
        {
            var res = new List<SendSayProperty>();
            var lists = await _manager.Fields.GetAllAsync();
            res = lists
                  .Select(x => new SendSayProperty(x))
                  .ToList();
            return res;
        }
 
        public async Task<IEnumerable<ServiceParamAbstract>> GetListParamsAsync(string listId, bool addEmailParam)
        {
            //return await GetListParamsAsync(listId, null);
            return null; // !
        }
 
        public async Task<ServiceListMemberAbstract> AddListMember(MemberDto member)
        {
            SendSayMember res = null;
            try
            {
                var sendSayMember = new SendSayMember(member);
                var subs = sendSayMember.GetSubscriber();
 
                var newMembers = await _manager.Subscribers.AddAsync(subs);
 
                return new SendSayMember
                {
                    Id = newMembers.Id
                };
            }
            catch
            {
                // ignore
            }
            return res;
        }
 
        public async Task<IEnumerable<int>> GetEmailTemplatesAsync()
        {
            IEnumerable<int> emailTemplates = await _manager.EmailTemplates.GetAllAsync();
            return emailTemplates;
        }
    }
}
 
using System;
using System.Collections.Generic;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using StecPoint.ES.SendSay.Sdk.Interface;
using StecPoint.ES.SendSay.Sdk.Model.Response;
using Newtonsoft.Json;
using System.Linq;
using RestSharp;
using Newtonsoft.Json.Linq;
using System.Collections;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class EmailTemplateManager : ManagerBase, IEmailTemplateManager
    {
        public EmailTemplateManager(IApiConfig config) : base(config)
        {
        }
 
        public async Task<IEnumerable<int>> GetAllAsync()
        {
            try
            {
                RestClient client = GetApiClient();
                RestRequest formsRequest = GetBaseRequest("infolett.list");
                CancellationTokenSource cancelTokenSrc = new CancellationTokenSource();
                IRestResponse formsResp = null;
                try
                {
                    formsResp = await client.ExecuteTaskAsync(formsRequest, cancelTokenSrc.Token);
                }
                catch (Exception e)
                { 
                }
                if (formsResp.StatusCode != HttpStatusCode.Created && formsResp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {formsResp.StatusCode}");
                }
                IEnumerable<int> templates = JsonConvert.DeserializeObject<EmailTemplateList>(formsResp.Content).List.Select(x => x.Id).ToArray();
                return templates;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }
    }
}
 
using System;
using System.Collections.Generic;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using StecPoint.ES.SendSay.Sdk.Interface;
using StecPoint.ES.SendSay.Sdk.Model.Response;
using Newtonsoft.Json;
using System.Linq;
using RestSharp;
using Newtonsoft.Json.Linq;
using System.Collections;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class FieldManager : ManagerBase, IFieldManager
    {
        public FieldManager(IApiConfig config) : base(config)
        {
        }
 
        public async Task<IList<Quest>> GetAllAsync()
        {
            try
            {
                RestClient client = GetApiClient();
                RestRequest formsRequest = GetBaseRequest("anketa.list");
                CancellationTokenSource cancelTokenSrc = new CancellationTokenSource();
                IRestResponse formsResp = await client.ExecuteTaskAsync(formsRequest, cancelTokenSrc.Token);
                if (formsResp.StatusCode != HttpStatusCode.Created && formsResp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {formsResp.StatusCode}");
                }
                List<Quest> quests = new List<Quest>();
                string[] formIds = JsonConvert.DeserializeObject<FormList>(formsResp.Content).List.Select(x => x.Id).ToArray();
                foreach (string formId in formIds)
                {
                    RestRequest readFormRequest = GetBaseRequest("anketa.get");
                    Parameter requestParam = readFormRequest.Parameters.FirstOrDefault(x => x.Name == "request");
                    requestParam.Value = AddObject(requestParam, "id", formId);
 
                    IRestResponse readFormResp = await client.ExecuteTaskAsync(readFormRequest, cancelTokenSrc.Token);
                    dynamic json = JArray.Parse('[' + readFormResp.Content + ']');
                    IEnumerable<Quest> questsFromJSON = ((IEnumerable)json[0].obj.quests)
                                                        .Cast<dynamic>()
                                                        .Select(x => Deserialize(x))
                                                        .Cast<Quest>()
                                                        .Select(x => new Quest(x.Id, x.Name));
                    quests.AddRange(questsFromJSON);
                }
                return quests;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }
 
        public async Task<IEnumerable<FormWithQuests>> GetFormsWithQuestsAsync()
        {
            try
            {
                RestClient client = GetApiClient();
                RestRequest formsRequest = GetBaseRequest("anketa.list");
                CancellationTokenSource cancelTokenSrc = new CancellationTokenSource();
                IRestResponse formsResp = await client.ExecuteTaskAsync(formsRequest, cancelTokenSrc.Token);
                if (formsResp.StatusCode != HttpStatusCode.Created && formsResp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {formsResp.StatusCode}");
                }
                List<FormWithQuests> formsWithQuests = new List<FormWithQuests>();
                string[] formIds = JsonConvert.DeserializeObject<FormList>(formsResp.Content).List.Select(x => x.Id).ToArray();
                foreach (string formId in formIds)
                {
                    RestRequest readFormRequest = GetBaseRequest("anketa.get");
                    Parameter requestParam = readFormRequest.Parameters.FirstOrDefault(x => x.Name == "request");
                    requestParam.Value = AddObject(requestParam, "id", formId);
 
                    IRestResponse readFormResp = await client.ExecuteTaskAsync(readFormRequest, cancelTokenSrc.Token);
                    dynamic json = JArray.Parse('[' + readFormResp.Content + ']');
                    IEnumerable<Quest> questsFromJSON = ((IEnumerable)json[0].obj.quests)
                                                        .Cast<dynamic>()
                                                        .Select(x => Deserialize(x))
                                                        .Cast<Quest>()
                                                        .Select(x => new Quest(x.Id, x.Name));
                    formsWithQuests.Add(new FormWithQuests(formId, questsFromJSON));
 
                }
                return formsWithQuests;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }
 
        Func<dynamic, dynamic> Deserialize = delegate (dynamic q)
        {
            return (dynamic)JsonConvert.DeserializeObject<Quest>(q.Value.ToString()
                                                                  .Replace("\r\n", string.Empty));
        };
    }
}
 
 
using System;
using System.Collections.Generic;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using StecPoint.ES.SendSay.Sdk.Interface;
using StecPoint.ES.SendSay.Sdk.Model.Response;
using Newtonsoft.Json;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class ListManager : ManagerBase, IListManager
    {
        public ListManager(IApiConfig config) : base(config)
        {
        }
 
        public async Task<IList<List>> GetAllAsync()
        {
            try
            {
                var client = GetApiClient();
                var request = GetBaseRequest("group.list");
                var cancelTokenSrc = new CancellationTokenSource();
                var resp = await client.ExecuteTaskAsync<ListResult>(request, cancelTokenSrc.Token);
                if (resp.StatusCode != HttpStatusCode.Created && resp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {resp.StatusCode}");
                }
                try
                {
                    ListResult lists = JsonConvert.DeserializeObject<ListResult>(resp.Content);
                    return lists.List;
                }
                catch (Exception e)
                {
                    Logger?.Error(e);
                    return null;
                }
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }
    }
}
 
using System;
using NLog;
using RestSharp;
using StecPoint.ES.SendSay.Sdk.Interface;
using Newtonsoft.Json;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class ManagerBase
    {
        protected readonly IApiConfig ApiConfig;
        protected readonly ILogger Logger;
        private static readonly string _baseApiServer = "https://api.sendsay.ru/clu206";
 
        public ManagerBase(IApiConfig config)
        {
            ApiConfig = config;
            Logger = config.Logger;
        }
 
        protected RestClient GetApiClient()
        {
            var client = new RestClient
            {
                BaseUrl = new Uri(_baseApiServer)
            };
            return client;
        }
 
        public RestRequest GetBaseRequest(string action)
        {
            RestRequest request = new RestRequest(_baseApiServer, Method.POST);
            request.AddParameter("apiversion", "100");
            request.AddParameter("json", "1");
            UserName username = new UserName(ApiConfig.UserName);
            string json = JsonConvert.SerializeObject(
                          new
                          {
                              one_time_auth = new
                              {
                                  login = username.Login,
                                  sublogin = username.Sublogin,
                                  passwd = ApiConfig.Password
                              },
                              action = action
                          });
            request.AddParameter("request", json);
            return request;
        }
 
        protected string AddObject(Parameter param, string key, string value)
        {
            value = key == "obj" ? value : '"'+value+'"';
            string paramVal = param.Value.ToString();
            return paramVal.Insert(paramVal.Length - 1, ",\"" + key + "\":" + value + "");
        }
    }
}
 
 
using StecPoint.ES.SendSay.Sdk.Interface;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class SendSayManager : ISendSayManager
    {
        public IApiConfig ApiConfig { get; set; }
 
        public IListManager Lists { get; set; }
 
        public ISubscriberManager Subscribers { get; set; }
 
        public IFieldManager Fields { get; set; }
 
        public IEmailTemplateManager EmailTemplates { get; set; }
 
        public SendSayManager(IApiConfig apiConfig)
        {
            ApiConfig = apiConfig;
            Lists = new ListManager(ApiConfig);
            Subscribers = new SubscriberManager(ApiConfig);
            Fields = new FieldManager(apiConfig);
            EmailTemplates = new EmailTemplateManager(apiConfig);
        }
    }
}
 
using System;
using System.Net;
using System.Threading;
using System.Threading.Tasks;
using StecPoint.ES.SendSay.Sdk.Interface;
using StecPoint.ES.SendSay.Sdk.Model.Request;
using StecPoint.ES.SendSay.Sdk.Model.Response;
using System.Linq;
using RestSharp;
using System.Collections.Generic;
using System.Text;
using Newtonsoft.Json;
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    public class SubscriberManager : ManagerBase, ISubscriberManager
    {
        public SubscriberManager(IApiConfig config) : base(config)
        {
 
        }
        public async Task<int> ValidateRequestAsync()
        {
            try
            {
                var client = GetApiClient();
                var request = GetBaseRequest("group.list");
                var cancelTokenSrc = new CancellationTokenSource();
                var resp = await client.ExecuteTaskAsync<ListResult>(request, cancelTokenSrc.Token);
                if (resp.StatusCode != HttpStatusCode.Created && resp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {resp.StatusCode}");
                    return (int)resp.StatusCode;
                }
                return 200;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return (int)HttpStatusCode.ServiceUnavailable;
            }
        }
 
        public async Task<Person> AddAsync(Subscriber subscriber)
        {
            try
            {
                RestClient client = GetApiClient();
                RestRequest request = GetBaseRequest("member.set");
 
                Parameter requestParam = request.Parameters.FirstOrDefault(x => x.Name == "request");
                requestParam.Value = AddObject(requestParam, "email", subscriber.Email);
                requestParam.Value = AddObject(requestParam, "newbie.confirm", subscriber.OptIn ? "1" : "0");
                if (subscriber.OptIn)
                    requestParam.Value = AddObject(requestParam, "newbie.letter.confirm", subscriber.TemplateId.ToString());
 
                requestParam.Value = AddObject(requestParam, "obj", await GetObjVal(subscriber.GroupId));
 
                CancellationTokenSource cancelTokenSrc = new CancellationTokenSource();
                IRestResponse resp = await client.ExecuteTaskAsync(request, cancelTokenSrc.Token);
                if (resp.StatusCode != HttpStatusCode.Created && resp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {resp.StatusCode}");
                }
                SubscriberResponse data = JsonConvert.DeserializeObject<SubscriberResponse>(resp.Content);
                return data.Member;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }
 
        private async Task<string> GetObjVal(string groupId)
        {
            FieldManager fieldMng = new FieldManager(ApiConfig);
            StringBuilder objVal = new StringBuilder("{");
            IEnumerable<FormWithQuests> formsWithQuests = await fieldMng.GetFormsWithQuestsAsync();
            foreach (FormWithQuests formWithQuests in formsWithQuests)
            {
                IEnumerable<Quest> quests = formWithQuests.Quests;
                if (!quests.Any()) continue;
                objVal.Append('"').Append(formWithQuests.Id).Append('"').Append(":{");
                for (int i = 0; i < quests.Count(); i++)
                {
                    Quest quest = quests.ElementAt(i);
                    objVal.Append('"').Append(quest.Id).Append('"')
                          .Append(':')
                          .Append('"').Append(quest.Name).Append('"');
                    if (i < quests.Count() - 1)
                    {
                        objVal.Append(',');
                    }
                }
                objVal.Append("},");
            }
            objVal.Append("\"-group\": {\"" + groupId + "\":\"1\"}}");
            return objVal.ToString();
        }
    }
}
 
 
namespace StecPoint.ES.SendSay.Sdk.Manager
{
    class UserName
    {
        private const char delimeter = '|';
        public string Login { get; set; }
        public string Sublogin { get; set; }
        public UserName(string username)
        {
            Login = username.Split(delimeter)[0];
            Sublogin = username.Split(delimeter)[1];
        }
    }
}

Этот модуль сделан на основе других подобных, т.е. архитектурно идентичный. Тут нельзя радикально что-то сделать не так. Толком и не объяснили в чём дело - толь по поводу десериализации, где приведение к dynamic. Мол, плохо код читается. Но он работает вообще-то. Весь модуль интеграции протестирован и корректно работает

@Cupko · 17.11.2018, 12:35

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
        public async Task<IList<List>> GetAllAsync()
        {
            try
            {
                var client = GetApiClient();
                var request = GetBaseRequest("group.list");
                var cancelTokenSrc = new CancellationTokenSource();
                var resp = await client.ExecuteTaskAsync<ListResult>(request, cancelTokenSrc.Token);
                if (resp.StatusCode != HttpStatusCode.Created && resp.StatusCode != HttpStatusCode.OK)
                {
                    Logger?.Error($"Server return error: {resp.StatusCode}");
                }
                try
                {
                    ListResult lists = JsonConvert.DeserializeObject<ListResult>(resp.Content);
                    return lists.List;
                }
                catch (Exception e)
                {
                    Logger?.Error(e);
                    return null;
                }
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return null;
            }
        }

Объясните мне логику try-catch здесь, и я всё прощу

Динамик - зло, делегаты - зло (да простят меня адепты функциональщины)

@olegall · 17.11.2018, 12:42 **[ТС]**

На комментарии, пустые методы можно не обращать внимание

Добавлено через 3 минуты
внешний try-catch ловит факт некорректного ответа на запрос (это так и было), внутренний - некорректную десериализацию

@Cupko · 17.11.2018, 12:47

Сообщение от olegall

внешний try-catch ловит факт некорректного ответа на запрос (это так и было), внутренний - некорректную десериализацию

Вы серьезно?
Вопрос на засыпку: Что изменится, если убрать внутренний try-catch?

Ну и return null; здесь, тоже.

@olegall · 17.11.2018, 12:54 **[ТС]**

если измениться формат АПИ, при десериализации выпадет ошибка, catch её запишет в лог - так я думал
return null по их логике возвращается при любом исключении. в это я лезть не стал

@sau · 17.11.2018, 16:00

Сообщение от olegall

public UserName(string username)
* * * * {
* * * * * * Login = username.Split(delimeter)[0];
* * * * * * Sublogin = username.Split(delimeter)[1];
* * * * }

Каким образом пользователь данного класса должен узнать о том , что строка username должна иметь определенную структуру ? - код , череватый в перспективе ошибками.

public async Task<int> ValidateRequestAsync()

- почему этот метод публичный , как пользователь должен понять , в каких случаях его нужно вызывать и вызывать ли вообще ? , что то там валидирует , без каких либо аргументов - очевидно это служебный метод.

Сообщение от olegall

private async Task<string> GetObjVal(string groupId)

- ну это вообще чудо , ну ладно , он приватный , но семантика ужасная и результат содержит json , формат которого вызывающая функция должна угадать ?

Сообщение от olegall

Func<dynamic, dynamic> Deserialize = delegate (dynamic q)

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

В общем ладно , тут действительно можно придраться к каждой функции , если это SDK для стороннего разработчика - то ему не позавидуешь. Принцип "Разделяй-И-Властвуй" не соблюдается.

@olegall · 19.11.2018, 10:44 **[ТС]**

Добавлено через 10 часов 53 минуты

Сообщение от sau

Принцип "Разделяй-И-Властвуй" не соблюдается.

Что нужно разделить?

Добавлено через 3 минуты
Упростил десериализацию:

C#
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
    public class Quest
    {
        public string Id { get; set; }
        public string Name { get; set; }
    }
 
 var json2 = @"[{
                            'obj':{ 'order':['q957','q479','q214'],
                                    'quests':{
                                             'q214':{'width':10,'name':'Телефон','id':'q214','type':'free'},
                                             'q479':{'width':15,'name':'Имя','type':'free','id':'q479'},
                                             'q957':{'width':100,'name':'Город','id':'q957','type':'free'}
                                            },
                                    'id':'a525',
                                    'param':{'system':0,'name':'Данные пользователя', 'multi':0}},
                            'request.id':'fake -EC06814E-E35B-11E8-B22D-F854389B935E',
                            'duration':0.042354,
                            '_ehid':'212432.23074209719.1541684457'
                        }]";
 
dynamic root = JArray.Parse(json2);
 
IEnumerable<Quest> quests2 = ((IEnumerable<dynamic>)root[0].obj.quests).Select(x => (Quest)JsonConvert.DeserializeObject<Quest>(x.Value.ToString()));

Как ещё можно упростить? Нужно как-то избавиться от dynamic

@SeIZVeIZ · 19.11.2018, 14:49

Сообщение от olegall

Как ещё можно упростить? Нужно как-то избавиться от dynamic

нужно правильно делать структуру классов JSON, по Вашему примеру она такая:

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
public class RootObject
    {
        [JsonProperty("obj")]
        public Obj Obj { get; set; }
 
        [JsonProperty("request.id")]
        public string RequestId { get; set; }
 
        [JsonProperty("duration")]
        public double Duration { get; set; }
 
        [JsonProperty("_ehid")]
        public string Ehid { get; set; }
    }
 
    public class Obj
    {
        [JsonProperty("order")]
        public string[] Order { get; set; }
 
        [JsonProperty("quests")]
        public Dictionary<string, Quest> Quests { get; set; }
 
        [JsonProperty("id")]
        public string Id { get; set; }
 
        [JsonProperty("param")]
        public Param Param { get; set; }
    }
 
    public class Param
    {
        [JsonProperty("system")]
        public long System { get; set; }
 
        [JsonProperty("name")]
        public string Name { get; set; }
 
        [JsonProperty("multi")]
        public long Multi { get; set; }
    }
 
    public class Quest
    {
        [JsonProperty("width")]
        public long Width { get; set; }
 
        [JsonProperty("name")]
        public string Name { get; set; }
 
        [JsonProperty("id")]
        public string Id { get; set; }
 
        [JsonProperty("type")]
        public string Type { get; set; }
    }

и по итогу вы десериализуете по головному классу и уже с ним далее работаете без всяких динамиков

@sau · 19.11.2018, 23:55

Сообщение от olegall

Нужно как-то избавиться от dynamic

dynamic это не зло , он оправдан в том случае , если нужна временная структура в рамках метода , которая более в приложении нигде не используется. Плохо , когда dynamic выносится на уровень свойств класса или является результатом функции - это уже чревато проблемами.

@olegall · 20.11.2018, 20:45 **[ТС]**

Сделал рефакторинг. Какая работа была проведена:
1. разделил на методы, вынес многие в BaseManager
2. упростил названия переменных
3. перенёс все try-catch в базовый класс BaseManager
4. сгруппировал методы по доступу (вверху private, далее publiс)
5. было:

C#
1
2
3
4
5
6
7
8
  protected RestClient GetApiClient()
        {
            var client = new RestClient
            {
                BaseUrl = new Uri(_baseApiServer)
            };
            return client;
        }

стало:

C#
1
2
3
4
5
6
7
8
9
10
 protected RestClient ApiClient
        {
            get
            { 
                return new RestClient
                {
                    BaseUrl = new Uri(_baseApiServer)
                };
            }
        }

6. Было

C#
1
private async Task<string> GetObjVal(string groupId)

стало:

C#
1
private async Task<JSON> GetObjValue(string groupId)

- теперь видно, что метод возвращает JSON. Для это пришлось ввести класс JSON,
а в нём поле Value

7.
Вынес константы:

C#
1
2
3
4
private const string getFormsAPI = "anketa.list";
private const string getFormAPI = "anketa.get";
private const string requestName = "request";
private const string idKey = "id";

причём можно так:

C#
1
2
3
4
5
private static class API
{
    public static string GetForms { get { return "anketa.list"; } } // ! const бессмысленно? Раз отсутствует set
    public static string GetForm = "anketa.get"; 
}

- мотив - чтобы избавится от префикса API в константах.

Как лучше?

8.

C#
1
Parameter request = ....

или лучше Parameter requestParam или requestP - чтобы было понятно,
что это параметр. Или по типу можно понять, наведя мышкой?

9. В UserName.cs я бросаю исключение, как мне посоветовали. Но везде по коду
ошибки пишутся в лог. Согласно стилю предыдущего кода мне тоже надо в лог писать?

10. JSON.cs, RootObject.cs, UserName.cs поместил в папку Model

11. Избавился от dynamic

Если какие-то пункты некорректны, исправьте, пожалуйста. + напишите что ещё исправить

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
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
using System;
using NLog;
using RestSharp;
using ES.SendSay.Sdk.Interface;
using Newtonsoft.Json;
using System.Threading;
using System.Net;
using System.Threading.Tasks;
using ES.SendSay.Sdk.Model;
 
namespace ES.SendSay.Sdk.Manager
{
    public class BaseManager
    {
        private static readonly string _baseApiServer = "https://api.sendsay.ru/clu206"; // ! подчёркивания
        protected const string getGroupsAPI = "group.list";
        protected readonly IApiConfig ApiConfig;
        protected readonly ILogger Logger;
        protected CancellationToken Token { get { return new CancellationTokenSource().Token; } }
 
        public BaseManager(IApiConfig config)
        {
            ApiConfig = config;
            Logger = config.Logger;
        }
 
        protected IRestRequest GetBaseRequest(string action)
        {
            IRestRequest request = new RestRequest(_baseApiServer, Method.POST);
            request.AddParameter("apiversion", "100");
            request.AddParameter("json", "1");
            UserName username = new UserName(ApiConfig.UserName);
            string json = JsonConvert.SerializeObject(
                          new
                          {
                              one_time_auth = new
                              {
                                  login = username.Login,
                                  sublogin = username.Sublogin,
                                  passwd = ApiConfig.Password
                              },
                              action = action
                          });
            request.AddParameter("request", json);
            return request;
        }
 
        protected RestClient ApiClient
        {
            get
            { 
                return new RestClient
                {
                    BaseUrl = new Uri(_baseApiServer)
                };
            }
        }
 
        protected string AddObjectToParam(Parameter param, string key, string value)
        {
            value = key == "obj" ? value : '"'+value+'"';
            string paramVal = param.Value.ToString();
            return paramVal.Insert(paramVal.Length - 1, ",\"" + key + "\":" + value + "");
        }
 
        protected async Task<IRestResponse> GetResponseAsync(IRestRequest request)
        {
            IRestResponse response = null; // !
            try
            {
                response = await ApiClient.ExecuteTaskAsync(request, Token);
                return response;
            }
            catch (Exception e)
            {
                Logger?.Error($"Response getting error: {e.Message}");
            }
            if (response.StatusCode != HttpStatusCode.Created && response.StatusCode != HttpStatusCode.OK)
            {
                Logger?.Error($"Server return error: {response.StatusCode}");
            }
            return null;
        }
 
        protected T GetDeserialized<T>(string content)
        {
            try
            {
                return JsonConvert.DeserializeObject<T>(content);
            }
            catch (Exception e)
            {
                Logger?.Error($"Deserialization error: {e.Message}. Probably API structure has changed");
            }
            return (T)(object)null; // !
        }
    }
}
 
 
 
 
 
 
 
 
 
 
 
 
using System;
using System.Net;
using System.Threading.Tasks;
using ES.SendSay.Sdk.Interface;
using ES.SendSay.Sdk.Model.Request;
using ES.SendSay.Sdk.Model.Response;
using System.Linq;
using RestSharp;
using System.Collections.Generic;
using System.Text;
using ES.SendSay.Sdk.Model;
 
namespace ES.SendSay.Sdk.Manager
{
    public class SubscriberManager : BaseManager, ISubscriberManager
    {
        private FieldManager fieldMng;
        private const string getMembersAPI = "member.set";
        private const string emailKey = "email";
        private const string newbieConfirmKey = "newbie.confirm";
        private const string newbieLetterConfirmKey = "newbie.letter.confirm";
        private const string objKey = "obj";
 
        public SubscriberManager(IApiConfig config) : base(config)
        {
            fieldMng = new FieldManager(ApiConfig);
        }
 
        private async Task<JSON> GetObjValue(string groupId)
        {
            StringBuilder objVal = new StringBuilder("{");
            IEnumerable<FormWithQuests> formsWithQuests = await fieldMng.GetFormsWithQuestsAsync();
            foreach (FormWithQuests formWithQuests in formsWithQuests)
            {
                IEnumerable<Quest> quests = formWithQuests.Quests;
                if (!quests.Any()) continue;
                objVal.Append('"').Append(formWithQuests.Id).Append('"').Append(":{");
                for (int i = 0; i < quests.Count(); i++)
                {
                    Quest quest = quests.ElementAt(i);
                    objVal.Append('"').Append(quest.Id).Append('"')
                          .Append(':')
                          .Append('"').Append(quest.Name).Append('"');
                    if (i < quests.Count() - 1)
                    {
                        objVal.Append(',');
                    }
                }
                objVal.Append("},");
            }
            objVal.Append("\"-group\": {\"" + groupId + "\":\"1\"}}");
            return new JSON(objVal.ToString());
        }
 
        public async Task<int> ValidateRequestAsync()
        {
            try
            {
                IRestResponse resp = await GetResponseAsync(GetBaseRequest(getGroupsAPI)); // ! уже есть try-catch внутри
                return 200;
            }
            catch (Exception e)
            {
                Logger?.Error(e);
                return (int)HttpStatusCode.ServiceUnavailable;
            }
        }
 
        public async Task<Person> AddAsync(Subscriber subscriber)
        {
            Parameter request = GetBaseRequest(getMembersAPI).Parameters.FirstOrDefault(x => x.Name == "request"); // ! requestP
            request.Value = AddObjectToParam(request, emailKey, subscriber.Email);
            request.Value = AddObjectToParam(request, newbieConfirmKey, subscriber.OptIn ? "1" : "0");
            if (subscriber.OptIn)
                request.Value = AddObjectToParam(request, newbieLetterConfirmKey, subscriber.TemplateId.ToString());
 
            request.Value = AddObjectToParam(request, objKey, (await GetObjValue(subscriber.GroupId)).Value);
 
            IRestResponse response = await GetResponseAsync(GetBaseRequest(getMembersAPI));
            SubscriberResponse data = GetDeserialized<SubscriberResponse>(response.Content);
            return data.Member;
        }
    }
}
 
 
 
 
 
 
 
 
 
 
 
 
 
using ES.SendSay.Sdk.Interface;
 
namespace ES.SendSay.Sdk.Manager
{
    public class SendSayManager : ISendSayManager
    {
        public IApiConfig ApiConfig { get; set; }
        public IListManager Lists { get; set; }
        public ISubscriberManager Subscribers { get; set; }
        public IFieldManager Fields { get; set; }
        public IEmailTemplateManager EmailTemplates { get; set; }
        public SendSayManager(IApiConfig apiConfig)
        {
            ApiConfig = apiConfig;
            Lists = new ListManager(ApiConfig);
            Subscribers = new SubscriberManager(ApiConfig);
            Fields = new FieldManager(apiConfig);
            EmailTemplates = new EmailTemplateManager(apiConfig);
        }
    }
}
 
 
 
using System.Collections.Generic;
using System.Threading.Tasks;
using ES.SendSay.Sdk.Interface;
using ES.SendSay.Sdk.Model.Response;
using RestSharp;
 
namespace ES.SendSay.Sdk.Manager
{
    public class ListManager : BaseManager, IListManager
    {
        public ListManager(IApiConfig config) : base(config)
        {
        }
 
        public async Task<IList<Group>> GetAllAsync()
        {
            IRestResponse response = await GetResponseAsync(GetBaseRequest(getGroupsAPI));
            ListResult lists = GetDeserialized<ListResult>(response.Content);
            return lists.List;
        }
    }
}
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
using System.Threading.Tasks;
using ES.SendSay.Sdk.Interface;
using ES.SendSay.Sdk.Model.Response;
using System.Linq;
using RestSharp;
using System.Collections.Generic;
using ES.SendSay.Sdk.Model;
 
namespace ES.SendSay.Sdk.Manager
{
    public class FieldManager : BaseManager, IFieldManager
    {
        private static class API
        {
            public static string GetForms { get { return "anketa.list"; } } // ! const бессмысленно? Раз отсутствует set
            public static string GetForm = "anketa.get"; 
        }
 
        private const string getFormsAPI = "anketa.list";
        private const string getFormAPI = "anketa.get";
        private const string requestName = "request";
        private const string idKey = "id";
 
        public FieldManager(IApiConfig config) : base(config)
        {
        }
 
        private async Task<string[]> GetFormIdsAsync(string apiMethod)
        {
            IRestResponse formsResp = await GetResponseAsync(GetBaseRequest(apiMethod));
            string[] formIds = GetDeserialized<FormList>(formsResp.Content).List.Select(x => x.Id).ToArray();
            return formIds;
        }
 
        private async Task<IEnumerable<QuestObj>> GetQuestsFromJSON(string formId)
        {
            IRestRequest readFormRequest = GetBaseRequest(getFormAPI);
            Parameter request = readFormRequest.Parameters.FirstOrDefault(x => x.Name == requestName);
            request.Value = AddObjectToParam(request, idKey, formId);
            IRestResponse readFormResp = await GetResponseAsync(readFormRequest);
            IEnumerable<QuestObj> questsFromJSON = GetDeserialized<RootObject>(readFormResp.Content).Obj.Quests.Select(x => x.Value);
            return questsFromJSON;
        }
 
        public async Task<IList<Quest>> GetAllAsync()
        {
            IList<Quest> quests = new List<Quest>();
            foreach (string formId in await GetFormIdsAsync(API.GetForms))
            {
                foreach (QuestObj quest in await GetQuestsFromJSON(formId))
                {
                    quests.Add(new Quest(quest.Id, quest.Name));
                }
            }
            return quests;
        }
 
        public async Task<IEnumerable<FormWithQuests>> GetFormsWithQuestsAsync()
        {
            IList<FormWithQuests> formsWithQuests = new List<FormWithQuests>();
            foreach (string formId in await GetFormIdsAsync(API.GetForms))
            {
                IList<Quest> quests = new List<Quest>();
                foreach (QuestObj quest in await GetQuestsFromJSON(formId))
                {
                    quests.Add(new Quest(quest.Id, quest.Name));
                }
                formsWithQuests.Add(new FormWithQuests(formId, quests));
            }
            return formsWithQuests;
        }
    }
}
 
 
 
 
 
 
 
 
 
 
 
 
namespace ES.SendSay.Sdk.Model
{
    class JSON
    {
        public string Value { get; set; }
        public JSON(string value)
        {
            Value = value;
        }
    }
}
 
using System.Collections.Generic;
using Newtonsoft.Json;
 
namespace ES.SendSay.Sdk.Model
{
    public class RootObject
    {
        [JsonProperty("obj")]
        public Obj Obj { get; set; }
    }
 
    public class Obj
    {
        [JsonProperty("quests")]
        public Dictionary<string, QuestObj> Quests { get; set; }
    }
 
    public class QuestObj
    {
        [JsonProperty("name")]
        public string Name { get; set; }
 
        [JsonProperty("id")]
        public string Id { get; set; }
    }
}
 
 
 
 
 
 
 
 
 
 
 
using System;
 
namespace ES.SendSay.Sdk.Model
{
    class UserName
    {
        private const char delimeter = '|';
        public string Login { get; set; }
        public string Sublogin { get; set; }
        public UserName(string username)
        {
            if (string.IsNullOrEmpty(username))
            {
                throw new ArgumentException("Username is not filled", nameof(username));
            }
            string[] logins = username.Split(delimeter);
            if (logins.Length == 2)
            {
                Login = logins[0];
                Sublogin = logins[1];
            }
            else
            {
                throw new ArgumentException($"The parameter {nameof(username)} has incorrect content.");
            }
        }
    }
}

@nicolas2008 · 21.11.2018, 10:22

Свойство при каждом обращении создающее новый обьект это плохо потому что неочевидно.
Нет единообразия стиля именования полей.
Микс констант и readonly полей.
Manager это ничего не говорящее название.
GetObjVal тоже не несет никакой информационной нагрузки.

@sau · 21.11.2018, 11:20

Сообщение от olegall

https://api.sendsay.ru/clu206

Да и в целом , мне кажется , перегружено излишними абстракциями все.
Это всего навсего SendsayHttpClient , который должен отражать максимально приближенный функционал по api документации, а пользователь данного клиента уже сам решит , какие ему абстракции поверх нужны и как обернуть для себя удобнее.

@olegall · 21.11.2018, 11:42 **[ТС]**

Вместо const везде можно использовать readonly? Как я понял основное отличие, что readonly поле через конструктор можно изменить, т.е. есть дополнительный манёвр?

Добавлено через 2 минуты

Сообщение от sau

Да и в целом , мне кажется , перегружено излишними абстракциями все.
Это всего навсего SendsayHttpClient , который должен отражать максимально приближенный функционал по api документации, а пользователь данного клиента уже сам решит , какие ему абстракции поверх нужны и как обернуть для себя удобнее.

ничего не понял. что мне изменить надо?

@sau · 21.11.2018, 11:45

Сообщение от olegall

ничего не понял. что мне изменить надо?

концептуальный подход ).

@olegall · 21.11.2018, 11:49 **[ТС]**

Сообщение от sau

концептуальный подход ).

что конкретно переделать, как?

@sau · 21.11.2018, 12:29

Ну смотрите , как обычно пишут клиентов для апи под разные языки.

В первую очередь , я как пользователь смотрю на документацию api , дальше качаю готовый клиент на c# и ожидаю увидеть методы и структуры в соответствии с документацией.
Дальше , уже исходя из логики своего приложения , я оборачиваю апи клиента в свои бизнес.сервисы и использую методы апи в нужной мне последовательности.

Т.е сам клиент апи должен четко соответствовать документации , иначе придется писать еще и документацию на реализацию , а пользователь должен будет не только доку по апи изучить , но и доку по вашему клиенту , дабы понять как им вообще пользоваться , какие его методы соответствуют методам реального сервиса по апи.

И еще , чем Вы обоснуете применение интерфейса IApiConfig в конструкторе , почему нельзя настройки передать простыми типами аргументов не обременяя данным интерфейсом ?
- интерфейс уместен тогда , когда предполагаются различные варианты реализации , какие варианты тут могут быть ?
- передача комплексного типа конфигурации в конструктор уместна тогда , когда этот комплексный тип конфигурации используется в нескольких классах и содержит какую то общую логику их конфигурирования. , а тут это чем обосновано ?

Аналогичные вопросы можно задать по отношению к прочим интерфейсам связанным с веб.апи интерфейсами , - Вы полагаете , что реализация клиента может быть в будущем переопределена ?
Вопрос , я как пользователь апи , какие плюшки от наличия интерфейсов получаю ? - у нас что , протокол взаимодействия с сервисом api планируется меняться ? - обычно , тогда и интерфейс как правило меняется.
Т.е мысль такая - не нужно перегружать код неуместными абстракциями с сомнительной перспективой использования , это усложняет жизнь без необходимости. - насколько это тут уместно Вам возможно виднее , но я как пользователь не вижу для себя пользы от их наличия в рамках текущей реализации , они даже нигде в других местах не используются.

@olegall · 21.11.2018, 12:34 **[ТС]**

Сообщение от sau

И еще , чем Вы обоснуете применение интерфейса IApiConfig в конструкторе , почему нельзя настройки передать простыми типами аргументов не обременяя данным интерфейсом ?
- интерфейс уместен тогда , когда предполагаются различные варианты реализации , какие варианты тут могут быть ?
- передача комплексного типа конфигурации в конструктор уместна тогда , когда этот комплексный тип конфигурации используется в нескольких классах и содержит какую то общую логику их конфигурирования. , а тут это чем обосновано ?
Аналогичные вопросы можно задать по отношению к прочим интерфейсам , - Вы полагаете , что реализация клиента может быть в будущем переопределена ?
Вопрос , я как пользователь апи , какие плюшки от наличия интерфейсов получаю ? - у нас что , протокол взаимодействия с сервисом api планируется меняться ? - обычно , тогда и интерфейс как правило меняется.
Т.е мысль такая - не нужно перегружать код неуместными абстракциями с сомнительной перспективой использования , это усложняет жизнь без необходимости.

Всё, что касается интерфейсов, делал не я, я их перенёс, чтобы сохранить стиль, так как остальные интеграции были выполнены подобным образом

Сообщение от sau

Ну смотрите , как обычно пишут клиентов для апи под разные языки.
В первую очередь , я как пользователь смотрю на документацию api , дальше качаю готовый клиент на c# и ожидаю увидеть методы и структуры в соответствии с документацией.
Дальше , уже исходя из логики своего приложения , я оборачиваю апи клиента в свои бизнес.сервисы и использую методы апи в нужной мне последовательности.
Т.е сам клиент апи должен четко соответствовать документации , иначе придется писать еще и документацию на реализацию , а пользователь должен будет не только доку по апи изучить , но и доку по вашему клиенту , дабы понять как им вообще пользоваться , какие его методы соответствуют методам реального сервиса по апи.

но ведь я делаю не клиента для конечного пользователя, а интеграции. структура интеграции (интерфейсы) были заданы до меня, и весь архитектурный каркас тоже.

Код, который я поместил первым - это до рефакторинга. В таком стиле выполнен остальной код. На меня грузили за качество, но я так понял качество везде плохое?

@maximka777 · 21.11.2018, 12:48

Сообщение от olegall

- теперь видно, что метод возвращает JSON. Для это пришлось ввести класс JSON,
а в нём поле Value

Только чтоб видно было )

C#
1
using JSON = System.String;

@sau · 21.11.2018, 13:02

Сообщение от olegall

На меня грузили за качество, но я так понял качество везде плохое?

это смотря о каком качестве конкретно мы говорим , качество реализации - одно . качество архитектуры - другое , качество оформления кода - третье , и т.п.

Сообщение от olegall

теперь видно, что метод возвращает JSON. Для это пришлось ввести класс JSON,
а в нём поле Value

мда , и что в итоге изменилось ? , ну да , какой то Json , а толку , как его юзать ? - об этом знает только та функции , которой он нужен , а значит нет смысла выносить такой код в отдельный метод , либо возвращаться должен вполне определенный тип.

@olegall · 21.11.2018, 13:22 **[ТС]**

Сообщение от sau

мда , и что в итоге изменилось ? , ну да , какой то Json , а толку , как его юзать ? - об этом знает только та функции , которой он нужен , а значит нет смысла выносить такой код в отдельный метод , либо возвращаться должен вполне определенный тип.

Сделал так:

C#
1
 using JSON = System.String;

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
        private async Task<JSON> GetObjKeyValue(string groupId)
        {
            StringBuilder objVal = new StringBuilder("{");
            IEnumerable<FormWithQuests> formsWithQuests = await fieldMng.GetFormsWithQuestsAsync();
            foreach (FormWithQuests formWithQuests in formsWithQuests)
            {
                IEnumerable<Quest> quests = formWithQuests.Quests;
                if (!quests.Any()) continue;
                objVal.Append('"').Append(formWithQuests.Id).Append('"').Append(":{");
                for (int i = 0; i < quests.Count(); i++)
                {
                    Quest quest = quests.ElementAt(i);
                    objVal.Append('"').Append(quest.Id).Append('"')
                          .Append(':')
                          .Append('"').Append(quest.Name).Append('"');
                    if (i < quests.Count() - 1)
                    {
                        objVal.Append(',');
                    }
                }
                objVal.Append("},");
            }
            objVal.Append("\"-group\": {\"" + groupId + "\":\"1\"}}");
            return objVal.ToString();
        }

Что сейчас плохо? Теперь метод возвращает JSON как строку. Это же нужно было? Метод я выделил из AddAsync, потому что он был слишком большой

Новые блоги и статьи Все статьи Все блоги /
Thinkpad X220 Tablet — это лучший бюджетный ноутбук для учёбы, точка. Programma_Boinc 23.12.2025 Thinkpad X220 Tablet — это лучший бюджетный ноутбук для учёбы, точка. Рецензия / Мнение Это мой обзор планшета X220 с точки зрения школьника. Недавно я решила попытаться уменьшить свой. . .	PhpStorm 2025.3: WSL Terminal всегда стартует в ~ and_y87 14.12.2025 PhpStorm 2025. 3: WSL Terminal всегда стартует в ~ (home), игнорируя директорию проекта Симптом: После обновления до PhpStorm 2025. 3 встроенный терминал WSL открывается в домашней директории. . .	Как объединить две одинаковые БД Access с разными данными VikBal 11.12.2025 Помогите пожалуйста !! Как объединить 2 одинаковые БД Access с разными данными.	Новый ноутбук volvo 07.12.2025 Всем привет. По скидке в "черную пятницу" взял себе новый ноутбук Lenovo ThinkBook 16 G7 на Амазоне: Ryzen 5 7533HS 64 Gb DDR5 1Tb NVMe 16" Full HD Display Win11 Pro	Музыка, написанная Искусственным Интеллектом volvo 04.12.2025 Всем привет. Некоторое время назад меня заинтересовало, что уже умеет ИИ в плане написания музыки для песен, и, собственно, исполнения этих самых песен. Стихов у нас много, уже вышли 4 книги, еще 3. . .
От async/await к виртуальным потокам в Python IndentationError 23.11.2025 Армин Ронахер поставил под сомнение async/ await. Создатель Flask заявляет: цветные функции - провал, виртуальные потоки - решение. Не threading-динозавры, а новое поколение лёгких потоков. Откат?. . .	Поиск "дружественных имён" СОМ портов Argus19 22.11.2025 Поиск "дружественных имён" СОМ портов На странице: https:/ / norseev. ru/ 2018/ 01/ 04/ comportlist_windows/ нашёл схожую тему. Там приведён код на С++, который показывает только имена СОМ портов, типа,. . .	Сколько Государство потратило денег на меня, обеспечивая инсулином. Programma_Boinc 20.11.2025 Сколько Государство потратило денег на меня, обеспечивая инсулином. Вот решила сделать интересный приблизительный подсчет, сколько государство потратило на меня денег на покупку инсулинов. . . .	Ломающие изменения в C#.NStar Alpha Etyuhibosecyu 20.11.2025 Уже можно не только тестировать, но и пользоваться C#. NStar - писать оконные приложения, содержащие надписи, кнопки, текстовые поля и даже изображения, например, моя игра "Три в ряд" написана на этом. . .	Мысли в слух kumehtar 18.11.2025 Кстати, совсем недавно имел разговор на тему медитаций с людьми. И обнаружил, что они вообще не понимают что такое медитация и зачем она нужна. Самые базовые вещи. Для них это - когда просто люди. . .

@olegall 0 / 0 / 0 Регистрация: 27.01.2014 Сообщений: 116
	17.11.2018, 12:42 [ТС]
	На комментарии, пустые методы можно не обращать внимание Добавлено через 3 минуты внешний try-catch ловит факт некорректного ответа на запрос (это так и было), внутренний - некорректную десериализацию 0

@olegall 0 / 0 / 0 Регистрация: 27.01.2014 Сообщений: 116
	17.11.2018, 12:54 [ТС]
	если измениться формат АПИ, при десериализации выпадет ошибка, catch её запишет в лог - так я думал return null по их логике возвращается при любом исключении. в это я лезть не стал 0

@nicolas2008 1152 / 860 / 263 Регистрация: 30.04.2009 Сообщений: 3,603
	21.11.2018, 10:22
	Свойство при каждом обращении создающее новый обьект это плохо потому что неочевидно. Нет единообразия стиля именования полей. Микс констант и readonly полей. Manager это ничего не говорящее название. GetObjVal тоже не несет никакой информационной нагрузки. 0

@sau 2773 / 2073 / 386 Регистрация: 22.07.2011 Сообщений: 7,820
	21.11.2018, 12:29
	Ну смотрите , как обычно пишут клиентов для апи под разные языки. В первую очередь , я как пользователь смотрю на документацию api , дальше качаю готовый клиент на c# и ожидаю увидеть методы и структуры в соответствии с документацией. Дальше , уже исходя из логики своего приложения , я оборачиваю апи клиента в свои бизнес.сервисы и использую методы апи в нужной мне последовательности. Т.е сам клиент апи должен четко соответствовать документации , иначе придется писать еще и документацию на реализацию , а пользователь должен будет не только доку по апи изучить , но и доку по вашему клиенту , дабы понять как им вообще пользоваться , какие его методы соответствуют методам реального сервиса по апи. И еще , чем Вы обоснуете применение интерфейса IApiConfig в конструкторе , почему нельзя настройки передать простыми типами аргументов не обременяя данным интерфейсом ? - интерфейс уместен тогда , когда предполагаются различные варианты реализации , какие варианты тут могут быть ? - передача комплексного типа конфигурации в конструктор уместна тогда , когда этот комплексный тип конфигурации используется в нескольких классах и содержит какую то общую логику их конфигурирования. , а тут это чем обосновано ? Аналогичные вопросы можно задать по отношению к прочим интерфейсам связанным с веб.апи интерфейсами , - Вы полагаете , что реализация клиента может быть в будущем переопределена ? Вопрос , я как пользователь апи , какие плюшки от наличия интерфейсов получаю ? - у нас что , протокол взаимодействия с сервисом api планируется меняться ? - обычно , тогда и интерфейс как правило меняется. Т.е мысль такая - не нужно перегружать код неуместными абстракциями с сомнительной перспективой использования , это усложняет жизнь без необходимости. - насколько это тут уместно Вам возможно виднее , но я как пользователь не вижу для себя пользы от их наличия в рамках текущей реализации , они даже нигде в других местах не используются. 0

Проведите ревью кода?

Решение