Wątek przeniesiony 2020-11-30 19:16 z C# i .NET przez somekind.

Properties Comparator - prośba o kostruktywną krytykę

0

cześć
napisałem sobie prostą biblioteczkę do porównywania zbioru obiektów tego samego typu, gdy nie ma zdefiniowanego klucza
Użytkownik definiuje klucz, i to co chce porównać i porównuje.
W rezultacie dostaje raporty porównywania mówiące - czy coś zostało usunięte, dodane, co zostało zmienione

Porównywane mogą być publiczne property jeśli są to

  • prymitywy
  • enumy
  • impelemtują IEqutable

Działa w trybie sync i async

Na razie jako konfiguracja jest opcja podania trybu porownywania stringów

Api - ptzyznam ze mi się podoba, bo to jest taki step builder, który wymusza kolejność dodawania argumentów

//sync
 PropertiesComparator.Before(before).After(after).Keys(keys).Compares(compares).Run();

//async
 PropertiesComparator.Before(before).After(after).Keys(keys).Compares(compares).RunAsync();

//string compare mode
 PropertiesComparator
               .Before(before)
               .After(after)
               .Keys(new[] { "Prop1" })
               .Compares(new[] { "Prop2" })
               .ComparatorSettings(new PropertiesComparer.Settings.ComparatorSettings { StringComparisonType = StringComparison.InvariantCultureIgnoreCase })
               .Run();

Z góry dziękuję za opinie

https://github.com/musselek/PropertiesComparator

1

Czy to ma być fluent api? Bo mi to tak nie wygląda. Bardziej coś takiego bym widział:

var result = Compare(before).With(after).UsingKeys(keys).UsingProperties(properties).RunSync();

1

Ten Async w tym przypadku nie ma sensu.

Robienie metody async w bibliotece, która pod spodem odpala tylko Task.Run na operacji CPU-bound to antypattern.

2

A, że u Ciebie before nazywa się source, a target to after? No jest to jakiś powiew nowości, ale nie jestem pewien, czy tędy droga. Ja bym jednak trzymał się jakichś konwencji, do których ludzie przywykli.
To API na stringach to z kolei takie oldschoolowe, no ale jest 2020, a nie 2002, to powinny być lambdy raczej.

Fajnie, gdybyś uzupełnił ten przykład z GitHuba o jego wynik, żeby nie musieć się zastanawiać.

Ogólnie spodziewałbym się raczej czegoś w tym stylu: https://fluentassertions.com/objectgraphs/

0

@somekind: dzięki za przykład
Jeśli dobrze zrozumiałem to chodzi o opcje porównania?
czy ogólnie o API

@some_ONE dzięki za odpowiedź, ale czy możesz jaśniej, nawet jak krowie na rowie
ja nie jestem super biegły w concurency w .net, mogę patrzeć a nie widzieć, próbowałem szukać na własną rękę

i to co znalazłem
https://blog.stephencleary.com/2013/10/taskrun-etiquette-and-proper-usage.html

First, let’s think of what Task.Run is really for (you wouldn’t want to use a fork to eat soup!). The purpose of Task.Run is to execute CPU-bound code in an asynchronous way. Task.Run does this by executing the method on a thread pool thread and returning a Task representing the completion of that method.

https://app.pluralsight.com/guides/using-task-run-async-await

The short explanation is that launching an operation on a separate thread via Task.Run is mainly useful for CPU-bound operations, not I/O-bound operations.

Na mój rozumek wydaje mi się że jest OK, ale jak wspomniałem, mogę patrzeć a nie widzieć dlatego proszę o wskazanie palcem nawet :)

Edit
@some_ONE

tak jeszcze raz przeczytałem co napisałeś
czy chodzi o to, że API powinno zwracać wartość - a jak tam już będzie w środku to inna para kaloszy, czyli chodzi aby TASKI nie wypływały na zewnątrz?

0
mussel napisał(a):

@somekind: dzięki za przykład

Jeśli dobrze zrozumiałem to chodzi o opcje porównania?
czy ogólnie o API

Chodzi ogólnie o to, żeby było intuicyjne, czyli np. żeby nie musieć podawać nazw właściwości w stringach.

czy chodzi o to, że API powinno zwracać wartość - a jak tam już będzie w środku to inna para kaloszy, czyli chodzi aby TASKI nie wypływały na zewnątrz?

A gdzie jest zysk z użycia async przy porównywaniu obiektów w pamięci?

0

@somekind:

Chodzi ogólnie o to, żeby było intuicyjne, czyli np. żeby nie musieć podawać nazw właściwości w stringach.
to pisałem pod kontrolke która jest i zwraca właśnie 3xstring[] - z czego 2 sa potrzebne

sam myślałem jak udekorować PropertyInfo[] aby mieć w tym wszystko zawarte, ale nie wymysliłem niczego lepszego, a jak kontrolka juz coś zwraca to tego uzyłem.

A gdzie jest zysk z użycia async przy porównywaniu obiektów w pamięci?

może jeszcze dużo nie rozumiem, ale to co chialem zrobić to zrównoleglić proces,
czyli jak rozumiem async/await bardziej nadaje się do procesów I/O bound? oczekujemy na odpowiedź, plik etc.?
a do cbu bound?
background workers?

przepraszam, że się tak dopytuje - ale może za głupi jestem - patrzę się na takie coś
https://docs.microsoft.com/en-us/dotnet/standard/async-in-depth

i mam taki tekst

async and await are the best practice for managing CPU-bound work when you need responsiveness. There are multiple patterns for using async with CPU-bound work. It's important to note that there is a small cost to using async and it's not recommended for tight loops. It's up to you to determine how you write your code around this new capability.

ok - mogę się zgodzić, że nie rozumiem, źle rozumiem, nie ograniam kontekstu itd tego co jest napisane - ale wydaje mi się, że jeśli jako zysk chce mieć responsywność to chyba(?) jest to dobra droga

EDIT

znalazłem jeszcze takie coś

Now CPU vs IO. IO is almost always handled outside the CPU proper and therefore can be run in parallel with the CPU work. Hence IO bound work is a good candidate for async operations. CPU bound work is a different story. If you have CPU heavy work and you move it to a task then you are now taxing the CPU more because you have the CPU task work and whatever work you're continuing to do. In a multi-core machine this is fine but on a single core processor you'll have to workloads fighting for the same CPU. Hence CPU bound work can be done in a task and many times it is fine you simply don't want to go over the # of cores available to the system. As a result you wouldn't want to create a bunch of CPU bound tasks that run at the same time. They'd just be fighting for CPU time. But having a CPU bound task, in general, is fine.

czyli jak rozumiem najlepiej żeby ewentualnie zrównoleglał to klient biblioteki, a nie sama biblioteka?

EDIT2
coś znalazłem
https://igorpopov.io/2018/06/30/asynchronous-programming-in-csharp/

The essential information is this:
I/O-bound operations? - then await asynchronous I/O methods provided by the .NET framework
CPU bound operations? - then expose it as a synchronous/plain implementation, the caller/user of the operation will wrap it in a Task.Run if needed

czyli - jeśli chciałbym cos zrównoleglić w bibliotece?
to mogę?

1
mussel napisał(a):

sam myślałem jak udekorować PropertyInfo[] aby mieć w tym wszystko zawarte, ale nie wymysliłem niczego lepszego, a jak kontrolka juz coś zwraca to tego uzyłem.

Czemu chcesz coś dekorować? Chodzi mi o API w stylu Keys<T>(Expression<Func<T, object>> keyPointer), żeby nie trzeba było używać stringów, w których można zrobić literówki.
Ale ok, jeśli źródłem danych jest jakieś GUI, to rozumiem pierwotną przyczynę i to ma sens w tym przypadku, ale z punktu widzenia potencjalnego użytkownika biblioteki to jest dziwne. W sensie biblioteka ta nie będzie wygodna w ogólnym przypadku.

może jeszcze dużo nie rozumiem, ale to co chialem zrobić to zrównoleglić proces,

Dodanie async ani Task.Run() niczego nie zrównolegla.

ok - mogę się zgodzić, że nie rozumiem, źle rozumiem, nie ograniam kontekstu itd tego co jest napisane - ale wydaje mi się, że jeśli jako zysk chce mieć responsywność to chyba(?) jest to dobra droga

Czy teraz brakuje Ci responsywności? W jakiej konkretnie sytuacji? Jak duże obiekty/kolekcje porównujesz i ile czasu to zajmuje?

0

ok - mogę się zgodzić, że nie rozumiem, źle rozumiem, nie ograniam kontekstu itd tego co jest napisane - ale wydaje mi się, że jeśli jako zysk chce mieć responsywność to chyba(?) jest to dobra droga

Dużo aplikacji ma tylko jeden główny wątek np UI w desktopie i domyślnie każda operacja jest blokująca, tak więc jeśli masz akcje zabierające powyżej `00 milisekund na klinięcie, warto by było puszczać je w osobnym watku, bo tak będzie płynniej.

Odnośnie operacji w pamieci, to równoleglość kosztuje czas na synchronizacje, i poniżej pewnego progu się nie opłaca, wiec jeśli te komparatory nie robią jakiś mega cieżkich zadań na sekundy, to się nie opłaca, a nawet jeśli tak, to warto było by sie dwa razy zastanowić, czy wszystko jest dobrze, bo pamieć dzieli sie problematycznie miedzy wątkami, i mozę nie dokonca wyjśc po Twojej myśli.

Expression to fajny pomysł, bo wtedy można wybierać property lamdą, dotego mógłbyś dać params na argumenty żeby uniknąć brzydkiego new. Przy params trzeba sie dwa razy zastanoci, czy nie będzie jakieś kolizji przy przeciązaniu :P

0

bardzo dziękuję za wszystkie rady, przydały się.
A projekt został wzbogacony o budowniczego komparatorów.
Wzorowalem się na Fluent Validatorze i wysżło takie coś jak ponizej

można sobie pomanipulować danymi wejściowymi, a pod spodem uruchamia się komparator już dla list

public class Complex
{
    public List<string> Prop1 { get; set; }
    public List<int> Prop2 { get; set; }
    public string[] Prop3 { get; set; }
}

public class ComplexComparer : ComparatorComposer<Complex>
{
    public ComplexComparer()
    {
        Compare(x => x.Prop1).PrepareBoth((x, y) => (x.Select(x => $"{x}_ch1").ToList(), y.Select(y => $"{y}_ch2").ToList())).WithKeys(x => new[] { "a", "b" }).ToCompare(x => new[] { "a", "b" }).AddDelete();
        Compare(x => x.Prop2).WithKeys(x => new[] { "a", "b" }).ToCompare(x => new[] { "a", "b" }).ComparatorSettings(null).AllChanges();
        Compare(x => x.Prop3).PrepareBefore(x=>x).WithKeys(x => new[] { "a", "b" }).ToCompare(x => new[] { "a", "b" }).ComparatorSettings(null).AllChanges();
    }
}
        
var c1 = new Complex { Prop1 = new List<string> { "a", "b" }, Prop2 = new List<int> { 1, 2 } };
var c2 = new Complex { Prop1 = new List<string> { "a1", "b1" }, Prop2 = new List<int> { 11, 21 } };

var complexComparer = new ComplexComparer();

foreach (var cr in complexComparer.ComparisonResults(c1, c2))
{
    //do sth with report
}
1

Nie wiem w sumie czemu się na FluentValidatorze wzorować, konieczność pisania nowej klasy do każdego porównania brzmi dla mnie niezbyt wygodnie. API też wydaje się niezrozumiałe, te wszystkie: PrepareBoth, AddDelete, na zdrowy rozum ciężko odgadnąć, co taka metoda robi w takiej bibliotece.

Ja bym raczej widział API w rodzaju tego z FluentAssertions, które linkowałem wyżej:

var result = beforeCollection.CompareWith(afterCollection)
    .ByKey(x => x.Prop1, x => x.Prop2)
    .Exclude(x => x.IDontCare1)
    .Exclude(x => x.IDontCare2)
    .ExcludeMembers(o => o.EndsWith("Date"))
    .IgnoreFields()
    .Customize(x => x.SomeString, o => o.WithStringComparisionOptions(StringComparison.OrdinalIgnoreCase))
    .Customize(x => x.SomeDouble, o => o.Round(2, MidpointRounding.AwayFromZero))
    .Run();

Dałbym też opcję, żeby zamiast Exclude, które wyklucza jakieś właściwości/pola z porównania, można było definiować Include, i wówczas sposób działania by się zmieniał, tzn. tylko właściwości/pola wskazane w ten sposób byłyby porównywane, a reszta ignorowana.

0

@somekind:
dzięki za przykład

gwoli ścisłości - to co dodałem to nie zastępuje "starego" API ale jest jako nową funkcjonalnością
Mamy dane po parsowaniu XML i można sobie robić taki komparator
np magazyn bieże coś innego pod uwage, produkcja coś innego, projektanci coś innego itd

przechodząc do rzeczy to tak - na pewno nie da się zrobić jakieś super narzędzia dla każdego, bo jak kluczy mamy 20 to pisać 20xByKey
albo jak klucze przychodzą "z zewnątrz" itp itd

co do samego przykładu
może to głupie, ale chciałbym zobaczyć kod który robi takiego buildera

  1. w cpp jest coś takiego jak varadic templates, z tego co wiem c# nie ma takiej dobroci
    .ByKey(x => x.Prop1, x => x.Prop2) jak takie coś zbudować jak nie poprzez .ByKey<TProp1, TProp2>(x => x.Prop1, x => x.Prop2)
    przy sporej kluczy to już zaczyna się robić problem i zostaje tylko ByKey<TProp> (chyba że czegoś nie wiem)

  2. mamy taki fragment
    beforeCollection.CompareWith(afterCollection)ByKey(x => x.Prop1, x => x.Prop2)
    za koleją List<T>..CompareWith(List<T>).ByKey(T=>T.Prop)

czyli przechodzimy z listy do typu listy - potem już w kodzie aby uruchomić tą funkcję to trzeba skąd pobrać te propertysy
mamy jakiś DoCompare(List<T> before, List<T> after) ale funkcję by Key trzeba będzie wywołać już tak
FunByKey(before[0])// czy tam after - jeden pies

to jest trochę takie zaskakujące w kodzie - niby nic, nie takie rzeczy się widziało, ale jak ktoś to zobaczy to się zdziwi czemu before(after) czemu [0] itd

Konkludując
budowa API to nie jest taka prosta sprawa, tu niby nieskoplikowany projekcik, ot taka se porównywarka, a jak zrobić aby zaspokajała potrrzeby wynikające z różnych możliwych przypadków użycia to już nie takie oczywiste (istnieje możliwość, że tak myślę bo brakuje mi obycia w temacie)

0

1 w C# widziałem głownie dwa rozwiązania. Jedno stosowane nawet przez microsoft, tak wiemy że język ma ograniczenia, dlatego dodaliśmy 16 przeciązeń z coraz wiekszą liczbą parametów, drugie zmiast ByKey(x => x.Prop1, x => x.Prop2) jest coś w stylu ByKey(x => x.Prop1).AndBy.(x => x.Prop2). Pierwszej z pierwszej opcji bym nie krozystał chyba że bardzo dobrze wiesz co robisz, lub coś w gatunku 4 przeciązeń powstało samo nie jako przy okazji, i masz to półdarmo. Dlaczego nie, bo zmiany pomnożone przez 16 przypadków to bardzo duzo pracy.

  1. nie rozumiem, problemu, jeśli masz DoCompare<T>(List<t> before, List<t> after) to czy keySelector nie bedzie zwyczajnie, 'ByKey(T item )' i problem solved?
    Chyba że chodziło Ci że DoCompare<T> gdzie T to `List<someotherType' w takim wypadku możesz napisać metode, ComepareEnumerebale, i problem solved :)
1
mussel napisał(a):

przechodząc do rzeczy to tak - na pewno nie da się zrobić jakieś super narzędzia dla każdego, bo jak kluczy mamy 20 to pisać 20xByKey
albo jak klucze przychodzą "z zewnątrz" itp itd

Nie trzeba, można mieć jedną metodę, która przyjmie 20 kluczy jako string podanych "z zewnątrz". A oprócz tego można mieć API oparte na lambdach, w którym działa podpowiadanie składni, i które jest dzięki temu wygodne dla programisty.

  1. w cpp jest coś takiego jak varadic templates, z tego co wiem c# nie ma takiej dobroci
    .ByKey(x => x.Prop1, x => x.Prop2) jak takie coś zbudować jak nie poprzez .ByKey<TProp1, TProp2>(x => x.Prop1, x => x.Prop2)
    przy sporej kluczy to już zaczyna się robić problem i zostaje tylko ByKey<TProp> (chyba że czegoś nie wiem)

Nie sądzę, aby variadic templates były rozwiązaniem pomogło. Problem nie polega na zmiennej liczbie typów ale na zmiennej liczbie parametrów. Do tego służy słowo kluczowe params.

to jest trochę takie zaskakujące w kodzie - niby nic, nie takie rzeczy się widziało, ale jak ktoś to zobaczy to się zdziwi czemu before(after) czemu [0] itd

W moim przykładzie chodziło mi o ogólną przejrzystość i sugestywne nazwy metod, nie o poprawność składniową.
Generalnie widzę opcje dwie:

  1. albo klasa porównująca z metodami konfigurującymi sposób porównania (na zasadzie buildera), a na końcu jawnym wywołaniem jakiejś metody "Run", która dokona porównania.
    To wygląda tak:
public class CollectionComparer<T>
{
    private IEnumerable<T> beforeCollection;
    private IEnumerable<T> afterCollection;

    public CollectionComparer(IEnumerable<T> beforeCollection, IEnumerable<T> afterCollection)
    {
        this.beforeCollection = beforeCollection;
        this.afterCollection = afterCollection;
    }

    public static MemberInfo GetMemberInfo(LambdaExpression exp)
    {
        var body = exp.Body as MemberExpression;
        return body.Member;
    }

    public CollectionComparer<T> ByKey(params Expression<Func<T, object>>[] keys)
    {
        // todo
        return this;
    }

    public CollectionComparer<T> Exclude(params Expression<Func<T, object>>[] excludedProps)
    {
        // todo
        return this;
    }

    public CollectionComparer<T> IgnoreFields()
    {
        // todo
        return this;
    }

    internal CollectionComparer<T> ExcludeMembers(Func<string, bool> propertyNameCondition)
    {
        // todo
        return this;
    }

    internal ComparisionResult Run()
    {
        // todo
        return new ComparisionResult();
    }
}

Użycie:

var result = new CollectionComparer<Person>(beforeCollection, afterCollection)
    .ByKey(x => x.Name, x => x.LastName)
    .Exclude(x => x.Age, x => x.City)
    .ExcludeMembers(o => o.EndsWith("Date"))
    .IgnoreFields()
    .Run();
  1. albo metoda rozszerzająca IEnumerable<T> przyjmująca w parametrze drugą kolekcję oraz konfigurację porównania (też na zasadzie buildera).
public static class EnumerableExtensions
{
    public static ComparisionResult CompareWith<T>(this IEnumerable<T> before, IEnumerable<T> after, 
        Func<ComparisionOptions<T>, ComparisionOptions<T>> options)
    {
        // todo
        return new ComparisionResult();
    }
}
public class ComparisionOptions<T>
{
    public ComparisionOptions<T> ByKey(params Expression<Func<T, object>>[] keys)
    {
        // todo
        return this;
    }

    public ComparisionOptions<T> Exclude(params Expression<Func<T, object>>[] excludedProps)
    {
        // todo
        return this;
    }

    public ComparisionOptions<T> IgnoreFields()
    {
        // todo
        return this;
    }

    public ComparisionOptions<T> ExcludeMembers(Expression<Func<string, bool>> propertyNameCondition)
    {
        // todo
        return this;
    }
}

Użycie:

var result2 = beforeCollection.CompareWith(afterCollection, options =>
    options.ByKey(x => x.Name, x => x.LastName)
        .Exclude(x => x.Age, x => x.City)
        .ExcludeMembers(o => o.EndsWith("Date"))
        .IgnoreFields()
    );

Konkludując
budowa API to nie jest taka prosta sprawa, tu niby nieskoplikowany projekcik, ot taka se porównywarka, a jak zrobić aby zaspokajała potrrzeby wynikające z różnych możliwych przypadków użycia to już nie takie oczywiste (istnieje możliwość, że tak myślę bo brakuje mi obycia w temacie)

Trudno się nie zgodzić. :)

_flamingAccount napisał(a):

1 w C# widziałem głownie dwa rozwiązania. Jedno stosowane nawet przez microsoft, tak wiemy że język ma ograniczenia, dlatego dodaliśmy 16 przeciązeń z coraz wiekszą liczbą parametów

Zgaduję, że masz na myśli konstruktory od Func i Action, ale tam problemem jest liczba parametrów generycznych.

1 użytkowników online, w tym zalogowanych: 0, gości: 1