Indigo icon indicating copy to clipboard operation
Indigo copied to clipboard

core: Replace text-based `ArrayOutput` with `StringOutput`

Open mkviatkovskii opened this issue 3 years ago • 2 comments

Motivation Currently Indigo uses own-written implementation of strings using class Array<char> (Array is similar to std::vector). There is no need in reinventing the wheel, since std::string can do all the same things.

ToDo

  1. Find all places in code that use ArrayOutput
  2. Check if it works with strings, not some binary arrays (not using Output::writeBinaryInt etc).
  3. Replace them with StringOutput.
  4. Since ArrayOutput accepts Array<char> in constructor, it's also required to replace that Array<char> with std::string.

It's better to create small pull requests, like 1 per modified class using ArrayOutput.

mkviatkovskii avatar Nov 05 '21 10:11 mkviatkovskii

it's c++ implementation right?

On Fri, 5 Nov 2021 at 10:48, Mikhail Kviatkovskii @.***> wrote:

Motivation Currently Indigo uses own-written implementation of strings using class Array (Array is similar to std::vector). There is no need in reinventing the wheel, since std::string can do all the same things.

ToDo

  1. Find all places in code that use ArrayOutput
  2. Check if it works with strings, not some binary arrays (not using Output::writeBinaryInt etc).
  3. Replace them with StringOutput.
  4. Since ArrayOutput accepts Array in constructor, it's also required to replace that Array with std::string.

It's better to create small pull requests, like 1 per modified class using ArrayOutput.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/epam/Indigo/issues/511, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHJOHYQNLNW66M2ZA735FTUKOY7VANCNFSM5HNRVOHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

MysterionRise avatar Nov 05 '21 11:11 MysterionRise

@MysterionRise, yes, it's deep in C++ core.

mkviatkovskii avatar Nov 08 '21 10:11 mkviatkovskii