using System;
using System.Collections.Generic;
//EXTERNAL CODE
//Приведен только для справки, никак нельзя менять, код обфусцирован, исходников нет
public sealed class DataProvider : IDisposable
{
public extern int LongRunningCalculation(int value, int value2);
public extern void Dispose();
}
// CODE FOR REVIEW:
class Class2 {
// Здесь есть какие-то ошибки тут?
private readonly object _sync = new object();
// Переменая должна быть задукументирована что это такое _ht?
// Предпологаю что нельзя использовать int[] как ключ Почему нельзя?
// Может такой словарь иметь ключ и второе значение как список или массив?
// Думаю да! Вот так Dictionary<int, List<string>>
// Памятка что значение как массив или список может быть, а может быть оно как object (boxing / unboxing)? Не класс ил strcut?
// Какой ключ ты предлогаешь использовать?
// struct (value type) лучше чем class (reference type) и поэтому если ты хочешь написать свой ключ хороший, то во первых ты должен использовать struct, это не ссылочный тип и будет работать быстрее чем класс.
private static Dictionary<int[], object> _ht;
public int GetValue(int index, int index2)
{
Init(); // Это должно быть в constructor? Вроде да! А где constructor?
// Как уже упоминал выше это плохой ключ
var key = new[] {index, index2};
// Слишком сложная логика проверки думаю что ее можно вынести в отдельный метод
if (_ht.ContainsKey(key) & _ht[key].GetType() == typeof(int))
// casting тип object to int Explicit conversions, каждый раз делать casting это хорошо? Думаю нет (плохой ключ)
return ((int)_ht[key]);
else
// Возвращается тип null а у нас int в сигнатуре метода
// По моему мнению можно сделать тип nullable в сигнатуре метода но я думаю это плохая практика
return null;
}
public void Init()
{
// Плохой стиль кода не используются if (code ...) { code ... } также и с lock
if (_ht == null) // try / catch / finaly - я думаю что lock дорогостоящая операция так ли это? (Мой вопрос)
lock (_sync)
Create();
// lock это одна из самых дешевых оперций по синхронизации (мнение Senior и Team Lead)
// Но под капотом lock - это Monitor реализован с использование try / catch / finally? (Мой вопрос)
}
public static void Create()
{
// Мне не неравиться как инициализировали DataProvider
// Нужно быть внимательным! DataProvider реализует интерфейс Dispose()
// using var provider = new DataProvider(); // some as using (var ...) {}
DataProvider provider = new DataProvider();
// Испольование констант в коде как 100 и 12 плохая практика
// Как вообще это явление называется в коде обычно (использование констант в коде) может быть bad style of code?
// Использование двух циклов обезательно или мы можем придумать что-то лучше? Но нам нужны индексы в LongRunningCalculation
for (int i = 0; i < 100; i++)
// Цикл начинается с j = 1
for (int j = 1; j <= 12; j++)
// Каждый раз создается new [] это тоже плохо (плохой ключ)
_ht[new [] { i, j }] = provider.LongRunningCalculation(i, j);
// if we used for using keyword provider will be disposed here
}
}public sealed class DataProvider : IDisposable
{
// nit: Предложил бы названия firstValue, secondValue либо более осмысленные, если возможно
public extern int LongRunningCalculation(int value, int value2);
public extern void Dispose();
}
// nit: сразу бы хотелось видеть уровень доступа и sealed (если класс не планируется наследовать)
// Class2 - дать нормальное имя
// { - перенести на 2ю строку по рекомендациям code style от microsoft (если не принято иных)
class Class2 {
// Синхронизация не нужна, если убрать метод Init, а Create вызвать в статическом конструкторе
private readonly object _sync = new object();
// _ht - дать осмысленное название
// Судя по использованию, value может быть int`ом. Не зачем иметь лишний boxing и проверки на тип
// _ht статический, значит к нему могут быть обращения из разных потоков, лучше сделать его ConcurrentDictionary
// Прям сходу не могу сказать, но, возможно, использовал бы какой то другой тип Dictionary <key, key, val> (самописный или существующий), кажется, так было бы быстрее чем массив в ключе
private static Dictionary<int[], object> _ht;
// nit: хотелось бы имена со смыслом
public int GetValue(int index, int index2)
{
// Лишний метод, удалить. Create вызовем в static конструкторе
Init();
// Если ключ у нас объект, то необходимо реализовать IEqualityComparer для этого Dictionary (иначе не понятно как по нему искать)
var key = new[] {index, index2};
// Проверка на тип не нужна, Dictionary сделаем типа int
if (_ht.ContainsKey(key) & _ht[key].GetType() == typeof(int))
// приведение типов больше не нужно
return ((int)_ht[key]);
// nit: else не обязателен
else
// int не может быть null, будет ошибка, вернуть либо default, либо возвращаемое значение должно быть int?
return null;
}
// Метод удалить, вызовем Create в статическом конструкторе без lock
public void Init()
{
if (_ht == null)
lock (_sync)
Create();
}
// Нет смысла делать метод public, сделать private
public static void Create()
{
// nit: и так видно какой тип создаём, можно использовать var
// Обернуть в using
DataProvider provider = new DataProvider();
// Тут следует инициализировать значение _ht, т.к. ранее оно нигде не создаётся
// Не забыть передать реализацию IEqualityComparer в конструктор
// nit: хотелось бы видеть использование фигурных скобок (если не принят иной code style)
// nit: вместо int можно var
// i и j, похоже, несут какой то смысл, можно попробовать придумать нормальное название (иначе не понятно почему 100 и 12, их можно в константы класса)
// nit: возможно можно использовать Parallel.ForEach
for (int i = 0; i < 100; i++)
for (int j = 1; j <= 12; j++)
_ht[new [] { i, j }] = provider.LongRunningCalculation(i, j);
}
}public interface IDataProvider : IDisposable
{
int LongRunningCalculation(int firstValue, int secondValue);
}
public sealed class DataProvider : IDataProvider
{
public extern int LongRunningCalculation(int firstValue, int secondValue);
public extern void Dispose();
}
public sealed class DataProviderService
{
public DataProviderService(IDataProvider dataProvider)
{
_dataProvider = dataProvider;
}
private static readonly ConcurrentDictionary<int[], int?> _calculatedCache = new ConcurrentDictionary<int[], int?>(new CalculatedEqualityComparer());
private readonly IDataProvider _dataProvider;
public int? GetValue(int firstValue, int secondValue)
{
var isNotSupportedValues = firstValue > 100 || firstValue < 0 || secondValue < 1 || secondValue > 12;
if (isNotSupportedValues)
{
return null;
}
var key = new[] { firstValue, secondValue };
if (!_calculatedCache.TryGetValue(key, out var result))
{
result = _dataProvider.LongRunningCalculation(firstValue, secondValue);
_calculatedCache.TryAdd(key, result);
}
return result;
}
}
public extern int LongRunningCalculation(int value, int value2);
public int GetValue(int index, int index2)
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
//EXTERNAL CODE
//Приведен только для справки, никак нельзя менять, код обфусцирован, исходников нет
public sealed class ExternalDataProvider : IDisposable
{
public extern int LongRunningCalculation(int value, int value2);
public extern void Dispose();
}
/// <summary>
/// Объект-значение
/// <para>Базовый класс</para>
/// </summary>
public abstract class ValueObject<T> where T : ValueObject<T>
{
public static bool operator ==(ValueObject<T> value1, ValueObject<T> value2)
{
if ((object)value1 == null)
return (object)value2 == null;
return value1.Equals(value2);
}
public static bool operator !=(ValueObject<T> value1, ValueObject<T> value2)
{
return !(value1 == value2);
}
public override bool Equals(object obj)
{
var valueObject = obj as T;
if (ReferenceEquals(valueObject, null))
return false;
return CompareValues(valueObject);
}
public override int GetHashCode()
{
return GetValueHashCode();
}
/// <summary>
/// Получить HashCode
/// </summary>
/// <returns></returns>
protected abstract int GetValueHashCode();
/// <summary>
/// Сравнить значения
/// </summary>
/// <returns></returns>
protected abstract bool CompareValues(T other);
}
/// <summary>
/// Параметр для DataProvider
/// </summary>
public class Parameter : ValueObject<Parameter>
{
private Parameter()
{
FirstValue = -1;
SecondValue = -1;
}
public Parameter(int firstValue, int secondValue)
{
if (firstValue > 100 || firstValue < 0 || secondValue < 1 || secondValue > 12)
throw new ArgumentException("Is not supported values");
FirstValue = firstValue;
SecondValue = secondValue;
}
/// <summary>
/// Первый параметр
/// </summary>
public int FirstValue { get; }
/// <summary>
/// Второй параметр
/// </summary>
public int SecondValue { get; }
/// <summary>
/// Default value
/// </summary>
public static readonly Parameter Empty = new Parameter();
protected override bool CompareValues(Parameter other)
{
return FirstValue == other.FirstValue && SecondValue == other.SecondValue;
}
protected override int GetValueHashCode()
{
return (FirstValue, SecondValue).GetHashCode();
}
}
/// <summary>
/// Результат для DataProvider
/// </summary>
public class Result : ValueObject<Result>
{
private Result()
{
// Default value
Value = default;
}
public Result(int value)
{
Value = value;
}
/// <summary>
/// Результат
/// </summary>
public int Value { get; }
/// <summary>
/// Default value
/// </summary>
public static readonly Result Empty = new Result();
protected override bool CompareValues(Result other)
{
return Value == other.Value;
}
protected override int GetValueHashCode()
{
return Value.GetHashCode();
}
}
/// <summary>
/// Интерфейс DataProvider
/// </summary>
public interface IDataProvider
{
public Result GetResult(Parameter parameter);
}
/// <summary>
/// Получить данные из ExternalDataProvider без кеша
/// </summary>
public class DataProvider : IDataProvider
{
public Result GetResult(Parameter parameter)
{
using var externalDataProvider = new ExternalDataProvider();
return new Result (externalDataProvider.LongRunningCalculation(parameter.FirstValue, parameter.SecondValue));
}
}
/// <summary>
/// DataProvider с кешированием данных
/// Реализация кеша не имеет принципиального значения
/// </summary>
public class CacheDataProvider : IDataProvider
{
private readonly IDataProvider dataProvider;
private readonly ConcurrentDictionary<Parameter, Result> cache = new();
public CacheDataProvider(IDataProvider dataProvider)
{
this.dataProvider = dataProvider;
}
public Result GetResult(Parameter parameter)
{
return cache.GetOrAdd(parameter, (p => dataProvider.GetResult(p)));
}
}/// <summary>
/// Project status
/// </summary>
public enum ProjectStatus
{
NotStarted,
Active,
Completed
}/// <summary>
/// Get project by id
/// </summary>
/// <param name="id">Id of project</param>
/// <param name="cancellationToken"></param>
/// <returns><see cref="ProjectResponse"/></returns>
[HttpGet]
[Route("projects/{id}")]
[ProducesResponseType(typeof(ProjectResponse), 200)]
[ProducesResponseType(400)]
[ProducesResponseType(500)]
public async Task<ProjectResponse> GetProject(long id, CancellationToken cancellationToken)
{
return await _projectService.GetByIdAsync(id, cancellationToken);
}using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading.Tasks;
using System.Threading;
//EXTERNAL CODE
//Приведен только для справки, никак нельзя менять, код обфусцирован, исходников нет
public sealed class DataProvider : IDisposable
{
public int LongRunningCalculation(int value, int value2)
{
Thread.Sleep(200); // milliseconds
if ((value + value2) % 3 == 0) // 1 of 3 throws exceptions
throw new ArgumentNullException();
else
return value + value2;
}
public void Dispose() { }
}
public class Calculator : IDisposable
{
private readonly Range rangeI = 0..10;
private readonly Range rangeJ = 1..10;
private readonly IDataProvider dataProvider;
private readonly ConcurrentDictionary<Key, int> results = new();
private readonly ConcurrentDictionary<Key, Exception> exceptions = new();
public Calculator(DataProvider externProvider)
{
dataProvider = new CustomProvider(externProvider);
}
public async Task Calculate() /// async!!!
{
var tasks = new ConcurrentBag<Task>();
for (var i = 0; i < rangeI.End.Value - rangeI.Start.Value; i++)
for (var j = 0; j < rangeJ.End.Value - rangeJ.Start.Value; j++)
{
var key = new Key(i, j);
tasks.Add(Task.Run(async () =>
{
try //try-get
{
var result = await dataProvider.LongRunningCalculationAsync(key.FirstValue, key.SecondValue);
results.TryAdd(key, result);
}
catch (Exception ex) // try-get
{
exceptions.TryAdd(key, ex);
}
}));
}
await Task.WhenAll(tasks);
}
public int GetValue(int first, int second)
{
if (results.TryGetValue(new Key(first, second), out int value))
{
return value;
}
else
{
if(exceptions.TryGetValue(new Key(first, second), out Exception ex))
throw ex;
}
return 0;
}
public void Dispose() // обязательно реализовать
{
if (dataProvider != null) // проверка на null
dataProvider.Dispose();
}
}
public interface IDataProvider : IDisposable
{
Task<int> LongRunningCalculationAsync(int firstValue, int secondValue);
}
public class CustomProvider : IDataProvider
{
private readonly DataProvider _provider;
public CustomProvider(DataProvider provider)
{
_provider = provider;
}
public void Dispose()
{
if (_provider != null)
_provider.Dispose();
}
public Task<int> LongRunningCalculationAsync(int firstValue, int secondValue)
{
var tsc = new TaskCompletionSource<int>();
Task.Run(() =>
{
try
{
var result = _provider.LongRunningCalculation(firstValue, secondValue);
tsc.SetResult(result);
}
catch (Exception e)
{
tsc.SetException(e);
}
});
return tsc.Task;
// no, because Exception
return Task.Run(() => _provider.LongRunningCalculation(firstValue, secondValue));
}
}
readonly public struct Key // struct immutable
{
public readonly int FirstValue;
public readonly int SecondValue;
public Key(int first, int second)
{
FirstValue = first;
SecondValue = second;
}
public override int GetHashCode() // override !!!
{
return (FirstValue.GetHashCode() * 1_000_000 + SecondValue.GetHashCode()) % 1_000_000_000;
}
public override bool Equals(object obj) // override !!!
{
var other = (Key)obj;
return GetHashCode() == other.GetHashCode();
}
public override string ToString() // не обязательно, но поскольку у нас ключи закрытые - сделаем
{
return FirstValue.ToString() + " " + SecondValue.ToString();
}
}
class Program
{
static void Main(string[] args)
{
using var calculator = new Calculator(new DataProvider());
calculator.Calculate().Wait();
try
{
var r = calculator.GetValue(1, 1);
Console.WriteLine("1: 1: " + r);
r = calculator.GetValue(1, 2);
Console.WriteLine("1: 1: " + r);
r = calculator.GetValue(1, 3);
Console.WriteLine("1: 1: " + r);
}
catch (Exception ex)
{
Console.WriteLine(ex.Message);
}
Console.ReadLine();
}
}
Простой
