merge list of strings into one sorted string So I was tasked the following:

  Given a list of strings, output a sorted string that consists of all
  lowercase characters  (a-z) from each string in the list.

  Example

  [aba, xz] -> aabxz

I tried this:

from collections import Counter
from string import ascii_lowercase
def merge(strings):
    counter = Counter()
    for string in strings:
        for char in string:
            counter[char] +=1

    result = []
    for c in ascii_lowercase:
        if c in counter:
            for _ in range(counter[c]):
                result.append(c)
    return "".join(result)

This works on most test cases, but times out on a couple test cases. I am extremely frustrated. How can I  improve the time complexity of this solution?

counter = Counter()
for string in strings:
    for char in string:
        counter[char] +=1

You are flattening your strings list to count each individual characters. For starter, if you were to extract letters individually, you could feed it to the Counter constructor and avoid the += 1 operation. Second, flattening an iterable is best done using itertools.chain.from_iterable:

counter = Counter(itertools.chain.from_iterable(strings))

result = []
for c in ascii_lowercase:
    if c in counter:
        for _ in range(counter[c]):
            result.append(c)
return "".join(result)

Instead of the inner for loop, you can create repeated sequence of characters using c * counter[c]. This also have the neat advantage to produce the empty string ('') when counter[c] is 0, removing the need for the c in counter test:

return ''.join(c * counter[c] for c in ascii_lowercase)

These changes result in a ×4 speedup on my machine.

My general approach of an algorithm, in readable form. It is in C#, but it should be clear enough to read. And I personally prefer an algorithmic sketch over a 1-liner, for the sake of clarity. Dictionary is the same as hash table, and StringBuilder is like a list of chars (because strings are immutable in C#). I did not test timing.

    public string Count(List<string> strings)
    {
        var mapping = new Dictionary<char, int>();
        for (char c = 'a'; c < 'z'; c++)
        {
            mapping[c] = 0;
        }
        foreach (var str in strings)
        {
            foreach (var ch in str)
            {
                if(Char.IsLower(ch))
                {
                    mapping[ch]++;
                }
            }
        }
        var b = new System.Text.StringBuilder();
        for (char c = 'a'; c < 'z'; c++)
        {
            b.Append(c, mapping[c]); // Append(c, i) : adds c i times
        }
        return b.ToString();
    }


Simple integer range for C++11 range-based for loops I'm really tired of having to type

for (int iSomething = rangeBegin; iSomething < rangeEnd; ++iSomething)
{
   ...
}

whenever I want to iterate over an integer range (most IDEs help with the typing, but still it looks so verbose, naming the integer 3 times!)

I wanted something like this:

for (int iSomething : LoopRange(rangeBegin, rangeEnd))
{
   ...
}

Or if rangeBegin is 0 (the majority of the cases) then a simple

for (int iSomething : LoopRange(rangeEnd))
{
   ...
}

My very simple implementation:

class LoopRangeIterator
{
public:
    LoopRangeIterator(int value_)
        : value(value_){}

    bool operator!=(LoopRangeIterator const& other) const
    {
        return value != other.value;
    }

    int const& operator*() const
    {
        return value;
    }

    LoopRangeIterator& operator++()
    {
        ++value;
        return *this;
    }

private:
    int value;
};

class LoopRange
{
public:
    LoopRange(int from_, int to_)
        : from(from_), to(to_){}

    LoopRange(int to_)
        : from(0), to(to_){}

    LoopRangeIterator begin() const
    {
        return LoopRangeIterator(from);
    }

    LoopRangeIterator end() const
    {
        return LoopRangeIterator(to);
    }

private:
    int const from;
    int const to;
};

I named it LoopRange to make it clear that it's for loops and it isn't some general integer range class that you would use for intersecting or building union etc.

Of course this class could be generalized in many ways, but I think if you need more complex functionality (e.g. custom step sizes, double values), then you are doing something special and you are better off writing the explicit for loop.

What do you think about it?
If I use such a thing throughout my project, would it confuse and disturb/distract people too much compared to just using the classic and verbose for(...; ...; ...) style?

You can make the class template with trivial changes (add template<typename T> and change int by T in your classes), then make a construction function that deduces integer types:

template<typename T>
LoopRange<T> range(T from, T to)
{
    static_assert(std::is_integral<T>::value,
                  "range only accepts integral values");

    return { from, to };
}

That will even allow you to explicitly tell what kind of integer you want to loop with if needed:

for (auto i: range<unsigned>(0, 5))
{
    std::cout << i << " ";
}

If you need to generate indices to iterate through a std::vector, this can be useful since std::vector<T>::size_type is probably bigger than int. While the static_assert avoids some potential problems with floating point values, it also inhibits the use of integer-like classes (for example, a hypothetical BigNum class).

You can simplify some of your functions thanks to list initialization. For example, used in a return statement, it sallows you not to explicitly repeat the return type (unless the return type's constructor is explicit):

LoopRangeIterator begin() const
{
    return { from };
}

LoopRangeIterator end() const
{
    return { to };
}

On a side note, such a range utility would also be interesting if it worked with floating point numbers, and maybe decimal numbers in the future (akin to Python's numpy.arange). However, you would have to special-case the class for those types if you want to avoid problems: if you repeatedly add the same floating point (say 0.01), you will accumulate rounding errors. Computing every value from the base value with a multiplication could be away to circumvent such a problem.

Preprocessor Macros! All of the loops below will produce the same result. Unfortunately, macros can't be defined more than once, so you have to give them different names.

#include <iostream>

#define irange(i,a,b) int i = (a); i < (b); ++i // if you want to use ints all the time
#define range(i,a,b) i = (a); i < (b); ++i      // if you ever want to use something other than an int
#define zrange(i,b) i = 0; i < (b); ++i       // if you want to start at zero
#define izrange(i,b) int i = 0; i < (b); ++i  // if you want to start at zero and use ints

int main ( )
{
    for ( int num = 0; num < 10; ++num ) std::cout << num << ' ';

    std::cout << '\n';

    for ( irange ( num, 0, 10 ) ) std::cout << num << ' ';

    std::cout << '\n';

    for ( int range ( num, 0, 10 ) ) std::cout << num << ' ';

    std::cout << '\n';

    for ( int zrange ( num, 10 ) ) std::cout << num << ' ';

    std::cout << '\n';

    for ( izrange ( num, 10 ) ) std::cout << num << ' ';

    std::cout << '\n';
}

You could also write more macros to do things like increment by something other than 1.


Is this UnitTest for updating an object in data-access layer sensible? I'm still trying to learn how to write good tests (and write testable code).

This is the function I want to verify,

 void IAutoTisDal.UpdateRange(Range range)
  {
     using (var repo = _repositoryFactory.GetRepository(_baseline))
     {
        var rangeqry = from r in repo.Query<Range>()
                       where r.Id == range.Id
                       select r;
        var rangeToUpdate = rangeqry.SingleOrDefault();
        if (rangeToUpdate == null)
           throw new Exception(string.Format("Range ID {0} not found", range.Id));

        rangeToUpdate.Status = range.Status;
        rangeToUpdate.FirstPsn = range.FirstPsn;
        rangeToUpdate.LastPsn = range.LastPsn;
        rangeToUpdate.PageCount = (int)(range.LastPsn - range.FirstPsn);
        rangeToUpdate.CourierId = range.CourierId;
        rangeToUpdate.WorkflowServer = range.WorkflowServer;
        rangeToUpdate.DoubleFeedInd = range.DoubleFeedInd;

        repo.Commit();
     }
  }

And the unit test:

  [TestMethod]
  public void UpdateRange_RangesPropertiesAreCopied()
  {
     var rangeToUpdate = new Range()
     {
        Id = 1, CourierId = 0, Status = 20,
        WorkflowServer = null,
        FirstPsn = -1, LastPsn = -1, PageCount = 0
     };
     var rangeToSave = new Range()
     {
        Id = 1, CourierId = 1234, Status = 40,
        WorkflowServer = new PhysicalStation(){ Id = 1, Name = "fakeserver"},
        FirstPsn = 123, LastPsn = 456, PageCount = 333
     };
     var repoMock = new Mock<IRepository>();
     repoMock.Setup(x => x.Query<Range>()).Returns(new List<Range>(new[] {rangeToUpdate}).AsQueryable()).Verifiable();
     repoMock.Setup(x => x.Commit()).Verifiable();
     var factoryMock = new Mock<IRepositoryFactory>();
     factoryMock.Setup(x => x.GetRepository(It.IsAny<string>())).Returns(repoMock.Object);
     IAutoTisDal dal = new AutoTisDal(factoryMock.Object, "");

     dal.UpdateRange(rangeToSave);

     repoMock.VerifyAll();

     Assert.AreEqual(rangeToUpdate.Id, rangeToSave.Id);
     Assert.AreEqual(rangeToUpdate.CourierId, rangeToSave.CourierId);
     Assert.AreEqual(rangeToUpdate.LastPsn, rangeToSave.LastPsn);
     Assert.AreEqual(rangeToUpdate.FirstPsn, rangeToSave.FirstPsn);
     Assert.AreEqual(rangeToUpdate.PageCount, rangeToSave.LastPsn - rangeToSave.FirstPsn);
     Assert.AreSame(rangeToUpdate.WorkflowServer, rangeToSave.WorkflowServer);
     Assert.AreEqual(rangeToUpdate.Status, rangeToSave.Status);
  }

I know I need to write another test against the expected exception if no RangeId matches.

I also realize I'm writing test after the code.

First of all, your single method is doing multiple things:

Get Range by id
Copy properties from one instance to another
Commit

Let's try to separate responsibilities:

 Range GetRangeById(Repository repo, int id)
 {
    return repo.Query<Range>().SingleOrDefault(r=>r.Id == id);
 }

 void UpdateRange(Range rangeToUpdate, Range range)
 {
        // I'd even make this Range extension method
        rangeToUpdate.Status = range.Status;
        rangeToUpdate.FirstPsn = range.FirstPsn;
        rangeToUpdate.LastPsn = range.LastPsn;
        rangeToUpdate.PageCount = (int)(range.LastPsn - range.FirstPsn);
        rangeToUpdate.CourierId = range.CourierId;
        rangeToUpdate.WorkflowServer = range.WorkflowServer;
        rangeToUpdate.DoubleFeedInd = range.DoubleFeedInd;
 }

 void IAutoTisDal.UpdateRange(Range range)
  {
     using (var repo = _repositoryFactory.GetRepository(_baseline))
     {
        var rangeToUpdate = GetRangeById(range.Id);
        if (rangeToUpdate == null)
           throw new Exception(string.Format("Range ID {0} not found", range.Id));

        UpdateRange(rangeToUpdate, range);
        repo.Commit();
     }
  }

Two new methods are already easier to understand and test.
And, honestly, I don't think that writing test for IAutoTisDal.UpdateRange makes much sense. It won't provide much value.

I dunno if I would use reflection for the range mapping to properties.  I think this would be a great instance for Func<> lambda expressions.  Or perhaps a dictionary key value implementation that can be iterated through..


C++ how to use std::any_cast and avoid else if statements I have created a Node class that I am using for various projects in my data structures class(at school) and I am overloading the insertion operator <<. I am using std::any for my container datatype inside Node class and am using std::any_cast to insert this data into a stream whenever possible. Std::any_cast requires that I know the datatype beforehand, and so I've come up with a hacky way to get this information by detangling the value typename into a string and then using a ton of else if statements to compare this string, as you can see in my code below... I'm looking for a different solution or even some advice about why this approach is fundamentally wrong and how to go about making better programming decisions. Lay it on me, I need help and can take it!

friend std::ostream& operator << (std::ostream& out, Node& n){

    std::string t;
    t = abi::__cxa_demangle(n.value.type().name(), 0, 0, 0);;

    if (t.find("string") != std::string::npos) { out << std::any_cast<std::string>(n.value); }
    else if (t == "unsigned long long int") { out << std::any_cast<unsigned long long int>(n.value); }
    else if (t == "unsigned long int") { out << std::any_cast<unsigned long int>(n.value); }
    else if (t == "long long int") { out << std::any_cast<long long int>(n.value); }
    else if (t == "long int") { out << std::any_cast<long int>(n.value); }
    else if (t == "unsigned short int") { out << std::any_cast<unsigned short int>(n.value); }
    else if (t == "short int"){ out << std::any_cast<short int>(n.value); }
    else if (t == "int") { out << std::any_cast<int>(n.value); }
    else if (t == "long double") { out << std::any_cast<long double>(n.value); }
    else if (t == "double") { out << std::any_cast<double>(n.value); }
    else if (t == "float") { out << std::any_cast<float>(n.value); }
    else if (t == "bool") { out << std::any_cast<bool>(n.value); }
    else if (t == "unsigned char") { out << std::any_cast<unsigned char>(n.value); }
    else if (t == "signed char") { out << std::any_cast<signed char>(n.value); }
    else if (t == "char") { out << std::any_cast<char>(n.value); }
    else { out << t; };

    return out;
};

First, this call to __cxa_demangle returns a buffer allocated with malloc and which must be freed. This isn't freed, so this code has a memory leak.

Second, as I understand it, __cxa_demangle does not exist in MSVC, so this is not portable.

Third, the .find("string") catches more than std::string. It would match std::wstring, std::stringstream, void (*)(std::string).

Fourth, this code relies essentially on knowing the potential types that are in that std::any. That is knowledge at a distance, and if you allow more types at the place of construction, you need more types here.

Fifth, if the type isn't one of those types, the error is silent and the output is the type contained, not the value. That could be harmless, but it could be a real problem. If you allowed type Foo in the any, and neglected to update this location, you wouldn't be able to tell if the output Foo was that type name or the value of a string stored in the any.

The suggestion of the other answer is to constrain the types stored using the type storing them. Instead of any use std::variant<std::string, long long, unsigned long long, double>. I added the string and unsigned options relative to the other answer. Then just visit:

std::visit([&](const auto& value) { out << value; }, n.value);

This is a very strong option.

A second option would be to erase not almost every aspect of the type (as with std::any) but instead leave the interface you need, e.g. erase everything but the stream operator if that's all you need:

    printable p = 2.3;
    std::cout << p << "\n";
    p = "foo";
    std::cout << p << "\n";

Example: https://godbolt.org/z/NtJqWk

(edit: added a missing virtual destructor to impl_base)

You kept it concise.  Anyone concerned with your code can easily comment it out and rewrite it.  Add a test case and everyone will help you fix stuff if it needs it.


Tic-Tac-Toe code using Python with Tkinter `I made this Tic-Tac-Toe game first, by using user input in the console here:
https://pastebin.com/6zLjrWcf
, and now new and improved using Tkinter:

import tkinter as tk
from tkinter import ttk
import time

def create_button(relx, rely):
    button = tk.Button(width=10, height=2, command=lambda: callback(button))
    button.place(relx=relx, rely=rely)
    return button

def check_win():
    if (buttons[0]['text'] == buttons[1]['text'] == buttons[2]['text'] != '') or \
            (buttons[3]['text'] == buttons[4]['text'] == buttons[5]['text'] != '') or \
            (buttons[6]['text'] == buttons[7]['text'] == buttons[8]['text'] != '') or \
            (buttons[0]['text'] == buttons[3]['text'] == buttons[6]['text'] != '') or \
            (buttons[1]['text'] == buttons[4]['text'] == buttons[7]['text'] != '') or \
            (buttons[2]['text'] == buttons[5]['text'] == buttons[8]['text'] != '') or \
            (buttons[2]['text'] == buttons[4]['text'] == buttons[6]['text'] != '') or \
            (buttons[0]['text'] == buttons[4]['text'] == buttons[8]['text'] != ''):
        return True
    else:
        return False

def callback(button):
    global turn, x
    if x == 1:
        time.sleep(1)
        game.quit()
    invalid['text'] = ''
    if button['text'] != '':
        invalid['text'] = 'Invalid space try again'
        return
    button['text'] = turn
    if check_win():
        invalid['text'] = 'Player ' + turn + ' WINS!!!!!'
        x = 1
    turn = ('0' if turn == 'X' else 'X')
    label_button['text'] = 'PLAYER ' + turn + '\'S TURN.'

x = 0
turn = 'X'
game = tk.Tk()
game.title('TicTacToe')
game.geometry('700x500')
buttons = []
for i in range(1, 10):
    button_created = create_button(0.25 if i / 3 <= 1 else 0.45 if i / 3 <= 2 else 0.65, 0.2 if i in [1, 4, 7] else
                                   0.4 if i in [2, 5, 8] else 0.6)
    buttons.append(button_created)
label_button = ttk.Button(game, text='PLAYER ' + turn + '\'S TURN.', style='Fun.TButton', width=20, state='disabled')
label_button.pack(pady=30)
invalid = tk.Label(text='')
invalid.place(relx=0.4, rely=0.12)
game.mainloop()

My main question is if there is a way to compact check_win()?
Also please review the rest of the code.

Let us begin with some layout. This is a typical grid use case so that you won't have to manually manage grid. At the lowest level, a grid structure essentially places buttons by calculating coordinates. But this gets tedious over time, that's why a grid layout is provided. Also, it was a clever way of cheking win by being symbol agnostic. A normal tic tac toe game would have used checkwin(symbol) to determine win.

On strings

To escape ' within ' ' you can use " instead. From

'\'S TURN.'

to

"'S TURN."

and you can also use string formatting to clear up some clutter.

"PLAYER {}'S TURN.".format(turn)

On layout

Modifying your create_button function to this allows a grid structure

def create_button(x, y):
    button = tk.Button(width=10, height=2, command=lambda: callback(button))
    button.grid(row=x, column=y)
    return button

Then we modify others since different layouts can't be mixed

label_button = ttk.Button(
                        game,
                        text="PLAYER {}'S TURN.".format(turn),
                        style='Fun.TButton', width=20,
                        state='disabled')
label_button.grid(row=0, column=1)
invalid = tk.Label(text='')
invalid.grid(row=4, column=1)

adding the buttons can be then done as

buttons = []

buttons.append(create_button(1, 0))
buttons.append(create_button(1, 1))
buttons.append(create_button(1, 2))
buttons.append(create_button(2, 0))
buttons.append(create_button(2, 1))
buttons.append(create_button(2, 2))
buttons.append(create_button(3, 0))
buttons.append(create_button(3, 1))
buttons.append(create_button(3, 2))

You can use a loop for the row and itertools.cycle for the 0, 1, 2 if you want to simplify it.

The check_win function

Simplifying if

Adding a () to if statements allows you to write or without \

if ... :
    ...

to

if (...):
    ...

thus the win_function can be simplified from

if (buttons[0]['text'] == buttons[1]['text'] == buttons[2]['text'] != '') or \
(buttons[3]['text'] == buttons[4]['text'] == buttons[5]['text'] != '') or \

to

def check_win():
    if (
        (buttons[0]['text'] == buttons[1]['text'] == buttons[2]['text'] != '') or
        (buttons[3]['text'] == buttons[4]['text'] == buttons[5]['text'] != '') or
        ...
        ):
        return True
    else:
        return False

Simplifying values write-up

This can also be further simplified by defining a function to replace buttons[0]['text']

def btext(i):
    return buttons[i]['text']

and using it

def check_win():
    if (
        (btext(0) == btext(1) == btext(2) != '') or
        (btext(3) == btext(4) == btext(5) != '') or
        (btext(6) == btext(7) == btext(8) != '') or
        (btext(0) == btext(3) == btext(6) != '') or
        (btext(1) == btext(4) == btext(7) != '') or
        (btext(2) == btext(5) == btext(8) != '') or
        (btext(2) == btext(4) == btext(6) != '') or
        (btext(0) == btext(6) == btext(8) != '')
        ):
        return True
    else:
        return False

On architecture

A common pattern is the MVC (Model, View, Controller). While checking and updating gui directly works here, you might consider adding states in a structure like this:

board = [
    ['', '', ''],
    ['', '', ''],
    ['', '', '']
]

Operations are done on this and the gui is updated according to this.

From :
Python Tic Tac Toe Game

    win_commbinations = ((0, 1, 2), (3, 4, 5), (6, 7, 8), (0, 3, 6), (1, 4, 7), (2, 5, 8), (0, 4, 8), (2, 4, 6))

for a in win_commbinations:
            if board[a[0]] == board[a[1]] == board[a[2]] == "X":
                print("Player 1 Wins!\n")
                print("Congratulations!\n")
                return True

            if board[a[0]] == board[a[1]] == board[a[2]] == "O":
                print("Player 2 Wins!\n")
                print("Congratulations!\n")
                return True
        for a in range(9):
            if board[a] == "X" or board[a] == "O":
                count += 1
            if count == 9:
                print("The game ends in a Tie\n")
                return True

This alternative solution is a bit cleaner + includes a "Tie" check, which your original solution doesn't check ( just remove if you consider it irrelevant ofc )


Counting the minimum number of blocks to remove to form a triangular temple PROBLEM STATEMENT :

  You want to build a temple for snakes. The temple will be built on a
  mountain range, which can be thought of as n blocks, where height of
  i-th block is given by hi. The temple will be made on a consecutive
  section of the blocks and its height should start from 1 and increase
  by exactly 1 each time till some height and then decrease by exactly 1
  each time to height 1, i.e. a consecutive section of 1, 2, 3, .. x-1,
  x, x-1, x-2, .., 1 can correspond to a temple. Also, heights of all
  the blocks other than of the temple should have zero height, so that
  the temple is visible to people who view it from the left side or
  right side.

  You want to construct a temple. For that, you can reduce the heights
  of some of the blocks. In a single operation, you can reduce the
  height of a block by 1 unit. Find out minimum number of operations
  required to build a temple.

  INPUT

  The first line of the input contains an integer T denoting the number
  of test cases. The description of T test cases follows.

  The first line of each test case contains an integer n.

  The next line contains n integers, where the i-th integer denotes hi

  OUTPUT

  For each test case, output a new line with an integer corresponding to
  the answer of that testcase.

  CONSTRAINTS

  1 ≤ T ≤ 10

  2 ≤ N ≤ 105

  1 ≤ Hi ≤ 10^9

  Example Input

3
3
1 2 1
4
1 1 2 1
5
1 2 6 2 1

  Output

0
1
3

MY SOLUTION

#include <stdio.h>

void quickSort(int arr[], int low, int high);
int partition (int arr[], int low, int high);
void swap(int* a, int* b);

int main(void) {
    // your code goes here
    int t=0, i=0, n=0, move=0, j=0;
    scanf("%d\n", &t);
    for(i=1;i<=t;++i)
    {
        scanf("%d\n", &n);
        int arr[n];
        int i5;
        for(i5=0;i5<n;++i5)
        scanf("%d ", &arr[i5]);

        quickSort(arr, 0, n-1);

        int max= (arr[n-1]!=arr[n-2])?arr[n-2]+1:arr[n-1];
        move = arr[n-1]-max;
        --max;
        int i3=0;
        for(i3=n-2;i3>0;--max)
        {
            move+=((arr[i3]-max)+(arr[i3-1]-max));
            i3-=2;
            if(max==1)
            {
                break;
            }
        }
        for(j=0;j<=i3;++j)
        {
            move+=arr[j];
        }
        printf("%d\n", move);
    }
    return 0;
}

// A utility function to swap two elements
void swap(int* a, int* b)
{
    int t = *a;
    *a = *b;
    *b = t;
}

/* This function takes last element as pivot, places
   the pivot element at its correct position in sorted
    array, and places all smaller (smaller than pivot)
   to left of pivot and all greater elements to right
   of pivot */
int partition (int arr[], int low, int high)
{
    int pivot = arr[high];    // pivot
    int i = (low - 1);  // Index of smaller element
    int j;

    for (j = low; j <= high- 1; j++)
    {
        // If current element is smaller than or
        // equal to pivot
        if (arr[j] <= pivot)
        {
            i++;    // increment index of smaller element
            swap(&arr[i], &arr[j]);
        }
    }
    swap(&arr[i + 1], &arr[high]);
    return (i + 1);
}

/* The main function that implements QuickSort
 arr[] --> Array to be sorted,
  low  --> Starting index,
  high  --> Ending index */
void quickSort(int arr[], int low, int high)
{
    if (low < high)
    {
        /* pi is partitioning index, arr[p] is now
           at right place */
        int pi = partition(arr, low, high);

        // Separately sort elements before
        // partition and after partition
        quickSort(arr, low, pi - 1);
        quickSort(arr, pi + 1, high);
    }
}

Your code is not working for some cases.
Consider for example input 1 1 2 2 4.
The answer should be 6, but your code returns 1.

You can use mergesort instead of quicksort as that may encounter a Time Limit Exceeded.


Slow anagrams detection This is my code for this question on hackerrank.
The idea is to find the amount of substrings of a given word that are anagrams of each other.

The code works fine, but it times out with longer strings. I've downloaded the test cases to make sure the result is correct and it is, it just takes too long.
I can do this in C and pass all tests, but I was wondering if there is a faster alternative in Perl.

I also tried using numbers, i.e. multiplying the ord()-97 of the characters in the two substrings, and then doing a single check on the number, to see if they are anagrams of each other.
That is much faster, but only works on small strings because it overflows quickly.

In this specific case, I'm not really that concerned about style, as much as performance. How can this be improved?

use strict;
use warnings;

use Data::Dumper;

sub get_all_substr {
    my ($input_string) = @_;

    my @results;
    push @results, $input_string =~ /(?=(.{$_}))/g for 1 .. length($input_string);

    return \@results;
}

sub is_anagram {
    my ($string_a, $string_b) = @_;

    my $len_a = length($string_a);

    return 0 if (length($string_b) != $len_a);

    my %vec_a;
    my %vec_b;

    foreach my $i (0..$len_a-1) {
        $vec_a{substr($string_a, $i, 1)}++;
        $vec_b{substr($string_b, $i, 1)}++;
    }

    return 0 if (scalar(keys(%vec_a)) != scalar(keys(%vec_b)));

    foreach my $key (keys(%vec_a)) {
        if (!defined($vec_b{$key}) || ($vec_a{$key} != $vec_b{$key})) {
            return 0;
        }
    }

    return 1;
}

my @results;

my $t = <STDIN>;
chomp $t;

while($t--) {
    my $input_string = <STDIN>;
    chomp $input_string;

    my $count = 0;
    my $strings;
    $strings = get_all_substr($input_string);

    my $max_size = scalar(@{$strings})-1;
    foreach my $i (0..$max_size) {
        foreach my $k ($i+1..$max_size) {
            if (is_anagram($strings->[$k], $strings->[$i])) {
                $count++;
            }
        }
    }
    push @results, $count;
}

print join("\n", @results);

Finally nailed it in Perl. In Python this same approach worked no problem, but clearly Perl (or at least the interpreter run on hackerrank) is much slower.

The "trick" is to extract all the substrings already in the form of an array, preferably already sorted, so that there is no need to do the loop another time inside of is_anagram.

Here is my code, admittedly not a beauty, but now it passes all tests.

use strict;
use warnings;

sub get_all_subarr {
    my ($input_string) = @_;

    my %results;

    foreach my $i (1..length($input_string)-1) {
        foreach my $j (0..length($input_string)-$i) {
            push @{$results{$i}}, [sort(split(//,substr($input_string, $j, $i)))];
        }
    }

    return \%results;
}

sub is_anagram {
    my ($arr_a, $arr_b) = @_;

    return 0 if (join('', @{$arr_a}) ne join('', @{$arr_b}));
    return 1;
}

my @results;

my $t = <STDIN>;
chomp $t;

while($t--) {
    my $input_string = <STDIN>;
    chomp $input_string;

    my $count = 0;
    my $strings_arr = get_all_subarr($input_string);
    foreach my $i (keys(%{$strings_arr})) {
        my $max_size = scalar(@{$strings_arr->{$i}})-1;
        foreach my $j (0..$max_size) {
            foreach my $k ($j+1..$max_size) {
                if (is_anagram($strings_arr->{$i}[$j], $strings_arr->{$i}[$k])) {
                    $count++;
                }
            }
        }
    }
    push @results, $count;
}

print join("\n", @results);

I will let someone else discuss the style, which looks terrible to me, but then again it's Perl ;) As Jerry Coffin mentioned, it's all about sorting and keeping track of sorted strings. Your techniques will not work.

This is my entry in JS that gets full marks, it should be obvious how this works

function processData(input) {

  lines = input.split('\n');
  lines.shift();

  for( line of lines  ){
    var map = {}, count = 0;

    for( inner = 0 ; inner < line.length;inner++ ){
      for( outer = inner ; outer < line.length ; outer++ )
       //Create a sorted substring, and keep track of the count
       //This was never meant for CodeReview, so yeah, comma chained statement to avoid curly braces ;]
       s = [...line.substring(inner,outer+1)].sort().join(),
       map[s] = ( map[s] || 0 ) + 1;
    }
    for( a in map ){
      while(map[a]>1)
        count = count + --map[a];

    }
    console.log(count);

  }
}


Array duplication in JavaScript I want to achieve this

var arr = [1,2,3];

var duplicateArr = duplicate(arr, 2) // will be [1,2,3,1,2,3]

var duplicateArr2 = duplicate(arr, 3) // will be [1,2,3,1,2,3,1,2,3]

I have implemented it like this:

function duplicate(arr, n) {
  var len = arr.length;
  if (n == 0 || n == 1) {
    return arr;
  }
  if (n > 1) {
    for (var j = 0; j < n - 1; j++) {
      for (var i = 0; i < len; i++) {
        arr.push(arr[i])
      }
    }
  }
  console.log(arr);
}
duplicate([1, 2, 3], 3);

I know this may not be the efficient way to do it.

You can avoid the nested for loops by adding the entire array at the same time. (I'm also using Janos' suggestion here)

function duplicate(arr, n) {
  let result = [];
  for(let i=0; i<n; i++) {
    result.push(...arr);
  }
  return(result);
}

We need the original array unmodified, so we can keep adding it, so I'm using a new result array. This has a few benefits. Firstly it's more common to leave the input array unmodified and return a new array, and secondly it returns an empty array when n=0, which I think makes more sense.

This uses the ES6 spread syntax. If you don't want to use ES6 you can replace

result.push(...arr);

with

result = result.concat(arr);

I think, this will reduce one loop

function duplicate(arr, n) {
    var len = arr.length;
    if (n == 0 || n == 1) {
      return arr;
    }
  if (n > 1) {
    for (var j = 0; j < n - 1; j++) {
      arr = arr.concat(arr);   //**changed here**
     }
   }
 console.log(arr);;
}


Identify duplicate characters in a String and replace them Found a JavaScript assignment on Codewars.com:

  The goal of this exercise is to convert a string to a new string where
  each character in the new string is '(' if that character appears only
  once in the original string, or ')' if that character appears more
  than once in the original string. Ignore capitalization when
  determining if a character is a duplicate.

  Examples:

"din" => "((("
"recede" => "()()()"
"Success" => ")())())"
"(( @" => "))(("

I did the assignment in Java, and came up with below code. I want to know is this the write way to code or is there a better way? If so please let me know.

private static String duplicateEncode(String word) {
    String encodeString = "";
    String decodedString = word.toLowerCase();
    for (int i = 0; i < decodedString.length(); i++) {

        if (decodedString.substring(i + 1, decodedString.length())
                .contains(String.valueOf(decodedString.charAt(i)))
                || decodedString.substring(0, i).contains(String.valueOf(decodedString.charAt(i)))) {
            encodeString += ")";
        } else {
            encodeString += "(";
        }

    }
    return encodeString;
}

First of all, nullbyte is exactly right about the use of a StringBuilder: using a += b for Strings roughly equals new StringBuilder().append(a).append(b).toString() under the hood and thus is a quite expensive operation.

Then, your method to check whether a given charcter exists in the string after a given index is s.substring(i).contains(ch) which again creates a superfluos string object via the substring-call. Instead, simply use s.indexOf(ch, i) to find the next index of ch after the given starting index (returns -1 for "not found").

Additionally, you might consider nullbyte's suggestion to hold the seen-or-not-seen status in a set, which also was my first intuition when reading the exercise. (Right now, you have runtime complexity of O(n^2) which you could reduce to O(n)). You only have to somehow come up with a method to revisit the first character, maybe by not using  a set but by usign a map from character to number of occurrences or something like that.

Naming: why decodedString? This is not decoded in any way, this is just lowercase word. I can live with encodeString (or probably better encoded), but decodedString does not look right and does not give the reader any clue about the variable's contents.

Here's a better solution using a hash map:

 public static String encode(String input) {
    final StringBuilder builder = new StringBuilder();
    final Map<Character, Integer> indexMap = new HashMap<>();
    for (int index = 0; index < input.length(); index++) {
        char ch = Character.toLowerCase(input.charAt(index));
        if(indexMap.containsKey(ch)) {
            int previous = indexMap.get(ch);
            builder.replace(previous, previous+1, ")");
            builder.append(')');
        }else {
            indexMap.put(ch, index);
            builder.append('(');
        }
    }
    return builder.toString();
}

It requires O(n) time and O(n) additional space, but it's way faster than yours. Also, try using a StringBuilder if you append strings in a loop.


Consolidate list of ranges that overlap I wanted to implemented an algorithm in Python 2.4 (so the odd construction for the conditional assignment) where given a list of ranges, the function returns another list with all ranges that overlap in the same entry.

I would appreciate other options (more elegant or efficient) than the one I took.

ranges = [(1,4),(1,9),(3,7),(10,15),(1,2),(8,17),(16,20),(100,200)]

def remove_overlap(rs):
        rs.sort()
        def process (rs,deviation):
                start = len(rs) - deviation - 1
                if start < 1:
                        return rs
                if rs[start][0] > rs[start-1][1]:
                        return process(rs,deviation+1)
                else:
                        rs[start-1] = ((rs[start][0],rs[start-1][0])[rs[start-1][0] < rs[start][0]],(rs[start][1],rs[start-1][1])[rs[start-1][1] > rs[start][1]])
                        del rs[start]
                        return process(rs,0)
        return process(rs,0)

print remove_overlap(ranges)

Firstly, modify or return, don't do both.

Your process method returns lists as well as modify them. Either you should construct a new list and return that, or modify the existing list and return nothing.

Your remove_overlap function does the same thing. It should either modify the incoming list, or return a new list not both.

You index [0] and [1] on the tuples a lot to fetch the start and end. That's best avoided because its not easy to tell whats going on.

rs[start-1] = ((rs[start][0],rs[start-1][0])[rs[start-1][0] < rs[start][0]],(rs[start][1],rs[start-1][1])[rs[start-1][1] > rs[start][1]])

Ouch! That'd be much better off broken into several lines. You shouldn't need to check which of the starts is lower because sorting the array should mean that the earlier one is always lower. I'd also use the max function to select the larger item, (if you don't  have it in your version of python, I'd just define it)

Your loop is backwards, working from the end. That complicates the code and makes it harder to follow. I'd suggest reworking it work from the front.

return process(rs,0)

You start the checking process over again whenever you merge two ranges. But that's not so great because you'll end up rechecking all the segments over and over again. Since you've already verified them you shouldn't check them again.

Your recursion process can be easily rewritten as a while loop. All you're doing is moving an index forward, and you don't really need recursion.

This is my implementation:

def remove_overlap(ranges):
    result = []
    current_start = -1
    current_stop = -1

    for start, stop in sorted(ranges):
        if start > current_stop:
            # this segment starts after the last segment stops
            # just add a new segment
            result.append( (start, stop) )
            current_start, current_stop = start, stop
        else:
            # segments overlap, replace
            result[-1] = (current_start, stop)
            # current_start already guaranteed to be lower
            current_stop = max(current_stop, stop)

    return result

I think there is an error in the algorithm. Example:

ranges = [(1,5), (2,4), (8,9)]

Expected response:

ranges2 = [(1,5), (8,9)]

Response obtained:

ranges2 = [(1,4), (8,9)]

A small change and the error is fixed.

def remove_overlap(ranges):
    result = []
    current_start = -1
    current_stop = -1

    for start, stop in sorted(ranges):
        if start > current_stop:
            # this segment starts after the last segment stops
            # just add a new segment
            current_start, current_stop = start, stop
            result.append( (start, stop) )
        else:
            # segments overlap, replace
            # current_start already guaranteed to be lower
            current_stop = max(current_stop, stop)
            result[-1] = (current_start, current_stop)

    return result


Classifying the user into an age bracket In this program, I have to check the range of a given age and then print a message as per the age. I want to optimize the if else  logic.

/* Sample code to read in test cases:*/

import java.io.*;
public class Main {
 public static void main (String[] args) throws IOException {
    File file = new File(args[0]);
    BufferedReader buffer = new BufferedReader(new FileReader(file));
    String line;
    while ((line = buffer.readLine()) != null) {
        line = line.trim();
        // Process line of input Here

        int age = Integer.parseInt(line);

          if(age>=0 && age<=2){
              System.out.println("Still in Mama's arms");
             // break;
          }else if(age>=3 && age<=4){
               System.out.println("Preschool Maniac");
              // break;
          }else if(age>=5 && age<=11){
               System.out.println("Elementary school");
             // break;
          }else if(age>=12 && age<=14){
               System.out.println("Middle school");
             // break;
          }else if(age>=15 && age<=18){
               System.out.println("High school");
             // break;
          }else if(age>=19 && age<=22){
               System.out.println("College");
             // break;
          }else if(age>=23 && age<=65){
               System.out.println("Working for the man");
             // break;
          }else if(age>=66 && age<=100){
               System.out.println("The Golden Years");
              //break;
          }else{
               System.out.println("This program is for humans");

          }
         }
      }
    }

Is there any other way to do this program? Using a switch, it becomes more complex to type with so many cases for the age.

A TreeMap<Integer, String> with a for loop and stop on first number greater than or equaling the number to find will solve your problem.

treemap.put(-1, "This program is for humans");
treemap.put(2, "Still in Mama's arms");
//etc...
treemap.put(Integer.MAX_VALUE, "This program is for humans");

Then, loop through entries...

for(Map.Entry<Integer, String> entry : treemap.entrySet()){
    if(entry.getKey() >= age){
        System.out.println(entry.getValue());
        break;
    }
}

The default behaviour for a TreeMap is that the contents are sorted based on the key, which explains why this works. Use Integer.MAX_VALUE for the last case - it'll be equal to or greater than any other number.

You can even use a built-in function for this:

System.out.println(treemap.ceilingEntry(age).getValue());

Making the final code...

import java.io.*;
import java.util.TreeMap;

public class Main {
    private static TreeMap<Integer, String> treemap = new TreeMap<>();

    private static void initializeAgeDescriptions() {
        treemap.put(-1, "This program is for humans");
        treemap.put(2, "Still in Mama's arms");
        treemap.put(4, "Preschool Maniac");
        treemap.put(11, "Elementary school");
        treemap.put(14, "Middle school");
        treemap.put(18, "High school");
        treemap.put(22, "College");
        treemap.put(65, "Working for the man");
        treemap.put(100, "The Golden Years");
        treemap.put(Integer.MAX_VALUE, "This program is for humans");
    }

    public static void main (String[] args) throws IOException {
        initializeAgeDescriptions();
        File file = new File(args[0]);
        BufferedReader buffer = new BufferedReader(new FileReader(file));
        String line;
        while ((line = buffer.readLine()) != null) {
            line = line.trim();

            int age = Integer.parseInt(line);
            System.out.println(treemap.ceilingEntry(age).getValue());
        }
    }
}

With Pimgd's great and easy to use answer, I will provide a bit more of an (over engineerd, if you will) OOP solution that combines his answer and my own thoughts.

Consider a class called AgeDescription:

public class AgeDescription {
    private int age;
    private String description;

    public AgeDescription(int age, String descrption) {
        this.age = age;
        this.description = description;
    }

    public int getAge() [
        return age;
    }

    public String getDescription() {
        return description;
    }
}

Since there is a bunch of logic behind determining what description belongs to what age, this logic needs to be encapsulated, reusable and testable. For this we can use the Factory design-pattern:

public class AgeDescriptionFactory {
    private static TreeMap<Integer, String> ageDescriptionTreeMap = new TreeMap<>();

    static {
        // This gets executed once to fill up the TreeMap.
        ageDescriptionTreeMap.put(2, "Still in Mama's arms");
        ageDescriptionTreeMap.put(4, "Preschool Maniac");
        // etc...
        ageDescriptionTreeMap.put(Integer.MAX_VALUE, "This program is for humans");
    }

    public AgeDescription createFromAge(int age) {
        if (age < 0) {
            throw new InvalidArgumentException("Age cannot be negative");
        }

        return new AgeDescription(age, getDescriptionFromAge(age));
    }

    private String getDescriptionFromAge(int age) {
        return ageDescriptionMap.ceilingEntry(age).getValue();
    }
}

Thank you @Tunaki for mentioning the ceilingEntry function in your comment

I think checking for a negative age is a good thing to do, because it is technically not a valid argument. Really large positive numbers could be actual ages in the near future when medicine and what not have progressed tremendously, but negative ages I don't ever see existing.

Now you can use the classes like this:

// Retrieve age variable from user input

AgeDescriptionFactory ageDescriptionFactory = new AgeDescriptionFactory();
AgeDescription ageDescription = ageDescriptionFactory.createFromAge(age);

System.out.println(ageDescription.getDescription());

As an additional benefit, the code is now easily testable:

public class AgeDescriptionFactoryTest {
    private AgeDescriptionFactory ageDescriptionFactory;

    // This method is run before each test
    @before
    public void setUp() {
        this.ageDescriptionFactory = new AgeDescriptionFactory();
    }

    @Test(expected=InvalidArgumentException.class)
    public void testNegativeAgeThrowsException {
        this.ageDescriptionFactory.createFromAge(-1);
    }

    @Test
    public void testAgeInFourtiesReturnsWorkingMan {
        AgeDescription ageDescription = ageDescriptionFactory.createFromAge(44);
        Assert.equals("Working for the man", ageDescription.getDescription();
    }

    // Keep on testing!
}

A good thing to take away from this is that you want to keep your logic seperate from what is done with that logic. You have intertwined outputting the description of an age with the actual logical proces of determining this description based on the age. There is no way to use that logic for anything else. By seperating this, you get code that is testable and reusable.


Project Euler #1 - Multiples of 3 and 5 - First Program Taking a problem from the website, "Project Euler #1", I have created my first C# program. The problem I have used is as follows:

If we list all the natural numbers below 10 that are multiples of 3 or 5, we get 3, 5, 6 and 9. The sum of these multiples is 23.
Find the sum of all the multiples of 3 or 5 below 1000.

The program I have made:
using System;

namespace _5_multiples
{
    public class MainClass
    {
        public static void Main()
        {
            int currentNumber = 1000;
            int allSum = 0;

            currentNumber = currentNumber - 1;
            while (currentNumber > 0)
            {
                if (currentNumber % 3 == 0)
                {
                    allSum = allSum + currentNumber;
                }
                else
                {
                    if (currentNumber % 5 == 0)
                    {
                        allSum = allSum + currentNumber;
                    }
                }
                currentNumber = currentNumber - 1;
            }
            Console.WriteLine(allSum);
        }
    }
}

I understand there is probably a lot to improve - this is my first C# program after all. I did run into a few problems such as referencing non static members from a static context and no suitable entry method. Rather than finding a definitive fix, the solution I implemented was more of a workaround; hence why I feel there may be a lot to improve.
namespace _5_multiples - this doesn't look right? Name of the program is actually 3_5_multiples - perhaps a badly chosen name?
Anyway, please let me know on anything that could be improved.

Here's what I would change:

I'd use a for loop instead of a while loop here: for (int currentNumber = 999; currentNumber > 0; currentNumber--). This keeps all loop-related things grouped together. Loop bodies can be skipped or broken out of, so if code that must be executed every cycle is put at the end of a loop body it's easily broken when the loop body grows more complex and someone adds a continue or break in there.
Actually, there is no need to count down - the result will be the same either way. Counting up is often a little easier to understand: for (int currentNumber = 0; currentNumber < 1000; currentNumber++). Note that using < here means you don't need to decrement currentNumber by one before the loop starts.
You can write a += b instead of a = a + b (the same goes for various other operators).
else { if { .. } } can be simplified to else if { ... }.
Though in this case, a single if statement is sufficient: if (currentNumber % 3 == 0 || currentNumber % 5 == 0) { ... }. The 'or' operator (||) short-circuits if the first expression is true, so if a number can be divided by 3 it won't also try to divide it by 5.
C# naming conventions typically write class, method and namespace names in PascalCase.

Regarding your questions:

Static things belong to a class, non-static things belong to instances of that class. Classes are to instances what blueprints are to actual buildings. Trying to access a non-static (instance) members from a static context is like trying to open a door on a blueprint. The blueprint doesn't contain any real doors, only actual buildings do.
Programs need a place to start executing, and in C# the starting point is a static Main method. Putting everything in there is fine for a small program like this.
Identifier names in C# cannot start with a number, so 3_5_multiples isn't a valid name. I'd probably use EulerProblem1 or something like that here.
Namespaces are useful for organizing larger programs (namespaces to classes are a bit like folders to files). The root namespace of a project is usually the name of that project itself, and child namespaces often correspond to folders.

EDIT: Part of the fun on Project Euler is coming up with more efficient solutions. Can you find a way to solve this without checking every number below 1000?

using System;

namespace _5_multiples
{
    public class MainClass
    {
        public static void Main()
        {
            int currentNumber = 1000;
            int allSum = 0;

            currentNumber = currentNumber - 1;
            while (currentNumber > 0)
            {
                if ( (currentNumber % 3 == 0) | (currentNumber % 5 == 0))
                {
                    allSum = allSum + currentNumber;
                }
                currentNumber = currentNumber - 1;
            }
            Console.WriteLine(allSum);
        }
    }
}

This is my advice, you didn't need both -if-, only one is necessary.


Function outcome: consistent approach and easy logging My goal is to make a consistent and easy way to determine how a function fails/succeeds. I'm looking for code review of my Status class for any bad practices, code that isn't thread-safe and general improvements and changes.

For logging and debugging I always needed to know how a function failed and print this out as readable text. The solution I usually employed was using enumerations - enum ERROR_CODE {..}; - which is fine except they cannot be expanded/overridden - I'm making an API so enums don't allow the user to expand/add custom error codes. They also don't print out nicely to describe what error actually occurred - they just print out as numbers. So I made a status class.

Main features:

Allows creation of error/states wherever & whenever (runtime, compile time):

static const Status S_SUCCESS         = CStatus::registerState("Success");
static const Status S_EMPTY_CONTAINER = CStatus::registerState("Empty rectangle vectors");

General usage:

Status unionRectangles(const vector<Rect>& left, vector<Rect>& right)
{
  if (left.size() <= 0 || right.size() <= 0)
     return S_EMPTY_CONTAINER;

  ...

  return S_SUCCESS;
}

Print out descriptive errors without requiring conversion:

vector<Rect> v1, v2;
Status myStatus = unionRects(v1, v2)
cout << myStatus << end; // "Empty rectangle vectors"

Easily and efficiently compare objects without conversion:

if (myStatus == S_SUCCESS) // for eg; resolves to if (1 == 9), ie, not string comparison
    // do something

Easy to inline check error:

if (!unionRects(v1, v2))
    // do something

Full implementation:

#ifndef STATUS_H
#define STATUS_H

#include <iostream>
#include <hash_map>

typedef int STATE;
typedef std::hash_map<STATE, tstring>::iterator StateIterator;
typedef class CStatus;
typedef const CStatus Status;

#ifndef S_SUCCESS
#define S_SUCCESS CStatus::CS_SUCCESS
#endif // S_SUCCESS

#ifndef S_UNDEFINED_ERROR
#define S_UNDEFINED_ERROR CStatus::CS_UNDEFINED_ERROR
#endif // S_SUCCESS

class CStatus
{
public:
    // Static Properties//
    static Status CS_SUCCESS;
    static Status CS_UNDEFINED_ERROR;

    // Static Functions //
    static Status registerState(const tstring &stateMsg)
    {
        // Post:

        int nextStateTmp = nextState + 1;
        std::pair<StateIterator, BOOL> res = states.emplace(std::make_pair(nextStateTmp, stateMsg));

        // if state has been inserted
        if (res.second)
            return Status(++nextState);

        // return existing state id
        return Status(res.first->first);
    }

    // Properties //
    const STATE state;

    // Functions //
    CStatus(const STATE &state)  : state(state) {}
    virtual ~CStatus() {}

    const tstring& toString() const
    {
        return (states.find(state) == states.end()) ? states[state] : _T("Undefined");
    }

    operator tstring() const
    {
        return toString();
    }

    operator int() const
    {
        return state;
    }

    bool operator==(const CStatus& obj) const
    {
        return state == obj.state;
    }

    bool operator!=(const CStatus& obj) const
    {
        return state != obj.state;
    }

    bool operator!() const
    {
        return *this != S_SUCCESS;
    }

    std::ostream& operator <<(std::ostream &stream, const CStatus& obj)
    {
        return stream << obj.toString().c_str();
    }

    friend std::ostream& operator<<(std::ostream& stream, const CStatus& obj);

private:
    static std::hash_map<STATE, tstring> states;
    static STATE nextState;

protected:

};

//static Status S_SUCCESS         = CStatus::registerState(_T("Success"));
//static Status S_UNDEFINED_ERROR = CStatus::registerState(_T("Undefined Error"));

#endif // STATUS_H

Is this a working program? In other words, have you tried to compile this? I'd be very surprised it compiled.

#include <hash_map>

This header does not exist in the C++ Standard Library. If you want a hash map, you need to use #include <unordered_map> and use the C++11 standard.

typedef const CStatus Status;

I'm pretty sure that something like: const Status S_SUCCESS doesn't compile as that resolves to: const const CStatus S_SUCCESS.

#ifndef S_SUCCESS
#define S_SUCCESS CStatus::CS_SUCCESS
#endif // S_SUCCESS

#ifndef S_UNDEFINED_ERROR
#define S_UNDEFINED_ERROR CStatus::CS_UNDEFINED_ERROR
#endif // S_SUCCESS

You have an error in the comment for the S_UNDEFINED_ERROR macro.

Your RegisterState function could also be slightly simplified:

// Static Functions
static Status registerState(const tstring &stateMsg)
{
    // Post:
    auto res = states.emplace({nextState + 1, stateMsg});

    // if state has been inserted
    return res.second ? Status(++nextState) : Status(res.first->first);
}

const tstring& toString() const
{
    return (states.find(state) == states.end()) ? states[state] : _T("Undefined");
}

I'm not familiar with what a tstring is but this function should be marked static (as only static members can access static member variables) and you shouldn't be returning the string by const value. Let the caller decide whether or not they want to make it const.

operator tstring() const
{
    return toString();
}

operator int() const
{
    return state;
}

What is the point of these and what do they return? These should be removed as they aren't going to be useful to your API. The entire point of your API is to allow the user to easily create new States without having them know too many of the internal implementation details.

bool operator!=(const CStatus& obj) const
{
    return state != obj.state;
}

Implement this in terms of operator==():

bool operator!=(const CStatus &ob) const
{
    return !(state == obj.state);
}

Finally, with this setup, your user will not be able to add any states during compile time. They will only be able to add checks during runtime (most likely during program startup). You should change your description to reflect this.


URL-safe pseudorandom string generator in C# public static class SimpleToken
{
    const string TOKENALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-.";
    static string NewToken(int length = 16)
    {
        var rnd = new RNGCryptoServiceProvider();
        var tokenBytes = new byte[length];
        rnd.GetBytes(tokenBytes);
        var token =
            Enumerable
                .Range(0, length)
                .Select(i => TOKENALPHABET[tokenBytes[i] % TOKENALPHABET.Length])
                .ToArray();

        return new String(token);
    }
}

I needed a quick and dirty(?) way to generate long urls for onetime use. It's a simple login-scheme where a user enters his e-mail and gets a one-time URL for logging in. The URL is discarded after one use.

I.e. http://example.com/tokenlogin/3cuzLkh8GcANjqnWcijEeJIHphHx6ZDwfj-2XTR4bfkkqmzmmFYAY2tWsZWST1.5

const string TOKENALPHABET = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-.";

I learnt the hard way that the problem with using . in a URL token sent by e-mail is that certain mail clients (Outlook in particular) will attempt to auto-detect URLs in a plain text email, but will exclude a trailing . from the inferred URL, so when your user clicks on the auto-generated link they send an invalid token. I suggest that you change . to _.

    static string NewToken(int length = 16)

Length in what units? Generally with cryptographic stuff it's clearer to explicitly use bits as the unit of length.

        var rnd = new RNGCryptoServiceProvider();

As already noted in comments, this is IDisposable and the standard using pattern is preferred.

        var token =
            Enumerable
                .Range(0, length)
                .Select(i => TOKENALPHABET[tokenBytes[i] % TOKENALPHABET.Length])
                .ToArray();

Firstly, this could be simplified to

        var token =
            tokenBytes
                .Select(b => TOKENALPHABET[b % TOKENALPHABET.Length])
                .ToArray();

But secondly, by only using 6 bits per byte you're throwing away 25% of the entropy which the system just produced for you. On busy servers, cryptographic-grade entropy is a valuable resource and you should only request as much as you need.

If you just want to have one-time use random URL, i change your code and now

It,s cleaer
Much faster than your code
Really random string
URL safe

public static string NewToken()
{
    using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
    {
        byte[] randomBuffer = new byte[16];
        rng.GetBytes(randomBuffer);

        using (MD5 md5 = MD5.Create())
        {
            byte[] hashBytes = md5.ComputeHash(randomBuffer);

            StringBuilder sBuilder = new StringBuilder();
            foreach (byte byt in hashBytes)
            {
                sBuilder.Append(byt.ToString("x2"));
            }

            return sBuilder.ToString();
        }
    }
}

You can change the method to combine random generated buffer with user's email address and then calculate hash of theme, it's more reliable.


Is there a better, more efficient way than Dictionary(Of Tuple(), MyClass)? Suppose I have data coming from different sources that tells me information about a large group of stores that has the following (just an example):

Necessary to define what store we're dealing with:

Store_Name (String)
Location (String)

Store characteristics:

Square_Feet (Double)
No_Employees (Double)
Years_Open (Double)
... (Whatever else)

And the structure of the program I'm working with is such that we will always get the first 2 (defining) characteristics and one or more of the store-specific characteristics. My goal is to conglomerate all the characteristics for each of the specific stores together.

To do this, I have created the following class:

Public Class StoreChars
    Public Property Square_Feet as Double = 0
    Public Property No_Employees as Double = 0
    Public Property Years_Open as Double = 0
    ...
End Class

And then, in my program, I have created the following:

Dim StoreData As New Dictionary(Of Tuple(Of String, String), StoreChars)

The key being the Store_Name and Location as the defining tuple.

So, now, in my code, supposing I just got some data together with the defining characteristics for a set of stores (might be some, might be all... we don't know if we've seen these before or not even), my loop to put in that data looks as follows:

For Each PieceOfInfo in ReturnedData
   Dim TupleKey As New Tuple(Of String, String)(PieceOfInfo.Store_Name,
                                                PieceOfInfo.Location)

   If Not StoreData.ContainsKey(TupleKey) Then
        StoreData.Add(TupleKey, New StoreChars With
                                {
                                 .Square_Feet = PieceOfInfo.Square_Feet
                                     ... (whatever else we got) ...
                                })
   Else
        StoreData(TupleKey).Square_Feet = PieceOfInfo.Square_Feet
                     ... (whatever else we got) ...
   End If
Next

But this seems to be very slow and I'm wondering if it isn't overly-processor-intensive to create a tuple in each loop, check the existence as a key, etc, but I can't think of a better way to do this.

The other solution I was thinking of was to create a Dictionary(Of String, Dictionary(Of String, StoreChars)). That would eliminate having to create a Tuple with each loop, but it would mean having to check existence of the keys in 2 separate dictionaries.

If there is a better way to do this, I would LOVE to hear what it is because I find myself having to program solutions like this fairly often!

Also, even though I wrote this code in VB, I'm just as comfortable with a C# solution - I'm just currently working in a VB environment.

If you're stuck with Tuple<T1,T2> as a key, you want to provide an IEqualityComparer<Tuple<T1,T2>> implementation in the constructor of your dictionary.

If you can change the TKey type, you might want to provide a custom class that implements IEquatable<T>.

The problem with the current code is that comparison between keys of type Tuple<string, string> is not efficient.

You may like to create a Store class that holds all the properties of the store as

We are not using the key components individually.
Also, every class needs to have one purpose.

Then, we can create the dictionary as below

Dictionary(Of String, Store)
Key = StoreName & Location

Also, you may simply use Pascal case in variables, using underscore to seperate words is an old convention.

Using concatenated string (StoreName & Location) as the key doesn't seem to be very elegant but it is a simple solution and we don't need to override the GetHashCode method in our class as well.


Print a pattern of text based on a line from an Excel file I started to learn Python not long ago to automate certain tasks I work on. I want to optimize a script I wrote that takes a line in an Excel file and prints a pattern of text based on this line's content. Here's an example of output from my code:

mgmt_cli add access-rule layer "network" position bottom name "thirdline" source.1 "Value1front" source.2 "Value2front" source.3 "x3front" destination "Dest1front" service "HTTP" track.type "Log" action "Accept"

How would you optimize this code?

Link to the example file (just click Valider et Télécharger le Fichier) http://dl.free.fr/hFlGmywIN
Link to full code : https://pastebin.com/CR5SXHRQ

import xlrd
import re
loc = ("Dir\StackoverflowExample.xlsx")

wb = xlrd.open_workbook(loc)
sheet = wb.sheet_by_index(0)
sheet.cell_value(0, 0)

# Extracting number of rows

def CaseMulti(CellContent, CellName):
 #takes a cell and a string as parameters, prints a pattern like Cell.X ="string", X starts with 0 and ends at the number of line in the Cell
 case = str(CellContent)
 x= re.split ("\s", case)   #splits the cell contents in a list using regex where space is the separator for the list's elements
 if name == CellContent:  # deals with the cell with spaces between objects no clue how it work but it does
  return "name " + '"{}"'.format(CellContent)
 else:
  s=0
  string =""
  anycells = ["any","Any", "ANY"]  # tests if the soure, destination or service has any ==> returns nothing
  if str(x[0]) in anycells :
   return " "
  else:
   if len(x)==1:    # case where te cell has one line (or a word with no spaces)
    string= CellName + " " +'"{}"'.format(case)

   else:            # case where multilines/mutli spaces
    while s<len (x) :
     listelement = x[s]
     string = string + CellName+'.'+str(s+1)+ ' '  +'"{}"'.format(listelement)
     s=s+1

   return string

for numberofrows in range (5):

 print "Rule " + str (numberofrows)
 name= sheet.cell_value(numberofrows,1)
 source = sheet.cell_value(numberofrows,2)
 destination = sheet.cell_value(numberofrows,3)
 protocole = sheet.cell_value(numberofrows,4)
 log = sheet.cell_value(numberofrows,5)
 action = sheet.cell_value(numberofrows,6)
 #firewall = sheet.cell_value(numberofrows,7)

 try:
  print ("mgmt_cli add access-rule layer \"network\" position bottom " + str(CaseMulti(name,"name")) +  str(CaseMulti(source, " source")) + str(CaseMulti(destination, " destination")) +  str(CaseMulti(protocole, " service")) +   ' track.type "Log" ' + str(CaseMulti(action, " action")) )
 except :
  numberofrows +=1

STYLING
According to PEP-8, there are a few improvements to be made to your code:

Imports: Your imports should go in alphabetical order
Variable/Function Names: You should use snake_case instead of camelCase
Parameter Names: This may just be how I view your code, but how your parameters are named look like your taking in objects. You should use lowercase variables for better readability.
Spacing: You should use 4 spaces for each indent. For variable declaration and assignment, the = should be separated by a space on each side for readability, like so: s=0 to s = 0.
Main Guard: You should wrap any regular code in a if 'name == 'main' guard, to make sure that the code is only being run when that file is the main file, and not being imported by anything.

STRING FORMATTING
All throughout your code, you put strings together using + and .format. Luckily there is an easier way, by putting an f before the string like so: f"content here". This erases the need to have verbose + in your strings, and you no longer have to cast int to str with this method, you can just write the code in the brackets {your code here}.
Example:
return "name " + '"{}"'.format(cell_content)
to
return f"name \"{cell_content}\""
Here is the updated code:
import re
import xlrd

loc = ("Dir\StackoverflowExample.xlsx")

wb = xlrd.open_workbook(loc)
sheet = wb.sheet_by_index(0)
sheet.cell_value(0, 0)

# Extracting number of rows

def case_multi(cell_content, cell_name):
    #takes a cell and a string as parameters, prints a pattern like Cell.X ="string", X starts with 0 and ends at the number of line in the Cell
    case = str(cell_content)
    x = re.split ("\s", case)   #splits the cell contents in a list using regex where space is the separator for the list's elements
    if name == cell_content:  # deals with the cell with spaces between objects no clue how it work but it does
        return f"name \"{cell_content}\""
    else:
        s = 0
        string = ""
    if str(x[0]).lower() == "any":
        return " "
    else:
        if len(x) == 1: # case where the cell has one line (or a word with no spaces)
            string = f"{cell_name} \"{case}\""
        else: # case where multilines/mutli spaces
            while s < len(x):
                listelement = x[s]
                string += f"{cell_name}.{str(s + 1)} \"{listelement}\""
                s += 1

        return string

def main():

    for number_of_rows in range(5):

        print "Rule " + str(number_of_rows)
        name= sheet.cell_value(number_of_rows, 1)
        source = sheet.cell_value(number_of_rows, 2)
        destination = sheet.cell_value(number_of_rows, 3)
        protocole = sheet.cell_value(number_of_rows, 4)
        log = sheet.cell_value(number_of_rows, 5)
        action = sheet.cell_value(number_of_rows, 6)
        #firewall = sheet.cell_value(number_of_rows, 7)

        try:
            print ("mgmt_cli add access-rule layer \"network\" position bottom " + str(case_multi(name,"name")) +  str(case_multi(source, " source")) + str(case_multi(destination, " destination")) +  str(case_multi(protocole, " service")) +   ' track.type "Log" ' + str(case_multi(action, " action")) )
        except:
            number_of_rows += 1

if __name__ == '__main__':
    main()

It seems a main thing you need to learn about is f-strings

These are an update to the old format method.

print "Rule " + str (numberofrows)

becomes

print(f"Rule str(numberofrows)")

and

"name " + '"{}"'.format(CellContent)

becomes

f"name {CellContent}"

NOTE Python 3 uses () for print statements

It is also worth using a PEP8 checker (such as black - pip install black) to make your code more readable

You could also change things like

s=s+1

to

s += 1

Rather than saying

anycells = ["any","Any", "ANY"]
if str(x[0]) in anycells
...

You could say

if str(x[0]).lower() == "any"

Are you sure this is your code?
This looks very much like Python 2 code which hasn't been used for over 6 years. I am curious why you would write something in Python 2 still? It seems odd to learn Python 2 as a new beginner


Determine if two arrays are a rotated version of each other Following algorithm has time complexity \$O(N)\$ and space complexity \$O(1)\$.

Any suggestions to improve this algorithm or any alternate approach suggestions?

public static boolean isRotatedVersion(int[] a, int[] b){
        //Check length of arrays
        if(a.length != b.length)
            return false;

        //Declare two pointers. One for each array
        int aPointer = 0;
        int bPointer = 0;

        //This flag will prevent us walking through the 2nd array in a loop only once
        //Helps avoid going into infinite walk loop
        boolean isFirstTimeWalk = true;

        //This loop will run from 0 to n of array 1
        while(aPointer < a.length){

            //If elements at position aPointer and bPointer are equal then incr both pointer
            // to move to the element in both array
            if(a[aPointer] == b[bPointer]){
                aPointer++;
                if(bPointer < b.length-1)
                    bPointer++;
                else{ //If reached end of second array then it reset pointer to 0 to rotate
                    //Once rotated done mark is1stTimeWalked to false so avoid future loops
                    isFirstTimeWalk = false;
                    bPointer = 0;
                }
            }else if(isFirstTimeWalk && bPointer < b.length-1){
                //Move bPointer until a match is found
                bPointer++;
            }else {
                return false;
            }
        }
        return true;
    }
}

Bug

I ran your code with the following arrays and your function thought that the two arrays were rotated versions of each other but they were not:

public static void main(String [] args)
{
    int [] a = { 0, 1, 2, 3, 0 };
    int [] b = { 0, 1, 1, 2, 3 };
    System.out.println(isRotatedVersion(a, b));
}

Output:

  true

Achieving a correct \$O(n)\$ solution

Determining if an array is a rotated version of another array is equivalent to searching for the first array within the second array concatenated with itself.  For example, given:

a = { 0, 0, 1, 2, 0 }
b = { 0, 0, 0, 1, 2 }

You can search for pattern a within this array, which is b doubled:

{ 0, 0, 0, 1, 2, 0, 0, 0, 1, 2 }

To do this in \$O(n)\$ time, you just need to implement a linear time string searching algorithm.  I would suggest using the Knuth Morris Pratt algorithm, which is easy to understand and implement.

somehow your code looks quite complex an is hard to understand although you added lots of comments.

I tried to create my own version following the TDD work flow (test methods have been created in order of appearance and logic has been improved with each failing test):

public class IsRotatedArrayCheckTest {

    private static class IsRotatedArrayCheck {
        public boolean isRotatedVersion(int[] a, int[] b) {
            if (a.length != b.length)
                return false; // arrays of differing lenght cannot be rotated versions
            for (int offset = 0; offset < a.length; offset++) { // rotation may start at any offset
                if (isRotatedWithOffset(a, b, offset))
                    return true;
            }
            return false; // all indexes checked with no success
        }

        private boolean isRotatedWithOffset(int[] a, int[] b, int offset) {
            for (int index = 0; index < a.length; index++) {
                if (a[index] != b[calculateRotatedIndex(offset, a.length, index)])
                    return false;
            }
            return true; // all values matched with this offset
        }

        private int calculateRotatedIndex(int offset, int arrayLength, int currentIndex) {
            return (offset + currentIndex) % arrayLength;
        }
    }

    // test code begins here
    private static final IsRotatedArrayCheck IS_ROTATED_ARRAY_CHECK = new IsRotatedArrayCheck();

    @Test
    public void same_array_is_rotated() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 4, 5 };
        int[] test2 = test1;
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertTrue("same array is rotated", isRotatedArray);
    }

    @Test
    public void shorter_array_is_not_rotated() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 4, 5 };
        int[] test2 = Arrays.copyOfRange(test1, 2, test1.length);
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertFalse("arrays not same size", isRotatedArray);
    }

    @Test
    public void longer_array_is_not_rotated() throws Exception {
        int[] test2 = new int[] { 0, 1, 2, 3, 4, 5 };
        int[] test1 = Arrays.copyOfRange(test2, 2, test2.length);
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertFalse("arrays not same size", isRotatedArray);
    }

    @Test
    public void not_same_numbers_is_not_rotated() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 4, 5 };
        int[] test2 = Arrays.copyOf(test1, test1.length);
        test2[2] = 7;

        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertFalse("arrays have differing values", isRotatedArray);
    }

    @Test
    public void no_repeated_digits() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 4, 5 };
        int[] test2 = new int[] { 1, 2, 3, 4, 5, 0 };
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertTrue("no repeated digits offset 1", isRotatedArray);
    }

    @Test
    public void repeated_digits() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 2, 5 };
        int[] test2 = new int[] { 1, 2, 3, 2, 5, 0 };
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertTrue("repeated digits offset 1", isRotatedArray);
    }

    @Test
    public void repeated_digits2() throws Exception {
        int[] test1 = new int[] { 0, 1, 2, 3, 2, 3 };
        int[] test2 = new int[] { 2, 3, 2, 3, 0, 1 };
        boolean isRotatedArray = IS_ROTATED_ARRAY_CHECK.isRotatedVersion(test1, test2);
        assertTrue("repeated digits offset 2", isRotatedArray);
    }
}

What do you think about that?

  This O(N^2) algorithm. Cannot be the best solution. – AKh

There is no solution for the problem with less than O(N^2) (worst case).

The reason is that you have to compare each an every element of both arrays to prove that they are equal with the chosen offset. There are some special cases (which are somewhat likely) where the values within the array differ enough so that you can aboard the compare after having checked enough values.

eg with the input

 a= { 1, 2, 3, 4, 5, 6, 7}
 b= { 7, 1, 2, 3, 4, 5, 6}

you need only one check per offset -> O(N)

but with

 a= { 1, 1, 1, 1, 1, 1, 7}
 b= { 1, 1, 1, 1, 1, 7, 1}

you need to check (almost) all elements ->O(N^2). The fact that you safe one single compare does not change the time complexity...

You might come up with the idea to check the array for possible duplicated sub arrays, but that would only add another power to N (at least).

However: Time complexity always considers the worst case, therefor the solution cannot be better than O(N^2).

The OP stated that her algorithm is has time complexity of O(N) but that is not true. She might have based that conclusion on the fact that there is only one loop visible, but her code does the second loop implicitly.


Reversing words in a string I have to reverse the string "He is the one" to "one the is He".  I have written some programs in Java but am looking for other best solutions.  Suggest any possible ways to minimize the current program.

First approach:

class StringRev{
    public static void main(String args[]){
    String str = "He is the one";
    String temp = "";
    String finalString = "";
        for(int i =str.length()-1;i>=0;i--){
            temp +=i!=0?str.charAt(i):str.charAt(i)+" ";
            if(str.charAt(i) == ' '||i==0){
                for(int j=temp.length()-1;j>=0;j--){
                    finalString += temp.charAt(j);
                }
                temp = "";
            }
        }
            System.out.println(finalString);
    }
}

Second approach:

class StringRev2{
    public static void main(String args[]){
    String str[] = "He is the one".split(" ");
    String finalStr="";
        for(int i = str.length-1; i>= 0 ;i--){
            finalStr += str[i]+" ";
        }
        System.out.println(finalStr);
    }
}

"Minimize" is a really bad word to use when trying to consider the quality of your code.  It's vague, and it can lead to undesirable outcomes, such as broken or unusable code.

For example, is your goal to minimize the number of bytes used storing the source code on a hard drive? That might have been important when paper tape was a popular storage medium, but it's irrelevant today.  It's so irrelevant that the difference between these two files is less than the size of a sector, so both actually occupy the same amount of storage space.

Is your goal to reduce the number of CPU cycles spent?  Given that cell phones have 100MHz processors these days, you need to do more work to figure out if the effort you expend thinking about the problem will ever be made up for in efficiency experienced by your users.  If this is run once or twice, efficiency simply is not important. On the other hand, if it could ever become part of a networking protocol, efficiency is extremely important.  Quite honestly, English word reversal seems to be too specialized to fit the test of practicality for efficiency.

In general, what most people should be striving for in their code is "correctness" and "clarity".  You want to know that it works correctly in all situations.  The answer to that is to write unit tests.  For clarity, you want the code to be readable, understandable, and usable.  Make sure you have chosen good names.  Modularize the functions.  For example, you should consider extracting the dependency on System.out.println, as outputting the string has nothing to do with reversing the string.

Approach 1:

JDK provides java.util.StringTokenizer to do the split or tokenizing a given text. This API helps avoid going character by character. No external jar/libraries required. Tokenizing gives you each separated word as an array element that you could print in reverse.

import java.util.ArrayList;
import java.util.StringTokenizer;

public class reverseString {
    public static void main(String args[])
    {
            StringTokenizer st = new StringTokenizer("this is a string");
            ArrayList<String> arrLstStrings = new ArrayList<>();
            while (st.hasMoreTokens()) {
                arrLstStrings.add(st.nextToken());
            }

            for(int loop=arrLstStrings.size()-1;loop>=0;loop--)
                System.out.println(arrLstStrings.get(loop));
    }
}

Approach 2:

import java.util.regex.*;

public class reverseString {
    public static void main(String args[])
{
        String strSource = "This is a string";
        // give a max int as limit in next stmt.
        String[] tokens = Pattern.compile(" ").split(strSource,15) ;
        for (int loop=tokens.length-1;loop>=0;loop--)
            System.out.println(tokens[loop]);
    }
}


Angular2/4 broadcast service using RxJS I am no expert in RxJS as I am still learning my way around the new paradigm of Observable/Subject.

I was trying to implement a broadcast service (based on NgRadio) that allows a new subscriber to get the nth last value of events he was listening to and the next emitted values after subscription.

As I stumbled upon issues with ReplaySubject to ask for the last n values... I ended up implementing it as follows. I feel that the code is not that crisp and clean so I would appreciate any improvement from more experienced RxJS developers.

import { BehaviorSubject } from 'rxjs/BehaviorSubject';
import { Observable } from 'rxjs/Observable';
import { ReplaySubject } from 'rxjs/ReplaySubject';
import { Subject } from 'rxjs/Subject';
import * as _ from 'lodash';
export interface RadioEvent {
    key: string;
    data?: any;
}

export class RadioService {
    private separator = ':';
    private _eventBus = new Subject<RadioEvent>();
    private _eventCacheBus = new ReplaySubject<RadioEvent>();

    keyMatch(key, wildcard) {
        var w = '*';
        var ww = '**';
        var partMatch = function (wl, k) {
            var match = (wl === w) || (wl === k);
            return match;
        };
        var sep = this.separator;
        var kArr = key.split(sep);
        var wArr = wildcard.split(sep);
        var kLen = kArr.length;
        var wLen = wArr.length;
        var max = Math.max(kLen, wLen);
        for (var i = 0; i < max; i++) {
            var cK = kArr[i];
            var cW = wArr[i];
            // '**' match all gragments
            if (cW == ww && (typeof cK !== 'undefined')) {
                return true;
            }
            // test if fragments match
            if (!partMatch(cW, cK)) {
                return false;
            }
        }
        return true;
    };

    cast<T>(key: string, data?: any) {
        if (typeof key !== 'string' || !key.length) {
            throw 'Bad key. Please provide a string';
         }

        this._eventBus.next({ key: key, data: data });
        this._eventCacheBus.next({ key: key, data: data });
    };

    on<T>(key: string, count?: number) {
        var _this = this;
        var normalobs = this._eventBus
        .filter(function (event: RadioEvent) {
            return _this.keyMatch(event.key, key);
        }).map(function (event) {
            return event.data;
        });
        if (_.isNil(count)) {
            return normalobs;
        } else {
            let obs = this._eventCacheBus
            .filter(function (event: RadioEvent) {
                return _this.keyMatch(event.key, key);
            }).map(function (event) {
                return event.data;
            });
            let subject = new ReplaySubject<T>(count);
            obs.subscribe(value => {
                subject.next(value);
            })
            return Observable.merge(normalobs, subject).distinct();
        }
    }
}

First of all, I recommend you to ensure your code is correct (see my comment on your question).

Here are a few suggestions about improving your code.

Please do not use non-informative names like w, ww, sep, kArg, cW, CK, and even the "well-known" i, j, k, m, n. They are absolutely horrible. They make decent code be an unreadable puzzle. They hide the intent and make me think where I should not be.
Please always make sure you use Type when a) the compiler loses (or is unable to infer) the type information; b) you can narrow down the type. That includes the function return type. Typing is not panacea but it helps prevent an entire class of bugs.
Do not use generic methods smth<T>(...) unless you use the type T information anywhere in the function.
Minimize the distance between variable declaration, assignment, and first usage. Better declare and assign in the same place. Ideally, find a way to not even declare a variable.
In error messages be more specific about what went wrong.

  const star = '*';
  const twoStars = '**';

  export class RadioService {
    private separator = ':';
    private _eventBus = new Subject<RadioEvent>();
    private _eventCacheBus = new ReplaySubject<RadioEvent>();

    keyMatch(keyList: string, wildcardList: string): boolean {
      const isMatch = (wildcard, key) => (wildcard === star) || (wildcard === key);

      const allKeys = keyList.split(this.separator);
      const allWildcards = wildcardList.split(this.separator);

      const keyWildcardPairs = allKeys
        .reduce((accum, current, index) => {
          accum.push([current, allWildcards[index]]);
          return accum;
        }, <string[][]>[]);

      return keyWildcardPairs.some(pair => {
        const key = pair[0];
        const wildcard = pair[1];
        // '**' match all gragments
        if (wildcard == twoStars && (typeof key !== 'undefined')) {
          return true;
        }
        // test if fragments match
        if (!isMatch(wildcard, key)) {
          return true;
        }
        return false;
      });
    };

    cast(key: string, data?: any): void {
      if (typeof key !== 'string' || key.length <= 0) {
        throw `Bad key '${JSON.stringify(key)}'. Please provide a non-empty string.`;
      }

      this._eventBus.next({ key: key, data: data });
      this._eventCacheBus.next({ key: key, data: data });
    };

    on(key: string, count?: number): Observable<any> {
      const _this = this;

      const normalObservable = this._eventBus
        .filter(event => _this.keyMatch(event.key, key))
        .map(event => event.data);

      const withoutReplay = _.isNil(count);
      if (withoutReplay)
        return normalObservable;

      const subject = new ReplaySubject<number>(count);
      this._eventCacheBus
        .filter(event => _this.keyMatch(event.key, key))
        .map(event => event.data)
        .subscribe(value => subject.next(value));

      return Observable
        .merge(normalObservable, subject)
        .distinct();
    }
  }

Thanks for your reply. To be honest part of the code related the key matching algorithm was copied so I did not take the time to clean it up. This is code in case someone wants to use it with correction to your review keymatch implementation (using .every()):

import { Injectable } from '@angular/core';
import { BehaviorSubject, Observable, ReplaySubject, Subject } from 'rxjs/Rx';
import * as _ from 'lodash';

const star = '*';
const twoStars = '**';

export interface RadioEvent {
    key: string;
    data?: any;
}

export class RadioService {
    private separator = ':';
    private _eventBus = new Subject<RadioEvent>();
    private _eventCacheBus = new ReplaySubject<RadioEvent>();

    public on(key: string, count?: number): Observable<any> {
        const _this = this;

        const normalObservable = this._eventBus
            .filter((event: RadioEvent) => _this.keyMatch(event.key, key))
            .map((event: RadioEvent) => event.data);

        const withoutReplay = _.isNil(count);
        if (withoutReplay) {
            return normalObservable;
        }

        const subject = new ReplaySubject<number>(count);
        this._eventCacheBus
            .filter((event: RadioEvent) => _this.keyMatch(event.key, key))
            .map((event: RadioEvent) => event.data)
            .subscribe((value) => subject.next(value));

        return Observable
            .merge(normalObservable, subject)
            .distinct();
    }

    public cast(_key: string, _data?: any): void {
        if (typeof _key !== 'string' || _key.length <= 0) {
            throw new Error(`Bad key '${JSON.stringify(_key)}'. Please provide a non-empty string.`);
        }

        this._eventBus.next({ key: _key, data: _data });
        this._eventCacheBus.next({ key: _key, data: _data });
    }

    private keyMatch(eventKey: string, listenerKey: string): boolean {
        const isMatch = (wildcard, key) => (wildcard === star) || (wildcard === key);

        const eventKeyArray = eventKey.split(this.separator);
        const listenerKeyArray = listenerKey.split(this.separator);

        const keyWildcardPairs = eventKeyArray
            .reduce((accum, current, index) => {
                accum.push([current, listenerKeyArray[index]]);
                return accum;
            }, [] as string[][]);

        let isWildcard = false;
        return keyWildcardPairs.every((pair) => {
            const key = pair[0];
            const wildcard = pair[1];
            // '**' match all fragments
            if (wildcard === twoStars && (typeof key !== 'undefined')) {
                isWildcard = true;
                return true;
            }
            // test if fragments match
            if (isMatch(wildcard, key) || isWildcard) {
                return true;
            }
            return false;
        });
    }
}


Golang execute several go routine in loop and then wait I'm new in Go and excited with its easy-to-use concurrency implementation. However I'm not sure if I'm doing it right in Golang way.
Consider the code where I spawn process(i int, ch chan int) every loop as separate goroutine. Then the result which comes from the channel will be consumed by consume(ch chan int, wg *sync.WaitGroup).
I know that the channel is blocking, so let's implement buffered channel. And of course I don't want the process terminated before every goroutine finished the operation, so I add WaitGroup.

func main() {
    var wg sync.WaitGroup
    n := 5
    ch := make(chan int, n)
    for i := 0; i < n; i++ {
        println("Processing ", i)
        go process(i, ch)
        go consume(ch, &wg)
    }
    println("Finished the process")
    wg.Wait()
}

func consume(ch chan int, wg *sync.WaitGroup) {
    wg.Add(1)
    defer wg.Done()
    println("Result ", <-ch)
}

func process(i int, ch chan int) {
    ch <- (i * 5)
}

Am I doing it right? Or there are better way to do this?
Many thanks!

Every time you execute a go statement it is passed to the scheduler. What if scheduling is delayed? wg.Add(1) is not executed and wg.Wait() is true. For example, run your code in the Go Playground where GOMAXPROCS is 1.

package main

import (
    "sync"
)

func main() {
    var wg sync.WaitGroup
    n := 5
    ch := make(chan int, n)
    for i := 0; i < n; i++ {
        println("Processing ", i)
        go process(i, ch)
        go consume(ch, &wg)
    }
    println("Finished the process")
    wg.Wait()
}

func consume(ch chan int, wg *sync.WaitGroup) {
    wg.Add(1)
    defer wg.Done()
    println("Result ", <-ch)
}

func process(i int, ch chan int) {
    ch <- (i * 5)
}

Playground: https://play.golang.org/p/dQ_lFRz2Y8a

Output:

Processing  0
Processing  1
Processing  2
Processing  3
Processing  4
Finished the process

Make sure that all the wg.Adds are run before wg.Wait. Move the println("Finished the process") to the correct place after the wg.Wait. For example,

waiting.go:

package main

import (
    "runtime"
    "sync"
)

func main() {
    println("GOMAXPROCS", runtime.GOMAXPROCS(0))
    var wg sync.WaitGroup
    n := 5
    ch := make(chan int, n)
    for i := 0; i < n; i++ {
        println("Processing ", i)
        go process(i, ch)
        wg.Add(1)
        go consume(ch, &wg)
    }
    wg.Wait()
    println("Finished the process")
}

func consume(ch chan int, wg *sync.WaitGroup) {
    defer wg.Done()
    println("Result ", <-ch)
}

func process(i int, ch chan int) {
    ch <- (i * 5)
}

Playground: https://play.golang.org/p/3czBixAjxdT

Output:

GOMAXPROCS 1
Processing  0
Processing  1
Processing  2
Processing  3
Processing  4
Result  0
Result  5
Result  10
Result  15
Result  20
Finished the process

Run the Go data race detector to check for data races. It finds none.

$ go run -race waiting.go
GOMAXPROCS 4
Processing  0
Processing  1
Result  0
Processing  2
Result  5
Processing  3
Result  10
Processing  4
Result  15
Result  20
Finished the process
$

Given that you seem to be experimenting, it's hard to say if the pattern you've employed is correct since there really isn't context. But this is largely a valid use of channels. There is one problem though: you have a race condition. More on that in a bit.

Using sync.WaitGroup is the correct approach here. Ideally, you should thread this through all goroutines that you spawn. In this case, because of the data dependency through ch (namely that there are exactly n processes and n consumers and each sends/receives one int), we know that if all of the consumers finish (and call the deferred wg.Done()) there can't be any processes running. But often, for larger projects such a dependency may not be as obvious, and may change if you alter other parts of your codebase. So, in any nontrivial application where all channel usages aren't limited to a small area of the code, you probably want to thread the sync.WaitGroup through all goroutines spawned.

A note on blocking: you made a vague statement about channel blocking. To clarify, make(chan int, 5) produces a buffered channel which only blocks if more than 5 ints have been sent but not received. make(chan int) produces a channel that blocks until a consumer receives the value. In your example, both of these will work.

The race condition:

Doing wg.Add(1) inside the goroutine is racy. Why? Because once you do go consume(ch, &wg) you can't say for sure that the next thing that will happen is the wg.Add(1). In fact, you can't even be sure that that will run at all (we could reach the end of main()). In a degenerate case, if you reached the end of main() and wg.Wait() was called before a wg.Add(1) from a goroutine, the counter in the Wait() wouldn't account for this goroutine. You need to do the wg.Add(1) before you spawn the goroutine:

wg.Add(1)
go consume(ch, &wg)

Since you know there will be n goroutines spawned, you could also do wg.Add(n) before the loop, but if that loop can exit prematurely, it is more wise (and clear) to do the wg.Add(1) before each go consume().

You should also probably defer wg.Wait() right after var wg sync.WaitGroup in main(). The point of defer is to keep resource allocation and cleanup close so you don't forget it!

Also, you should get in the habit of using directional channels. They give you some safety that you don't accidentally receive from producers or send from consumers. They also quickly communicate to readers of your code what each function does at a high level.

func consume(ch <-chan int, wg *sync.WaitGroup) {
    // ...
}

func process(i int, ch chan<- int) {
    // ...
}


Get the length of data within a buffer (by getting the position of the last byte with data) Background. I start out with a completely empty byte[4096]. As data streams into my application, the byte[4096] gets filled with data starting from first position.

This results in a byte array that is partially filled, but mostly empty, like so...

                0  1  2  3  4  5  6  7  8                       4096
byte[4096] --> [+][+][+][+][+][+][+][+][+][ ][ ][ ][ ][ ][ ] ... [ ]

[+] = Data
[ ] = Empty

I am trying to quickly obtain the position of the last byte with data.

My Question for Code Review

I'm looking for feedback for performance and possibly any correctness in unanticipated cases of this code block:

/// <summary>
/// Get the position of the last byte with data
/// </summary>
public static int GetPositionOfLastByteWithData(this byte[] array)
{
    int i = array.Length - 1;
    // find the end of the buffer by iterating backwards
    while (array[i] == 0) --i;

    return i;
}

Note: This method will throw an exception in the event that the byte[] passed in is totally blank/empty. In the actual use, I do validate that there is data in the byte array before it gets to this method. This method is aimed squarely for high performance.

If your array is completely "empty" your current method will throw an exeption because it will try to get array[-1].

Just use a for loop starting at 0 and if you find the first 0 return index-1. For the case that the whole buffer is filled return array.Length - 1.

Update:

As it seems it is possible that there are 0 values in between valid data like [+][+][+][+][ ][ ][ ][+][+][+][ ]...[ ]. Hence going from start to end of the array doesn't seem a reliable action.

Using a for loop instead of using the while loop enables your code to do the empty check as well like so

public const int NotFound = -1;
public static int GetPositionOfLastByteWithData(this byte[] array)
{
    for (int i = array.Length - 1; i > -1; i--)
    {
        if (array[i] > 0) { return i; }
    }
    return NotFound;
}

As you are talking about streams in your question, wouldn't it be better to store the index of the last written byte ? This would be the easiest and the most safe variant because, assuming you are reading exactly 10 bytes from the stream and later on you need exactly these 10 bytes to restore the read data. If the last byte of that stream would be a 0 like
[+][+][+][+][+][+][+][+][+][ ][ ][ ][ ][ ]...[ ]

both methods, yours and mine, would return 8 but the really read byte count would be 9.

To increase performance you could use the concept of Binary search:

look at the length/2 index (= Split the array in two parts)
If this value contains data take the right array-part, if not the left and start at step 1 again.
if your remaning array has length 1 you found your index

I did not test my 'code' but you will get the idea, how it works. This code will only work if you have no empty indices between the data.