Start convert from C# to C++. Issue: https://github.com/linksplatform… #80

VasiliyevAD · 2021-11-14T16:11:11Z

#48

Для начала обсуждения.
Пока получается вот так перевести этот файл

cpp/Platform.Converters/CachingConverterDecorator.h

Konard · 2021-11-15T05:16:28Z

cpp/Platform.Converters/CachingConverterDecorator.h

-        private: readonly IDictionary<TSource, TTarget> *_cache;
-
-        public: CachingConverterDecorator(IConverter<TSource, TTarget> &baseConverter, IDictionary<TSource, TTarget> &cache) { (_baseConverter, _cache) = (baseConverter, cache); }
+        private: std::unique_ptr<IConverter<TSource, TTarget>> _baseConverter;


_baseConverter should be a reference to a callable object.

cpp/Platform.Converters/CachingConverterDecorator.h

Konard · 2021-11-18T18:16:37Z

cpp/Platform.Converters/CachingConverterDecorator.h

    {
-        private: readonly IConverter<TSource, TTarget> *_baseConverter;
-        private: readonly IDictionary<TSource, TTarget> *_cache;
+        private: std::unique_ptr<std::function<TTarget(TSource)>> _baseConverter;


Do not depend directly on std::function type here.
Let the user specify any callable type here.
The is no need to restrict the user with pointer here.
Catching converter can be a container for callable object.

Yes. Forward converter as template parameter.

template<typename F> struct Converter { F base; Converter(F&& base) : base(base) {} } template<typename F> Converter(F) -> Converter<F>;

cpp/Platform.Converters/CachingConverterDecorator.h

Konard · 2021-11-18T18:21:46Z

cpp/Platform.Converters/CachingConverterDecorator.h

 {
    template <typename ...> class CachingConverterDecorator;
-    template <typename TSource, typename TTarget> class CachingConverterDecorator<TSource, TTarget> : public IConverter<TSource, TTarget>
+    template <typename TSource, typename TTarget> class CachingConverterDecorator<TSource, TTarget>


We might have only single function type as a type parameter. TSource and TTarget can be inferred from the callable object type.

@Konard, please, show this magic. There is no way to do this in C++.
In extreme case, use deduction guides.

cpp/Platform.Converters/CachingConverterDecorator.h

uselessgoddess · 2021-11-19T17:12:11Z

cpp/Platform.Converters/IConverter[T].h

 {
    template <typename ...> class IConverter;
    template <typename T> class IConverter<T> : public IConverter<T, T>
-    {
-    public:
-    };
+    { };
 }


Remove this file

cpp/Platform.Converters/IConverter[T].h

cpp/Platform.Converters/CachingConverterDecorator.h

Konard · 2021-12-03T09:41:03Z

cpp/Platform.Converters/CachingConverterDecorator.h

-        public: TTarget Convert(TSource source) { return _cache.GetOrAdd(source, _baseConverter.Convert); }
+        public: auto operator()(auto&& source)
+        {
+            static std::unordered_map<std::decay_t<decltype(source)>> cached;


This std::unordered_map should be a member of the class.

Konard · 2021-12-03T09:41:29Z

cpp/Platform.Converters/CachingConverterDecorator.h

+            }
+            else 
+            {
+                return *cached.insert({source, _baseConverter(source)})


Suggested change

return *cached.insert({source, _baseConverter(source)})

return *cached.insert({ source, _baseConverter(source )})

Konard · 2021-12-19T06:53:44Z

cpp/Platform.Converters/CachingConverterDecorator.h

-    template <typename ...> class CachingConverterDecorator;
-    template <typename TSource, typename TTarget> class CachingConverterDecorator<TSource, TTarget> : public IConverter<TSource, TTarget>
+    template <typename ...> class Cached;
+    template <typename Function> class Cached<Function>


Suggested change

template <typename Function> class Cached<Function>

template <typename TFunction, typename TSource, typename TTarget> class Cached<Function>

We should add these types back to make it possible to store unordered_map as a class member.

uselessgoddess · 2022-01-09T19:19:24Z

cpp/Platform.Converters/CachingConverterDecorator.h

+    public: TTarget operator()(const TSource& source) {
+        if (auto cursor = _cache.find(source); cursor != _cache.end())


Suggested change

public: TTarget operator()(const TSource& source) {

if (auto cursor = _cache.find(source); cursor != _cache.end())

public: TTarget operator()(const TSource& source)

{

if (auto cursor = _cache.find(source); cursor != _cache.end())

uselessgoddess · 2022-01-09T19:19:50Z

cpp/Platform.Converters/CachingConverterDecorator.h

+#pragma once
+#include <functional>


Suggested change

#pragma once

#include <functional>

#pragma once

#include <functional>

poul250 · 2022-01-10T22:36:38Z

cpp/Platform.Converters/CachingConverterDecorator.h

    {
-        private: readonly IConverter<TSource, TTarget> *_baseConverter;
-        private: readonly IDictionary<TSource, TTarget> *_cache;
+    private: std::unique_ptr<std::function<TTarget(TSource)>> _baseConverter;


Looks like unique_ptr is superfluous here

private: std::function<TTarget(TSource)>> _baseConverter;

poul250 · 2022-01-10T22:37:47Z

cpp/Platform.Converters/CachingConverterDecorator.h

-        private: readonly IDictionary<TSource, TTarget> *_cache;
+    private: std::unique_ptr<std::function<TTarget(TSource)>> _baseConverter;
+
+    private: std::unordered_map<TSource, TTarget> _cache;


nit: Definitely need to accept an allocator in template parameters

poul250 · 2022-01-10T22:53:30Z

cpp/Platform.Converters/CachingConverterDecorator.h


-        public: CachingConverterDecorator(IConverter<TSource, TTarget> &baseConverter) : this(baseConverter, Dictionary<TSource, TTarget>()) { }
+    public: Cached(std::function<TTarget(TSource)>& baseConverter) : _baseConverter(baseConverter), _cache() {};


manual default cache initialization is optional here unnecessary, it can be removed

public: Cached(std::function<TTarget(TSource)> baseConverter) : _baseConverter(std::move(baseConverter)) {}

poul250 · 2022-01-10T23:02:14Z

cpp/Platform.Converters/CachingConverterDecorator.h


-        public: TTarget Convert(TSource source) { return _cache.GetOrAdd(source, _baseConverter.Convert); }
+    public: TTarget operator()(const TSource& source) {


If TTarget is a heavy object, it may be unnecessary copies here. Maybe worth It to return const TTarget& here, but it's better to ask @Konard

@poul250 Oh my God, return isn't move?

poul250 · 2022-01-10T23:04:55Z

cpp/Platform.Converters/CachingConverterDecorator.h


-        public: TTarget Convert(TSource source) { return _cache.GetOrAdd(source, _baseConverter.Convert); }
+    public: TTarget operator()(const TSource& source) {


Add overloading for rvalues here

public: TTarget operator()(TSource&& source)

poul250 · 2022-01-10T23:10:09Z

cpp/Platform.Converters/CachingConverterDecorator.h

+    public: TTarget operator()(const TSource& source) {
+        if (auto cursor = _cache.find(source); cursor != _cache.end())
+        {
+            return *cursor;


return cursor->second;

FreePhoenix888 · 2022-01-11T05:13:49Z

@poul250 It is better to use Suggestion feature instead of raw example to see what changes you suggest :)

Konard

Is it possible to let user decide how function and cache are stored?

Konard · 2022-01-13T18:43:45Z

cpp/Platform.Converters/CachingConverterDecorator.h


-        public: CachingConverterDecorator(IConverter<TSource, TTarget> &baseConverter, IDictionary<TSource, TTarget> &cache) { (_baseConverter, _cache) = (baseConverter, cache); }
+    private: std::unordered_map<TSource, TTarget> _cache;


Suggested change

private: std::unordered_map<TSource, TTarget> _cache;

private: TCache& _cache;

Konard · 2022-01-13T18:51:59Z

cpp/Platform.Converters/CachingConverterDecorator.h

    {
-        private: readonly IConverter<TSource, TTarget> *_baseConverter;
-        private: readonly IDictionary<TSource, TTarget> *_cache;
+    private: std::function<TTarget(TSource) > _baseConverter;


Suggested change

private: std::function<TTarget(TSource) > _baseConverter;

private: std::function<TTarget(TSource)>& _cachedFunction;

uselessgoddess · 2022-01-15T13:27:06Z

Is it possible to let user decide how function and cache are stored?

@Konard use ideas from our friend's code https://godbolt.org/z/jW4dG6f7K

uselessgoddess · 2022-01-18T18:25:25Z

cpp/Platform.Converters/CachingConverterDecorator.h

+        TTarget&& operator()(TSource&& source) &&
+        {
+            return std::move(CachedCall(_cache, _cachedFunction, std::move(source)));
+        }
+
+        TTarget&& operator()(const TSource& source) &&
+        {
+            return std::move(CachedCall(_cache, _cachedFunction, source));
        }


Suggested change

TTarget&& operator()(TSource&& source) &&

{

return std::move(CachedCall(_cache, _cachedFunction, std::move(source)));

}

TTarget&& operator()(const TSource& source) &&

{

return std::move(CachedCall(_cache, _cachedFunction, source));

}

TTarget operator()(TSource&& source) &&

{

return _cachedFunction(std::move(source));

}

TTarget operator()(const TSource& source) &&

{

return _cachedFunction(source);

}

cpp/Platform.Converters/CachingConverterDecorator.h

Co-authored-by: uselessgoddess <[email protected]>

VasiliyevAD self-assigned this Nov 14, 2021

Konard requested changes Nov 14, 2021

View reviewed changes

Konard requested changes Nov 15, 2021

View reviewed changes

Konard reviewed Nov 15, 2021

View reviewed changes

cpp/Platform.Converters/CachingConverterDecorator.h Outdated Show resolved Hide resolved

Konard requested changes Nov 18, 2021

View reviewed changes

uselessgoddess reviewed Nov 19, 2021

View reviewed changes

cpp/Platform.Converters/CachingConverterDecorator.h Outdated Show resolved Hide resolved

uselessgoddess reviewed Nov 19, 2021

View reviewed changes

cpp/Platform.Converters/CachingConverterDecorator.h Outdated Show resolved Hide resolved

uselessgoddess reviewed Nov 19, 2021

View reviewed changes

Konard reviewed Nov 19, 2021

View reviewed changes

cpp/Platform.Converters/IConverter[T].h Outdated Show resolved Hide resolved

Start convert from C# to C++. Issue: linksplatform#48

48a377f

VasiliyevAD force-pushed the 48_translate branch from 8bfb237 to 48a377f Compare November 19, 2021 19:31

FreePhoenix888 self-requested a review November 24, 2021 08:29

FreePhoenix888 approved these changes Nov 24, 2021

View reviewed changes

cpp/Platform.Converters/CachingConverterDecorator.h Outdated Show resolved Hide resolved

Dynamic target and source types

9374f1b

Konard requested changes Dec 3, 2021

View reviewed changes

Konard requested changes Dec 19, 2021

View reviewed changes

flakeed added 2 commits January 1, 2022 20:43

Update CachingConverterDecorator.h

d41c312

Update CachingConverterDecorator.h

0e2010c

uselessgoddess reviewed Jan 9, 2022

View reviewed changes

flakeed added 6 commits January 10, 2022 20:58

Update IConverter[TSource, TTarget].h

acb77e3

Update IConverter[T].h

23af059

Update ConverterBase.h

11294e0

Update UncheckedConverter.h

b73946d

Update CheckedConverter.h

d9c78b7

Update UncheckedSignExtendingConverter.h

7243e54

poul250 reviewed Jan 10, 2022

View reviewed changes

Update CachingConverterDecorator.h

c040357

Konard requested changes Jan 13, 2022

View reviewed changes

Update CachingConverterDecorator.h

21137b6

Update CachingConverterDecorator.h

09f490f

uselessgoddess reviewed Jan 18, 2022

View reviewed changes

Update CachingConverterDecorator.h

4c27a42

uselessgoddess reviewed Jan 21, 2022

View reviewed changes

cpp/Platform.Converters/CachingConverterDecorator.h Outdated Show resolved Hide resolved

FreePhoenix888 changed the base branch from master to main March 27, 2022 07:14

FreePhoenix888 mentioned this pull request Mar 27, 2022

Change organization pull requests base branch from master to main linksplatform/Data.Doublets#340

Closed

Delete _cachedFunction(source)

08d5b1e

Co-authored-by: uselessgoddess <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start convert from C# to C++. Issue: https://github.com/linksplatform… #80

Start convert from C# to C++. Issue: https://github.com/linksplatform… #80

VasiliyevAD commented Nov 14, 2021

Konard Nov 15, 2021

Konard Nov 18, 2021

uselessgoddess Nov 19, 2021 •

edited

Loading

Konard Nov 18, 2021

uselessgoddess Nov 19, 2021 •

edited

Loading

uselessgoddess Nov 19, 2021 •

edited

Loading

Konard Dec 3, 2021

Konard Dec 3, 2021

Konard Dec 19, 2021

uselessgoddess Jan 9, 2022

uselessgoddess Jan 9, 2022

poul250 Jan 10, 2022

poul250 Jan 10, 2022

poul250 Jan 10, 2022

poul250 Jan 10, 2022

uselessgoddess Jan 11, 2022 •

edited

Loading

poul250 Jan 10, 2022

poul250 Jan 10, 2022

FreePhoenix888 commented Jan 11, 2022

Konard left a comment

Konard Jan 13, 2022

Konard Jan 13, 2022

uselessgoddess commented Jan 15, 2022

uselessgoddess Jan 18, 2022

	return *cached.insert({source, _baseConverter(source)})
	return *cached.insert({ source, _baseConverter(source )})

	template <typename Function> class Cached<Function>
	template <typename TFunction, typename TSource, typename TTarget> class Cached<Function>

		public: TTarget operator()(const TSource& source) {
		if (auto cursor = _cache.find(source); cursor != _cache.end())


		public: CachingConverterDecorator(IConverter<TSource, TTarget> &baseConverter) : this(baseConverter, Dictionary<TSource, TTarget>()) { }
		public: Cached(std::function<TTarget(TSource)>& baseConverter) : _baseConverter(baseConverter), _cache() {};


		public: TTarget Convert(TSource source) { return _cache.GetOrAdd(source, _baseConverter.Convert); }
		public: TTarget operator()(const TSource& source) {


		public: CachingConverterDecorator(IConverter<TSource, TTarget> &baseConverter, IDictionary<TSource, TTarget> &cache) { (_baseConverter, _cache) = (baseConverter, cache); }
		private: std::unordered_map<TSource, TTarget> _cache;

	private: std::unordered_map<TSource, TTarget> _cache;
	private: TCache& _cache;

	private: std::function<TTarget(TSource) > _baseConverter;
	private: std::function<TTarget(TSource)>& _cachedFunction;

Start convert from C# to C++. Issue: https://github.com/linksplatform… #80

Are you sure you want to change the base?

Start convert from C# to C++. Issue: https://github.com/linksplatform… #80

Conversation

VasiliyevAD commented Nov 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uselessgoddess Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uselessgoddess Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

uselessgoddess Nov 19, 2021 • edited Loading

Choose a reason for hiding this comment

Remove this file

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uselessgoddess Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FreePhoenix888 commented Jan 11, 2022

Konard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uselessgoddess commented Jan 15, 2022

Choose a reason for hiding this comment

uselessgoddess Nov 19, 2021 •

edited

Loading

uselessgoddess Nov 19, 2021 •

edited

Loading

uselessgoddess Nov 19, 2021 •

edited

Loading

uselessgoddess Jan 11, 2022 •

edited

Loading