Efficient way to sort Matrix elements into separate bins in Python I'm sorting matrix elements into separate bins.

The approach is: If the difference of each element and the 1st element in existing list is smallest, then add it to the current list. If not, create a new list and add the element there.

Following is my code which I'm applying on a small 3X3 test matrix of random numbers, as is evident, I'm using multiple for loops and and if staements to achieve this and hence it isn't memory efficient and is consuming a lot of time to execute. Is there a better way to handle this in python? Please recommend.

import numpy as np
import sys
import matplotlib.pyplot as plt

A = np.random.random((3, 3))

#Initialize the first cluster
k1 = [A[0,0]]
#Initialize the set of clusters
K = [k1]
#Threshold value
Theta = 0.3

counter = 1

for i in range(A.shape[1]):
    for j in range(A.shape[0]):
        diff = []
        for l in K:
            diff_l = abs(A[i,j] - l[0])
            diff.append(diff_l)
            mini = min(float(s) for s in diff)
            x = diff.index(min(diff))
            if mini < Theta:
                K[x].append(A[i,j])
            else:
                counter = counter+1
                new_list = 'k'+str(counter)
                new_list= []
                new_list.append(A[i,j])
                K.append(new_list)

print len(K)

Here are some alternative clustering (binning) methods.

I assumed that the line mini = min(float(s) for s in diff) and the following code was indented incorrectly. If the indent was correct then that is your problem.  If not, here are three implementations that might speed up the work:

Method #1:

Here is an implementation of the original code with redundancies removed and a comprehension for the inner loop:

def method1():
    K = [[A[0, 0]]]
    for element in A.flatten():
        min_diff = min((abs(element - l[0]), i) for i, l in enumerate(K))
        if min_diff[0] < Theta:
            K[min_diff[1]].append(element)
        else:
            K.append([element])

Method #2:

This is bit more complex, in that it keeps a sorted list of the first elements, and for each element first does an efficient lookup to find the two closest first elements, and then performs the distance/Theta calc against only the two closest.

import bisect
def method2():
    global A
    A = np.random.random(A.shape)
    data_iter = iter(A.flatten())
    K = [[next(data_iter)]]
    first_elements = [K[0][0]]
    for element in data_iter:
        x = bisect.bisect_left(first_elements, element)
        if x == 0:
            # below lowest value
            min_diff = abs(element - first_elements[0]), x
        elif x == len(first_elements):
            # above highest value
            min_diff = abs(element - first_elements[-1]), x-1
        else:
            min_diff = min((abs(element - l[0]), i)
                           for i, l in enumerate(K[x-1:x+1]))
        if min_diff[0] < Theta:
            K[min_diff[1]].append(element)
        else:
            first_elements.insert(x, element)
            K.insert(x, [element])

Method #3:

I was not quite sure why you are using the first element found outside of the range of theta to define the grouping center, so I tried a small experiment.  This sorts the data, and then builds clusters that are built around 90% of theta above the minimum value in each cluster.  So the results will not match the other methods, but it is quite a bit faster.

def method3():
    global A
    A = np.random.random(A.shape)
    K = []
    target = -1E39
    for element in np.sort(A.flatten()):
        if element - target > Theta:
            target = element + Theta * 0.9
            K.append([])
        K[-1].append(element)

Timing Test Code:

import numpy as np
A = np.random.random((100, 100))

# Threshold value
Theta = 0.03

from timeit import timeit

def runit(stmt):
    print("%s: %.3f" % (
        stmt, timeit(stmt + '()', number=10,
                     setup='from __main__ import ' + stmt)))

runit('method0')
runit('method1')
runit('method2')
runit('method3')

Run Times

4 Methods:

0. Original
1. Original recast to more Pythonic
2. Use a search to minimize the number of comparisons
3. Sort the values, and then cluster

For the run times, I had to decrease Theta.  With the original theta there are only very small number of clusters with the data given.  If the number of clusters is small method #2 is not a lot faster.  But as the number of clusters goes up method #2 will get much faster than methods #0 and #1.  Method #3 (the sort) is fastest by far.

method0: 5.641
method1: 4.430
method2: 0.836
method3: 0.057

(Seconds, small numbers are better)


Simple GUI unit converter with tkinter I happened to delve into tkinter and created a couple of mini projects so I could get more familiar with it. I was wondering if there was any portion of my code or code structure that is sloppily written and could be more python-esque. As a beginner in both python and tkinter (been learning during this quarantine) it would be great if you fellow coders would be willing to give me some pointers about my code which will benefit me in the future (breaking bad habits now rather than later, creating good habits instead). Thanks!

from tkinter import *

window = Tk()
window.title("Basic Converter")

window.geometry("500x300+500+350")

#globally declare measurement variables
measurement1 = ""
measurement2 = ""

def convert_SI(val, unit_in, unit_out):     #based on unitconverters.net
    SI = {'Meter':1, 'Kilometer':1000, 'Centimeter':0.01, 'Millimeter':0.001,
          'Micrometer':0.000001, 'Mile':1609.35, 'Yard':0.9144, 'Foot':0.3048,
          'Inch':0.0254}
    return val*SI[unit_in]/SI[unit_out]

def helpsection():
    pass    #put helpful info text here (e.g. no entering in right entry box else error)

def selectedInput():
    global measurement1
    measurement1 = listbox.get(listbox.curselection())#whatever is currently selected

def selectedOutput():
    global measurement2
    measurement2 = listbox1.get(listbox1.curselection()) #whatever is currently selected

def converter():
    try:
        global measurement1, measurement2
        result.set(str(convert_SI(float(inputEntry.get()), measurement1, measurement2)))

    except:
        result.set("Error")

title = Label(window, text="Basic Converter", font="Calibri 16")
title.grid(columnspan=3)
result = StringVar()    #initalize dispalyed output variable
#create a top-level menu
filemenu = Menu(window)
filemenu.add_command(label='Help', command=helpsection)
window.config(menu=filemenu)    #displays menu
#input and output entry fields
inputEntry = Entry(window)
inputEntry.grid(row=1, column=0)
arrow = Label(window, text="--->", font="Calibri 20").grid(row=1, column=1)
outputEntry = Entry(window, textvariable=result).grid(row=1, column=2)

convertButton = Button(window, text='Convert!', command=converter).grid(row=2, column=1)

#if nonetype error, because .grid needs to be called on their own line since it always returns None
scrollbar = Scrollbar(window)   #left scrollbar
scrollbar.grid(row=2, column=0, sticky = NE + SE)
listbox = Listbox(window, exportselection=False)   #left listbox
#exportselection option in order to select 2 different listbox at same time
listbox.grid(row=2, column=0)

measurement_list = ['Meter', 'Kilometer', 'Centimeter', 'Millimeter',
                    'Micrometer', 'Mile', 'Yard', 'Foot', 'Inch']
for measurement in measurement_list:
    listbox.insert(END, measurement)
listbox.bind("<<ListboxSelect>>", lambda x: selectedInput())   #this instead of command= option
# attach listbox to scrollbar
listbox.config(yscrollcommand=scrollbar.set)
scrollbar.config(command=listbox.yview)

scrollbar1 = Scrollbar(window)   #right scrollbar
scrollbar1.grid(row=2, column=2, sticky = NE + SE)   #add sticky if scrollbar not showing
listbox1 = Listbox(window, exportselection=False)   #right listbox
listbox1.grid(row=2, column=2)

for measurement in measurement_list:
    listbox1.insert(END, measurement)
listbox1.bind("<<ListboxSelect>>", lambda x: selectedOutput())
listbox1.config(yscrollcommand=scrollbar1.set)
scrollbar1.config(command=listbox1.yview)

#configure grid layout to adjust whenever window dimensions change
for i in range(3):
    window.grid_rowconfigure(i, weight=1)
    window.grid_columnconfigure(i, weight=1)

window.mainloop()

Don't use wildcard imports.

Import tkinter with import tkinter as tk, and then prefix all of your calls to tkinter classes with tk.. For example:

import tkinter as tk

window = tk.Tk()
...
title = tk.Label(window, text="Basic Converter", font="Calibri 16")
filemenu = tk.Menu(window)

...

PEP8 is a set of guidelines that most python programmers follow, and it recommends against wildcard imports.

Don't call grid or pack inline

In python, x().y() always returns the value of y(). In tkinter, grid, pac, and place all return None. Thus, when you do this:

arrow = Label(window, text="--->", font="Calibri 20").grid(row=1, column=1)

... all you're doing is setting arrow to None, which renders arrow useless. Plus, doing it inline (or worse, sometimes in line and sometimes now) makes the code hard to read.

Which leads us to the next point...

Separate layout from widget creation.

I see this mistake all the time: create a widget, call grid, create a widget, call grid, ... This results in code that is hard to read, and makes it hard to visualize the layout. If you have to change the layout, such as changing the order of two rows, it's easier if those two rows are together in the code rather than intermixed with other code.

As a rule of thumb, create all widgets within a given group together, and lay them out together.

For example:

title = Label(window, text="Basic Converter", font="Calibri 16")
inputEntry = Entry(window)
arrow = Label(window, text="--->", font="Calibri 20")
convertButton = Button(window, text='Convert!', command=converter)
outputEntry = Entry(window, textvariable=result)
scrollbar = Scrollbar(window)   #left scrollbar
...
title.grid(columnspan=3)
inputEntry.grid(row=1, column=0)
arrow.grid(row=1, column=1)
convertButton..grid(row=2, column=1)
outputEntry.grid(row=1, column=2)
scrollbar.grid(row=2, column=0, sticky = NE + SE)
...

In this specific example everything is in the root window so it doesn't matter quite so much, but if you used multiple frames for organizing your widgets this would make it much easier to see which widgets were grouped together.

Make your UI responsive

When I resized your window, the layout didn't respond very well. This is what I see when I resize the program; notice how the scrollbars are the wrong size, the listboxes didn't expand, etc.

Tkinter makes this easy to do, but you have to use the options tkinter gives you. Think about what should happen when the user resizes the program. This becomes much easier when you group all of your calls to grid together.


Getting the unique identifier from a given login username Are there any issues in this code?

/// <summary>
/// This function gets the unique identifier of a member by the given
/// login user name of that member on our web site.
/// </summary>
/// <param name="userName">The username</param>
public static string GetMemberNumberByUserId(string userName, string connectionString)
{
    try
    {
        SqlConnection database = new SqlConnection(connectionString);
        SqlCommand command = new SqlCommand("GetMemberInfoByUserID", database);

        command.CommandType = CommandType.StoredProcedure;
        command.Parameters.AddWithValue("@userID", userName);
        database.Open();
        SqlDataReader reader = command.ExecuteReader();
        reader.Read();
        string memberNumber =
            reader.GetString(reader.GetOrdinal("memberNumber"));

        // Make sure the member number is a 11 or 12 digit number
        if (Regex.IsMatch(memberNumber, @"^(000|100)\d{6,7}(?!00)\d{2}$"))
        {
            return memberNumber;
        }
        else
        {
            return "";
        }
    }
    catch(Exception)
    {
        return null;
    }
}

catch(Exception)
{
    return null;
}

You should never do this. Exception means that there is some sort of problem. And you shouldn't ignore problems, you should learn about them and do something about them as soon as possible.

What you should do is that at first, don't catch any exceptions. Then, if you find out that some exceptions actually do happen under normal conditions, then handle those, but only those.


Brainf**k to Ruby converter -- v2 Previous iteration.

You know, I think this is the fastest I've ever pushed out an update to anything. This is Version 2 of my Brainf**k to Ruby converter, and the generated code looks... Well, like Brainf**k, converted directly to Ruby, with no attempt at making it more readable.

I'm looking for any tips on making things more idiomatic, both in the generator and generated code. The nest if/cases really bug me, but I'm not quite sure how to get rid of them, especially since just two characters are blindly replaced. I'd also like advice on making it run faster.

bf_to_ruby.rb

input_file, output_file = ARGV
code = IO.read(input_file).tr('^+-<>.,[]', '')

open(output_file, File::CREAT | File::WRONLY) do |output|
  output.puts <<-END.gsub(/^[ \t]*\||\s*#@.*$/, '')
    |#!/usr/bin/env ruby
    |class Mem < Hash #@ `Hash` because it's more memory-efficient and allows negative values.
    |  def initialize; super(0); end
    |  def []=(i, val); super(i, val & 255); end
    |end
    |data = Mem.new
    |pointer = 0
  END
  indent_level = 0
  code.scan(/(\++)|(\-+)|(<+)|(>+)|([.,\[\]])/)
      .map do |string|
        if string[0]
          next "#{'  ' * indent_level}data[pointer] += #{string[0].length}"
        elsif string[1]
          next "#{'  ' * indent_level}data[pointer] -= #{string[1].length}"
        elsif string[2]
          next "#{'  ' * indent_level}pointer -= #{string[2].length}"
        elsif string[3]
          next "#{'  ' * indent_level}pointer += #{string[3].length}"
        elsif string[4]
          case string[4]
            when '['
              ret = "#{'  ' * indent_level}until data[pointer] == 0"
              indent_level += 1
              next ret #Split it so that it's clear that indent is increased *after* the line
            when ']'
              indent_level -= 1
              next "#{'  ' * indent_level}end"
            when ','
              next "#{'  ' * indent_level}data[pointer] = $stdin.readbyte"
            when '.'
              next "#{'  ' * indent_level}putc data[pointer]"
          end
        end
      end.each { |line| output.puts(line) }
end

Demo

Input:

++++++++[>++++[>++>+++>+++>+<<<<-]>+>+>->>+[<]<-]>>.>---.+++++++..+++.>>.<-.<.+++.------.--------.>>+.>++.

Output:

#!/usr/bin/env ruby
class Mem < Hash
  def initialize; super(0); end
  def []=(i, val); super(i, val & 255); end
end
data = Mem.new
pointer = 0
data[pointer] += 8
until data[pointer] == 0
  pointer += 1
  data[pointer] += 4
  until data[pointer] == 0
    pointer += 1
    data[pointer] += 2
    pointer += 1
    data[pointer] += 3
    pointer += 1
    data[pointer] += 3
    pointer += 1
    data[pointer] += 1
    pointer -= 4
    data[pointer] -= 1
  end
  pointer += 1
  data[pointer] += 1
  pointer += 1
  data[pointer] += 1
  pointer += 1
  data[pointer] -= 1
  pointer += 2
  data[pointer] += 1
  until data[pointer] == 0
    pointer -= 1
  end
  pointer -= 1
  data[pointer] -= 1
end
pointer += 2
putc data[pointer]
pointer += 1
data[pointer] -= 3
putc data[pointer]
data[pointer] += 7
putc data[pointer]
putc data[pointer]
data[pointer] += 3
putc data[pointer]
pointer += 2
putc data[pointer]
pointer -= 1
data[pointer] -= 1
putc data[pointer]
pointer -= 1
putc data[pointer]
data[pointer] += 3
putc data[pointer]
data[pointer] -= 6
putc data[pointer]
data[pointer] -= 8
putc data[pointer]
pointer += 2
data[pointer] += 1
putc data[pointer]
pointer += 1
data[pointer] += 2
putc data[pointer]

You'll notice that the generated code is nearly the same as the last version's generated code, but with a lot of duplicate lines merged and indenting.

You're correct that nesting a case in an elsif ladder is a bit clunky here, but the two things that jump out at me as making the code difficult to comprehend are: 1) we have to know everything that's going on with the Regexp passed to scan() in order to figure out the intent of the if clauses, and 2) the variable string is not a String, but an Array.

You can remove the capture groups from your Regexp to just get an Array of Strings back from scan() (instead of an Array of Arrays of Strings), and figure out what exactly was matched by looking at the strings themselves - which means you don't need a nested case anymore, and makes it a little more obvious what scan() is doing without having to actually parse its argument.

Like this:

# example.rb
code = '--+++.,,'
code.scan(/\++|\-+|[.,]/).map { |str|
  case str[0]
  when '+'
    "Plus signs: #{str.length}"
  when '-'
    "Minus signs: #{str.length}"
  when '.'
    'Dot!'
  when ','
    'Comma!'
  end
}.each {|ll| puts ll}

Produces:

$ ruby example.rb
Minus signs: 2
Plus signs: 3
Dot!
Comma!
Comma!


Implementation of static_for to iterate over elements of std::tuple using C++17 features I have implemented a static_for loop to iterate over the elements of an std::tuple using C++17 features. Here is what I have currently:

#include <iostream>
#include <utility>
#include <tuple>

template <std::size_t I>
struct wrapper
{
    static constexpr std::size_t n = I;
};

template <class Func, std::size_t ...Is>
constexpr void static_for_impl( Func &&f, std::index_sequence<Is...> )
{
    ( f( wrapper<Is>{} ),... );
}

template <std::size_t N, class Func>
constexpr void static_for( Func &&f )
{
    static_for_impl( f, std::make_index_sequence<N>{ } );
}

int main()
{
    auto t = std::make_tuple( 1, 22, 3, 4 );

    std::size_t weighted = 0;
    static_for<4>( [&](auto w)
    {
        weighted += ( w.n + 1 ) * std::get<w.n>( t );
    });

    std::cout << "Weighted: " << weighted << std::endl;

    return 0;
}

Demo

My questions are:

Is there a more standard way that this can be implemented (like maybe using std::apply or invoke)?
Is there another idiom that would be better than the ugly wrapper struct that can be used to access the index at compile time from within a lambda?

Accepted modification:
Incorporated all the suggested modifications here.

IMHO, I think this is indeed pretty ugly. Assuming you just want to iterate over all of the elements of tuple (which is what you've done here), you can simplify this both in implementation and in usage:

template <class Tup, class Func, std::size_t ...Is>
constexpr void static_for_impl(Tup&& t, Func &&f, std::index_sequence<Is...> )
{
    ( f(std::integral_constant<std::size_t, Is>{}, std::get<Is>(t)),... );
}

template <class ... T, class Func >
constexpr void static_for(std::tuple<T...>&t, Func &&f)
{
    static_for_impl(t, std::forward<Func>(f), std::make_index_sequence<sizeof...(T)>{});
}

int main()
{
    auto t = std::make_tuple( 1, 22, 3, 4 );

    std::size_t weighted = 0;
    static_for(t, [&] (auto i, auto w) { weighted += (i+1) * w; });

    std::cout << "Weighted: " << weighted << std::endl;

    return 0;
}

This is more like for_each, or maybe for_each + enumerate from python. On the implementation side, you no longer need this wrapper stuff. On the usage side, the lambda just directly gets passed the index and the the element, so it doesn't need to extract it from the tuple. Note that I pass the integer by "type" so that you can do crazy things with it but for most purposes it would probably just be simpler to pass an actual size_t as the first argument. Though integral_constant does implicitly convert anyhow.

If you wanted to iterate over multiple tuples or something like that (very rare), you could take my approach and modify it so that the lambda only gets passed the first argument, and then captures the tuple and uses std::get<i> to index into it. But you still don't need the wrapper struct or w.n.


Sending a string with a certain path I want to send a string with the following path, and I want to verify if this is the right way to do that in JavaScript.

Is this the right way to use the return in an if/else, or there is shorter way?
Is the indexOf using correctly?

https://test.com/plugins/plug/pluginr/poc/fact

function isExt(path) {

  if(path.indexOf("/plug/pluginr/")) {

    return true;

  }
  else {

    return false;

  }

}

Your code seems not to be correct. In case you don't have the substring /plug/pluginr/ it would still return true since indexOf returns an integer containing the position of the first ocurrence. If theres no ocurrence of the substring then it returns -1 which is evaluated to true in JavaScript.

You better use this instead;

function isExt(path){
    return path.indexOf("/plug/pluginr/") > -1;
}


Horrible nested if statement for changing CSS based on RadioButton selection I'm writing away for this website, and I've come into a problem. I need to display only one td based on the corresponding radio button pressed. However, there are two 'master' radio buttons that dictate how many radio buttons are shown, and after that, which are selected. - if that makes any sense!

Now, I am only showing a very small snippet of what I actually have at the moment - this nested if statement goes on 5 more times. So, as you can probably imagine, it looks horrendous. And I'm not sure if there is a better way, maybe there is a way to create functions that can be called? What do you all think?

$('input:radio').change(function () {
                if (this.id == prodMoveRadio) {
                    if ($("#ExtIdTd").is(":visible") === true) {
                        $("#ExtIdTd").css({ visibility: "hidden" });
                        $("#LocIdTd").css({ visibility: "visible" });
                        $("#" + locIdRadio).prop("checked", true);
                        $("#ExtIdTr").css({ visibility: "hidden" });
                    }
                    else {
                        $("#MainContent_DetailsPanel").css({ visibility: "hidden" });
                        $("#ExtIdTr").css({ visibility: "hidden" });
                        $("#LocIdTd").css({ visibility: "visible" });
                        $("#" + locIdRadio).prop("checked", true);
                    }
                }
                else if (this.id == opMoveRadio) {
                    if ($("#ExtIdTd").is(":visible") === false) {
                        $("#ExtIdTr").css({ visibility: "visible" });
                        $("#ExtIdTd").css({ visibility: "visible" });
                    }
                    else {
                        $("#MainContent_DetailsPanel").css({ visibility: "visible" });
                        $("#" + extIdRadio).prop("checked", true);
                        $("#ExtIdTr").css({ visibility: "visible" });
                        $("#ExtIdTd").css({ visibility: "visible" });
                        $("#LocIdTd").css({ visibility: "hidden" });
                    }
                }

Edit

Would it be 'clean' to put a class onto the td's I need to hide, and hide that whole class then set only the visibility to visible on the td I require? Thoughts?

You could use polymorphism to represent the states, and to apply the needed formatting.


replace digit at some positions in number I'm trying to write function, that replace digits at some positions in number.

For example f 91521 [3,4] -> [91001,91111,91221,91331,91441,91551,91661,91771,91881,91991]

It's behavior like 91**1 -> ..., so I named it as a 'whitemask'.

Question 1: is "whitemask" a normal and adequately name for that function?

After that I've formulated that 'whitemask' function.

sublists :: (Eq a) => [a] -> [[a]]
sublists list = [x | x <- subsequences list, x /= []]

numbers :: [Int]
numbers =  take 10 [0..]

numLength :: Integer -> Int
numLength = length . show

decreaseList :: Num a => [a] -> a -> [a]
decreaseList = (\a b c -> map (a c) b) (flip (-))

listToNum :: [Int] -> Integer
listToNum digits = toInteger $ foldl1 (\x y -> x*10 + y) digits

numToList :: Integer -> [Int]
numToList x = map digitToInt $ show x

remove :: [Int] -> [Int] -> [[Int]]
remove [] x = [x]
remove (x:xs) list = putted : remove (decreaseList xs x) unputted
    where [putted,unputted] = (\(x,y) -> [init x,y]) $ splitAt x list

sew :: [[a]] -> a -> [a]
sew list sewer = foldr1 (\x y -> x ++ [sewer] ++ y) list

whitemask :: Integer -> [Int] -> [Integer]
whitemask num places =  filter (\x -> numLength x == numLength num ) $ map (listToNum . sew (remove places (numToList num))) numbers

But in context of all problem where that function might be used I figured out, that I need only fix-length answers. For examples, it works like

> whitemask 75148 [1,3,4]
[15118,25228,35338,45448,55558,65668,75778,85888,95998]

and answer isn't contain '5008' element.

Question 2: is that fix-length filtering 'whitemask' function's business? Or it would be better to return all possible variants and filtering them if it really needed?

When I've started to use that function in complicated iterates ways I figured out that it's might do some unnecessary and superfluous calculations.

Question 3: how can I refactor 'whitemask' function with less-usage folds maps and recursions?

Any cosmetics and style advices are appreciated.

ps: did I select the right place to asking for? (question could be migrated to SO actually)

UPD:
I figured out that sew can be written much easier.

sew list sewer = concat $ intersperse [sewer] list

I didn't thought about the algorithm itself, but shortened the code a little bit:

decreaseList :: Num a => [a] -> a -> [a]
decreaseList b c = map (subtract c) b

listToNum :: [Int] -> Integer
listToNum = toInteger . foldl1 ((+).(*10))

numToList :: Integer -> [Int]
numToList = map digitToInt . show

remove :: [Int] -> [Int] -> [[Int]]
remove [] x = [x]
remove (x:xs) list = let (putted, unputted) = splitAt x list
                     in (init putted) : remove (decreaseList xs x) unputted

sew :: [[a]] -> a -> [a]
sew list sewer = foldr1 ((++).(++[sewer])) list

whitemask :: Integer -> [Int] -> [Integer]
whitemask num places =  let  list = map (listToNum . sew (remove places (numToList num))) [0..9]
                        in filter (((==) `on` length . show) num) list

I think there is still room for improvement...

[Edit]

A shorter version:

whitemask :: Integer -> [Int] -> [Integer]
whitemask num places = map (listToNum . replace)  [0..9]  where
  replace digit = foldr ((:).findDigit) [] $ zip (numToList num) [1..] where
    findDigit (d,i) | i `elem` places = digit
                    | otherwise = d

listToNum :: [Int] -> Integer
listToNum = toInteger . foldl1 ((+).(*10))

numToList :: Integer -> [Int]
numToList = map digitToInt . show

[Edit]

A shorter version, with fixed-length filtering:

whitemask :: Integer -> [Int] -> [Integer]
whitemask num places = map replace digits  where
  digits = if 1 `elem` places then [1..9] else [0..9]
  toList = map digitToInt $ show num
  replace digit = foldl toNum 0 $ zip toList [1..] where
    toNum r (d,i) = 10*r + (fromIntegral $ if i `elem` places then digit else d)

I can't promise that I'll end up with a one-liner, though...

To your questions:

Depends on the context. If you are unsure about the name, just explain the function with a comment.
Depends if you would ever need unfiltered output. In this case that looks pretty unlikely, so I would include the filtering.
See above.


Thumbnail caption in vue.js with possibly missing fields in caption I am using the following to output a thumbnail with text. It works fine but I don't like having the use a conditional to check every prop before outputting.

E.g. this.post.user.name would indicate "cannot read property of null".
Some of the values coming from the database would be null.

Whats the standard procedure to do this?

<template>

{{ post.make }}

{{ post.model }}

{{ userName }}

</template>

By using v-if I was able to come up with a solution I am comfortable with.

<template>

{{ post.make }}

{{ post.model }}

{{ post.price }}

{{ post.user.name }}

</template>

I also considered not allowing null in the database so I would not have to check for it but that needed some special attention.


Finding all root-to-leaf paths in a binary tree The task is to return all root-to-leaf paths, given a binary tree (from leetcode).

This is my approach.

Take a helper array and a counter, keeping track of what has been traversed so far. If it is leaf node, print all till that point.
Reduce counter.

Please suggest improvements, if any.

import java.util.LinkedList;
import java.util.List;

public class LeafNode {

    private List<String> list;
    private int count = -1;//counts no. of element in print array
    private int[] printArray;// a helper array class to print nodes

    public List<String> binaryTreePaths(TreeNode root) {
        list = new LinkedList<>();
        if(root == null){

            return list;
        }
        printArray = new int[10000];// assuming the maximum size will be less than 10000
        printList(root);
        return list;

    }

    private void printList(TreeNode root) {
        printArray[++count] = root.val;
        if(root.left == null && root.right == null) {
            printTillNow();
        } else{
            if(root.left != null) {
                printList(root.left);
            }
            if(root.right != null){
                printList(root.right);
            }

        }
        --count;
    }

//just prints all the nodes so far
    private void printTillNow() {
        if(count < 0){
            return;
        }
        StringBuilder sb = new StringBuilder();
        int i =0;
        for(i = 0; i<count; i++){
            sb.append(printArray[i]);
            sb.append("->");
        }
        sb.append(printArray[i]);
        list.add(sb.toString());

    }

}

Names

What is the purpose of this class? My first thought when I see a class named LeafNode is that it's a model of a leaf node in a tree. But this class is not about modeling trees. The purpose of this class seems to be to contain the method binaryTreePaths, which returns a list of paths to all leaf nodes of the Tree parameter it receives.

A better (but still not great) name for this class would have been the name of the exercise: BinaryTreePaths.

The term "print" comes up several times, but there's nothing in the exercise about printing. The task is to return a list of strings. No printing. The overuse of this term throughout the implementation is misleading, confusing, noise.

Choice of storage

If you don't know the required size of a collection in advance, prefer a List, which can dynamically resize itself, such as an ArrayList, instead of a fixed size array. ArrayList exists exactly for this purpose.

Fragile state tracking

This class has a state, represented by the fields list, count, printArray. It's difficult to follow the state changes, as all methods may (and do) change these variables.

I recommend using so-called accumulator parameters. You pass an accumulator variable to recursive method calls, which append values appropriately. Consider this alternative implementation, using paths as an accumulator variable:

public List<String> binaryTreePaths(TreeNode root) {
    if (root == null) {
        return Collections.emptyList();
    }

    List<String> paths = new ArrayList<>();
    binaryTreePaths(root, "" + root.val, paths);
    return paths;
}

public void binaryTreePaths(TreeNode root, String prefix, List<String> paths) {
    if (root.left == null && root.right == null) {
        paths.add(prefix);
        return;
    }
    if (root.left != null) {
        binaryTreePaths(root.left, prefix + "->" + root.left.val, paths);
    }
    if (root.right != null) {
        binaryTreePaths(root.right, prefix + "->" + root.right.val, paths);
    }
}

This improves in the original in simplicity.

Alternative implementation

Another variation on the previous solution, but using a StringBuilder instead of string concatenation:

public List<String> binaryTreePaths(TreeNode root) {
    if (root == null) {
        return Collections.emptyList();
    }

    List<String> paths = new ArrayList<>();
    binaryTreePaths(root, new StringBuilder("" + root.val), paths);
    return paths;
}

public void binaryTreePaths(TreeNode root, StringBuilder builder, List<String> paths) {
    if (root.left == null && root.right == null) {
        paths.add(builder.toString());
        return;
    }
    if (root.left != null) {
        int len = builder.length();
        binaryTreePaths(root.left, builder.append("->").append(root.left.val), paths);
        builder.setLength(len);
    }
    if (root.right != null) {
        binaryTreePaths(root.right, builder.append("->").append(root.right.val), paths);
    }
}


Java binary search (and get index if not there) review I want to do binary search and if the target is not there get the index of where it should have been.

This is the code I came up with. It seems correct. Is there any problem? Can it be improved?

private int binarySearch(String[] sortedArray, String target, int start, int end) {

  if(start <= end){
     int mid = (start + end)/2;
     if(sortedArray[mid].equals(target)){
         return mid;
     }
     else if (target.compareTo(sortedArray[mid]) < 0){
         return binarySearch(sortedArray, target, start, mid - 1);
     }
     else{
         return binarySearch(sortedArray, target, mid + 1, end);
     }
  }

  return start;
}

2 minor things:

Remove the unnecessary comparison. If

sortedArray[mid].equals(target)

fails, it will again compare the two strings in the next if condition. Instead, you can just do:

int c = target.compareTo(sortedArray[mid]);
if(c == 0)
  return mid;
else if(c < 0)
  ...

To keep the method signature simple, you can overload it and delegate to the one that does the work:

public int binarySearch(String[] sortedArray, String target) {
  binarySearch(sortedArray, target, 0, sortedArray.length - 1);
}


String compaction algorithm I've been working on improving my C coding. I wanted to write two string algorithms:

to trim white space from the beginning and end of a string
to compact a string

I have a few problems with this code that I was hoping to get some advice on:

The casts from const char * to char *. Is there a clean and idiomatic way of cleaning those up?
The string compaction algorithm requires extra storage and I couldn't think of a clean way to do the compaction in place. Is there a clean way of doing it in place?

I'm looking for a review on algorithmic correctness, code style, design, and any other improvements you can think of. My main concern is performance!

#include <stdlib.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <assert.h>

static const char TERMINATOR = '\0';

/* skips space in @param[str] depending on the provided direction
   @param[in] -- str -- the string
   @param[in] -- direction -- the direction to skip space in
                 1 -- forward direction
                 0 -- back direction
   @return -- pointer to first non-space character in the string if
              @param[direction] and @param[str] are valid or the original
              string otherwise
*/
const char *skip_space(const char *str, int direction)
{
    if (direction == 1) {
        while (isspace(*str)) { ++str; }
    } else if (direction == 0) {
        while (isspace(*str)) { --str; }
    }
    return str;
}

const char *ltrim_white(const char *str, size_t len)
{
    char *end = (char *)&str[len - 1];
    end = (char *)skip_space(end, 0);
    *++end = TERMINATOR;
    return skip_space(str, 1);
}

const char *trim_white(const char *str)
{
    return ltrim_white(str, strlen(str));
}

char *compact_with_mem(char *str)
{
    str = (char *)skip_space(str, 1);

    size_t len = strlen(str);
    if (len == 0) {
        char *c = malloc(1);
        if (c) {
            *c = TERMINATOR;
        }
        return c;
    }

    char *back = (char *)skip_space(&str[len - 1], 0);
    *++back = TERMINATOR;

    char *buf = malloc((back - str) + 1);
    printf("Allocated %u memory\n", (back - str) + 1);
    char *copy = buf;

    while (*str) {
        if (isspace(*str) && isspace(*(str + 1))) {
            *copy++ = *str;
            str = (char *)skip_space(str, 1);
        } else {
            *copy++ = *str++;
        }
    }
    *copy = TERMINATOR;
    return buf;
}

void print(const char *s)
{
    const size_t len = strlen(s);
    printf("|%s", s);
    if (*(s + len) == TERMINATOR) { // redundant but just for testing
        putchar('N');
    }
    puts("|");
}

void test(char *s, char *cmp_str)
{
    const size_t blen = strlen(s);
    printf("Base string: |%s|\n", s);
    print(s);

    char *ret = (char *)compact_with_mem(s);
    const size_t len = strlen(ret);
    printf("Length before compaction: %u\nLength after compaction: %u\n", blen, len);

    print(ret);
    puts("");
    assert(strcmp(ret, cmp_str) == 0);
    free(ret);
}

int main()
{
    char s1[] = "   hello!";
    char s2[] = "   world.   ";
    char s3[] = "lol..";
    char s4[] = "";
    char s5[] = "               ";
    char s6[] = "lmfao  ";
    char s7[] = "         x";
    char s8[] = "ab   ";
    char s9[] = "   i am cthulhu   ";
    char s10[] = "  all    your    base are belong    to us      ";
    char s11[] = "  random    chars \\29289892 *&@ @)!  !*  # $ _ ! & # * !   @!  ^ *#^ | {][";
    char s12[] = "omfgwhyaretherenospaceshere!";

    test(s1, "hello!");
    test(s2, "world.");
    test(s3, "lol..");
    test(s4, "");
    test(s5, "");
    test(s6, "lmfao");
    test(s7, "x");
    test(s8, "ab");
    test(s9, "i am cthulhu");
    test(s10, "all your base are belong to us");
    test(s11, "random chars \\29289892 *&@ @)! !* # $ _ ! & # * ! @! ^ *#^ | {][");
    test(s12, "omfgwhyaretherenospaceshere!");
}

The functionality you have implemented in your “compact” is generally known as white-space normalization.

Get rid of redundant functions

You have a lot of functions. While breaking complicated logic up into manageable bits is a good thing, there is also a point where it becomes too much and hurts maintainability rather than helping it.

For example, the only use of ltrim_white is in trim_white which consists of a single line calling that function. You could have equally well put all the code into trim_white directly. ltrim_white also is a misnomer because unlike its name suggests, it trims white-space from both sides, not just from the left.

const char *skip_space(const char *str, int direction)
{
    if (direction == 1) {
        while (isspace(*str)) { ++str; }
    } else if (direction == 0) {
        while (isspace(*str)) { --str; }
    }
    return str;
}

This function doesn't really buy you anything. Making a function call

s = (char *) skip_space(s, 1);

isn't really simpler than writing

while (isspace(*s)) ++s;

directly. And the latter form saves you the headache figuring out what that 1 as second parameter means. As a bonus, it saves a branch and avoids the const trouble. If you really want a dedicated function, provide skip_space_forward and skip_space_backward instead of introducing the second parameter.

Make auxiliary functions static

Those auxiliary functions that you determine are in fact useful and should be kept, should still be private to your implementation and not pollute the global name-space. Therefore, you should declare them static. As a nice bonus, this might also make your code run faster (for free).

Put the standard library to good use

The <string.h> standard library header provides highly optimized functions for dealing with memory. Use them. In particular, memmove and strlen will come in handy.

Be const-correct

As you have already realized yourself, the adding and removing of const has become a total mess. You should only add const where you can, not at all cost. It is far better to have an internal pointer being not const even if you don't need to modify the pointed-to object than it is to repetitively cast const away. Once you start casting, const doesn't buy you anything in terms of code correctness.

If you have a function like skip_space, that takes a pointer to a buffer and returns a pointer into that buffer without modifying the buffer, it is good to keep the buffer const. Unfortunately, when you call skip_space with a non-const pointer, what you get back is always const and you cannot use the pointer to write through it even though you know it would be safe. This can be solved without a cast while still keeping the buffer skip_space operates on const, though: use the returned pointer to compute an index and use that one!

char buffer[] = "the quick brown fox";         // mutable buffer
const char *const p = skip_space(buffer, 1);   // pointer to const
const ptrdiff_t i = p - buffer;                // just an integer
buffer[i] = 'X';                               // perfectly fine now

If you have a decent compiler, it should be able to optimize the additional arithmetic away while still being able to verify the const-correctness of your code.

Don't over-generalize

NUL terminated character strings are so prevalent throughout C that you can safely assume the convention is here to stay. By spelling out '\0', you make your code actually more readable than by using a constant like TERMINATOR.

If you really want generality, make the caller pass in a pair of pointers. (If you have ever used the C++ <algorithm> standard library, this will be very familiar to you.) Then your function also works with arrays that are not terminated at all. You can provide a convenience function that only accepts a single pointer (that has to point to a terminated array in this case, of course) and uses rawmemchr (if you have it) or strlen to find the end. Then it calls the generic two-argument version.

Actually, a more useful generalization would probably be to accept a function pointer to use in place of isspace. Such indirect calls can have a performance penalty, though.

Consider pointer arithmetic

Instead of &p[n - i] you can just write p + n - i. I find this much more readable.

Modify pointers with care

Your trim_white function trims space from the beginning by simply adding an offset to the pointer. While this is super-fast, there is a big downside. If I call your function with a pointer that I obtained through malloc, I still have to keep the original pointer along with the pointer to the trimmed buffer. I need the former to pass it to free when I no longer need the string. I'm sure I will confuse them all the time.

Instead of advancing the start pointer, you could actually move the bytes of the string to the beginning. While this might seem costly at first, note that it doesn't actually increase the asymptotic complexity of your function. You already do at least one linear traversal (via strlen) and in the worst case (string is all-white-space) a second one finding the cutoff points. So adding a third one is not that bad. Actually, since you only need to move those bytes that are not white-space, the moving cost balances with the testing cost so you traverse the string exactly twice (regardless of its data) and the worst-case cost remains the same.

strlen             |---------------------------------------------------->|
skip_forward       |------------------->                                 |
skip_backward      |                                           <---------|
data               |␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣␣The␣quick␣brown␣fox␣...␣␣␣␣␣␣␣␣␣␣|
memmove            |                    <----------------------          |

Be sure to move the bytes with memmove rather than memcpy so it will be valid to pass the same buffer as input and output. (memcpy may only be used if the buffers are guaranteed to not overlap.)

Let the caller decide how to allocate memory

A white-space trimming / normalization function can be useful in a lot of contexts even when dynamic memory is not available or desirable. Allocating memory should not be one of your function's concerns. Instead, have the caller pass in a buffer where you will place the result into.

Since trimming / normalizing white-space can reduce but never enlarge the length of a string, a caller can easily determine the needed size of the buffer upfront by passing a buffer as long as the input string. They can even pass the same pointer twice and trim / normalize the string in-place! If the buffer was dynamically allocated and the string contained a lot of white-space, the caller might decide to realloc it afterwards so to give back the now-unused memory. Or maybe they have other smart options available. Again, your function should not be concerned with this. String manipulation already is a full-time job.

If you let the caller pass in the source and destination buffer, you don't have to return it any more. Instead, you can return something more interesting. For example, a pointer to the end of the result string. This could be useful if the caller wants to know the length of the new string. Having the pointer to its end, they can do so with a single subtraction rather than having to loop over the bytes (aka call strlen).

Recognize state machines

Some algorithms can be written in a more compact way if you think of them as state machines. Normalizing white-space is such an algorithm. It iterates over all characters of the input from left-to right and performs an action on each character depending on its internal state. There are three states.

PRE where all we have seen so far is white-space. As long as we continue to see white-space, we do nothing. Once we see a character that is not white-space, we copy it to the output buffer and switch to the
INTRA state where the last character we have seen was not white-space. If we see another white-space character, we copy it to the output buffer and remain in the INTRA state. If we encounter a white-space character, we transition into the
INTER state where the last character we have seen was white-space but we have also already seen non-white-space characters before. If we see a white-space character in this state, we do nothing. If we see a non-white-space character, we first append a single blank to the output buffer and then the current character; then we go back into the INTRA state.

Note that the only time we actually do something when we see white-space is in the INTRA state.

Not only is implementing the algorithm as such a state machine simpler and more elegant, it will also be more performant because we only iterate over each (the input and the output) buffer exactly once from left to right. Caching hardware loves such memory access and will honor it with amazing speed.

Put it all together

Combining all of the advice from this review, I'd suggest you'll end up with an implementation like this.

For trimming…

char *
trim_space(const char *const src, char *const dst)
{
  size_t first = 0;
  size_t last = strlen(src);
  size_t length = last - first;;
  if (length > 0)
    {
      while (isspace(src[first]))
        ++first;
      while ((last > first) && isspace(src[last - 1]))
        --last;
      length = last - first;
      memmove(dst, src + first, length);
    }
  dst[length] = '\0';
  return dst + length;
}

…and for normalizing white-space.

char *
normalize_space(const char * src, char * dst)
{
  typedef enum { states_pre, states_intra, states_inter } states;
  states state = states_pre;
  int c;
  while ((c = *src++) != '\0')
    {
      if (isspace(c))
        {
          if (state == states_intra)
            state = states_inter;
          continue;
        }
      switch (state)
        {
        case states_pre:
          *dst++ = c;
          state = states_intra;
          break;
        case states_intra:
          *dst++ = c;
          break;
        case states_inter:
          *dst++ = ' ';
          *dst++ = c;
          state = states_intra;
          break;
        }
    }
  *dst = '\0';
  return dst;
}


Linq to entities query for donor registrations I'm using Linq-to-Entities for querying, and MVC.NET and C# for coding. I'm trying to generate monthly donor registry report which needs donor count, donors who signed through web,donors who registered in current month and all this is to be display per county.

private MonthlySummaryViewModel
BuildMonthlySummaryViewModel(MonthlySummarySubModel model){

// Get the bounding datetimes for the previous and lookup month

DateTime curr = new DateTime(model.Year, model.Month, 1),
prev = curr.AddMonths(-1),
next = curr.AddMonths(1);
var monthlySummary = (from county in context.Counties.AsEnumerable()
                      let query = context.DonorStatusActions.Where(x => x.Donor.CountyId == county.CountyId && x.ActionTakenOn >= prev && x.ActionTakenOn < next)

let data = query.GroupBy(x => x.Donor, x => x, (key, grp) => new {Item1 = key, Item2 = grp.OrderByDescending(y => y.ActionTakenOn).FirstOrDefault()})
                                  .Where(x => x.Item2.IsDonor && ((model.WebSignUp != "2" && x.Item2.Donor.WebSignUp == (model.WebSignUp == "1" ? true : false)) || model.WebSignUp == "2"))
                                  let registrations = data.Select(x => x.Item2)
                                  let previous = registrations.Count(x => x.ActionTakenOn >= prev && x.ActionTakenOn < curr)                                                                                let current = registrations.Count(x => x.ActionTakenOn >= curr && x.ActionTakenOn < next)
select Tuple.Create(county.Name, previous, current)).ToList();

 return new MonthlySummaryViewModel()
 {
 Month = new System.Globalization.DateTimeFormatInfo().GetMonthName(model.Month),
Year = model.Year,
WebSignUp=DropDownLists.SignUpMethodList.First(m => m.Value==model.WebSignUp).Text,CountyData = monthlySummary,
TotalPrevious = monthlySummary.Sum(x => x.Item2),
TotalReport = monthlySummary.Sum(x => x.Item3), };}

First thing to do, is to layout the query so that you can actually read it. Optimizing something you can't read is not really possible.

var monthlySummary =
    (from county in context.Counties.AsEnumerable()
     let query = context.DonorStatusActions
                        .Where(x => x.Donor.CountyId == county.CountyId
                                 && x.ActionTakenOn >= prev
                                 && x.ActionTakenOn < next)
     let data = query
         .GroupBy(x => x.Donor, x => x, (key, grp) => new
             {
                 Item1 = key,
                 Item2 = grp.OrderByDescending(y => y.ActionTakenOn)
                            .FirstOrDefault()
             })
         .Where(x => x.Item2.IsDonor
                 && ((model.WebSignUp != "2"
                     && x.Item2.Donor.WebSignUp == (model.WebSignUp == "1" ? true : false))
                 || model.WebSignUp == "2"))
     let registrations = data.Select(x => x.Item2)
     let previous = registrations.Count(x =>
         x.ActionTakenOn >= prev && x.ActionTakenOn < curr)
     let current = registrations.Count(x =>
         x.ActionTakenOn >= curr && x.ActionTakenOn < next)
     select Tuple.Create(county.Name, previous, current)
    ).ToList();

Yikes!

Ok let's remove the fluff.

from county in context.Counties.AsEnumerable()

Counties is already an Enumerable - it's an IQueryable<County>, and by soft-casting it like this, you've told LINQ "don't bother translating that to T-SQL, give me all counties and let LINQ-to-Objects deal with it instead" (see this answer).

If you ran a SQL profiler when your code runs, I bet you're not going to see the T-SQL you'd expect.

Just do this instead:

from county in context.Counties

This part needs a cleanup:

 .Where(x => x.Item2.IsDonor
         && ((model.WebSignUp != "2"
             && x.Item2.Donor.WebSignUp == (model.WebSignUp == "1" ? true : false))
         || model.WebSignUp == "2"))

I'm scratching my head over this one:

x.Item2.Donor.WebSignUp == (model.WebSignUp == "1" ? true : false)

Why not just do this instead?

x.Item2.Donor.WebSignUp == (model.WebSignUp == "1")

I have to say that Donor.WebSignUp being a bool while model.WebSignUp is a string, is pretty confusing.

These magic "1" and "2" values are annoying. They have a special meaning, but nobody knows what it is. Looks like a misused string that should be an int, or even better, an enum.

Regardless, the condition is redundant and could be simplified to this:

 .Where(x => x.Item2.IsDonor && (
     model.WebSignUp == "2" || x.Item2.Donor.WebSignUp == (model.WebSignUp == "1")))

Be sure to profile the generated T-SQL and show execution plan to have SQL Server tell you if any index could speed things up; I suspect you might need one on Donors.WebSignUp, too.


Porting stemming algorithm from Java to Python I have ported Stempel stemmer in Java (Apache Lucene) to Python. I come from Java world, so I'm afraid my translation might not be "pythonic" enough.

I would like to hear your feedback on quite representative part of the code, translation of Diff class that applies stemming command (diff) to a string (dest).

 @classmethod
 def apply(cls, dest, diff):
    """
    Apply the given patch string diff to the given string dest
    :param dest: Destination string
    :param diff: Patch string
    :return:
    """
    if diff is None:
        return
    if not isinstance(dest, MutableString):
        raise ValueError
    if not dest:
        return

    pos = len(dest) - 1
    try:
        for i in range(int(len(diff) / 2)):
            cmd = diff[2 * i]
            param = diff[2 * i + 1]
            par_num = ord(param) - ord('a') + 1
            if cmd == '-':
                pos -= (par_num - 1)
            elif cmd == 'R':
                cls.__check_index(dest, pos)
                dest[pos] = param
            elif cmd == 'D':
                o = pos
                pos -= (par_num - 1)
                cls.__check_index(dest, pos)
                dest[pos:o + 1] = ''
            elif cmd == 'I':
                pos += 1
                cls.__check_offset(dest, pos)
                dest.insert(pos, param)
            pos -= 1
    except IndexError:
        # swallow, same thing happens in original Java version
        pass

@classmethod
def __check_index(cls, s, index):
    if index < 0 or index >= len(s):
        raise IndexError

@classmethod
def __check_offset(cls, s, offset):
    if offset < 0 or offset > len(s):
        raise IndexError

Some justifications of decisions I took:

The original implementation uses StringBuffer to manipulate characters in a string. Since Python str type is immutable I used my own class MutableString that behaves like a Python list.
Also, original logic was based on catching IndexOutOfBoundsException. Contrary to Java, Python allows negative indexes in a list and list ranges. Therefore, I've introduces guards like __check_X.
Java implementation uses switch/case/default clause. I translated that to if/elif/else clause in Python.

Specific suggestions:

Python will throw an IndexError if you try to access an element past the end of a list, so you don't need to check index >= len(s) or offset > len(s).
After the simplification above I would inline the __check methods as assertions.
Throwing an exception and catching it again within the same context is a code smell - it's too much like goto. Why not just return instead? Or let the user know the input could not be processed?
You don't actually use any OOP in your code, so apply might as well not be within a class
In the same way I think you could just use a List[str] for dest.
Checking the type of an argument is unpythonic. Basically you're responsible for passing some value which can be used by the code. And with type checking (below) you could even enforce this at a test or linting stage.
Abbreviations make code harder to read. I would expand things like destination, position and command.
Magic values like R and D should be named constants to improve readability.

General suggestions:

black can automatically format your code to be more idiomatic.
isort can group and sort your imports automatically.
flake8 with a strict complexity limit will give you more hints to write idiomatic Python:

[flake8]
max-complexity = 4
ignore = W503,E203

That limit is not absolute by any means, but it's worth thinking hard whether you can keep it low whenever validation fails. For example, I'm working with a team on an application since a year now, and our complexity limit is up to 7 in only one place. Conversely, on an ugly old piece of code I wrote without static analysis support I recently found the complexity reaches 87!)
I would then recommend adding type hints everywhere and validating them using a strict mypy configuration:

[mypy]
check_untyped_defs = true
disallow_untyped_defs = true
ignore_missing_imports = true
no_implicit_optional = true
warn_redundant_casts = true
warn_return_any = true
warn_unused_ignores = true

As a Java developer I'm sure you'd appreciate the clarity this lends to the code.


Poor error handling: Throw inside Finally I have the following code which I am running through fortify. Why it gets marked for poor error handling, throw inside finally?

private String getResourceContent(String fileName) throws IOException {

    try (InputStream resource = ErrorResource.classLoader.getResourceAsStream(fileName)) {
        return new String(resource.readAllBytes(), StandardCharsets.UTF_8);
    } catch (NullPointerException n) {
        throw new ErrorDescriptorException(
                String.format("Error loading Error description data from Resource file [%s].", fileName), n);
    }
}

ClassLoader.getResourceAsStream() throws a NullPointerException if the given name is null, something you can easily check. InputStream.readAllBytes() does not throw a NullPointerException, and neither does instatiating the String.

So basically, the null exception will happen if and only if fileName is null, or if the resource is not found and therefore resource is null; you can easily check both without catching an exception:

private String getResourceContent(String fileName) throws IOException {
    if (fileName == null) {
        // now what?
    }
    try (InputStream resource = ErrorResource.classLoader.getResourceAsStream(fileName)) {
        if (resource == null) {
            // now what?
        }
        return new String(resource.readAllBytes(), StandardCharsets.UTF_8);
    }
}

The question now is what you want to do if fileName or resource is null. This depends on how you want your function to be used; in my opinion, you should expect the user to give a file name that is not null, so throw a NullPointerException if that is not the case. In the resource case, you could use your custom exception for that.

Now, what if readAllBytes() throws an exception? You can either let it propagate to the caller (like it's happening now) or use ErrorDescriptorException as a wrapper (but maybe this makes fortify complain again):

private String getResourceContent(String fileName) throws ErrorDescriptorException {
    if (fileName == null) {
        throw new NullPointerException();
    }
    try (InputStream resource = ErrorResource.classLoader.getResourceAsStream(fileName)) {
        if (resource == null) {
            throw new ErrorDescriptorException(
                String.format("Error loading Error description data from Resource file [%s].", fileName));
        }
        return new String(resource.readAllBytes(), StandardCharsets.UTF_8);
    }
    catch (IOException | OutOfMemoryException e) {
        throw new ErrorDescriptorException(
                String.format("Error loading Error description data from Resource file [%s].", fileName), e);
    }
}


Visualize Parts of Song as Analyzed by Echonest This is the a bit of code that works with Echonest API's pyechonest and remix libraries combined with matplotlib.pyplot to offer a simple visual representation of the start and end "parts" of a music track:

#!/usr/bin/env python
# encoding: utf=8
"""
mix_track_utils.py

Analyze music track using echonest API with pyechonest and remix libraries,
show parts using matplotlib graph.

"""
from __future__ import print_function
import echonest.remix.audio as audio
import matplotlib.pyplot as plt

usage = """
Usage:
    import mix_track_utils

    track1 = audio.LocalAudioFile('some.mp3')

    mix_track_utils(track1)
"""

def visualize_analysis(track):

    rates = ["tatums", "segments", "beats"]

    title = "Start and End Bits of {}".format(track.filename)
    plt.figure()
    plt.title(title)
    plt.axis([-1, 5, -3, 3])
    plt.grid(True)

    label_height = .2
    graph_height = 0
    for name in rates:
        label_height += .5
        graph_height += .5
        plt.text(0, label_height, 'Start ' + name.capitalize())
        for i in getattr(track.analysis, name)[:8]:
            j = (i.start, i.end)
            plt.plot(j,[graph_height,graph_height], linewidth=10)

    label_height = 0
    graph_height = -.2
    offset = None

    for name in rates:
        label_height -= .5
        graph_height -= .5
        plt.text(0, label_height, 'End ' + name.capitalize())
        for i in getattr(track.analysis, name)[:8]:
            j = (i.start, i.end)
            if offset is None:
                offset = j[0]
            k = (j[0] - offset, j[1] - offset)
            plt.plot(k,[graph_height,graph_height], linewidth=10)

    plt.show()

if __name__ == "__main__":
    print(usage)
    sys.exit(-1)

Getting pyechonest to work also requires adding an ECHONEST_API_KEY key to the environment, as can be seen from the links above.

This code is really nice, and the output is very good looking! I do have a few nitpicky things that I want to cover.

Why are you storing the usage in a variable named usage? This should be in the file's main docstring. You don't need to print usage under if __name__ == "__main__":.
Is # encoding: utf=8 really needed? It doesn't seem like you're using any special unicode characters anywhere, so remove it.
You should have a docstring in visualize_analysis. Describe what the function does, and how it does that, preferably, in detail.

That's about all I see that should be improved! If there's anything else you want me to cover, mention it in the comments, and I'll see what I can do! Hope this helps!


Two ways to randomly shuffle cards Here are two implementations I wrote to randomly shuffle cards.

The first method (shuffle) selects a random card, then swaps it to the front.
The second method (shuffle_v2) assumes that if i-1 cards are shuffled well, try to randomly swap card i with any card from 0 to i-1.

Are both methods are good to randomly (uniformly) shuffle cards?

Any ideas to improve time complexity, any code bugs or code style advice are also welcome.

import random
def shuffle(cards):
    write_index = 0 # to write,
    while write_index < len(cards):
        read_index = random.randint(write_index, len(cards)-1)
        cards[read_index], cards[write_index] = cards[write_index], cards[read_index]
        write_index += 1
def shuffle_v2(cards):
    for i in range(1, len(cards)):
        index = random.randint(0, i-1)
        cards[index], cards[i] = cards[i], cards[index]
if __name__ == "__main__":
    cards = []
    for i in range(10):
        cards.append(random.randint(1,52))
    print cards
    shuffle(cards)
    print cards
    shuffle_v2(cards)
    print cards

Add space between your functions, PEP8 suggests two spaces, but honestly any amount of space would be better than none.

There's not much of a difference between shuffle and shuffle_v2:

You've written a for loop as a while loop. Which doesn't make much sense as Pythons for loops are really clean and easy to use.
You use different bounds for your random.randint calls. shuffle is from the index to the end, for shuffle_v2 it's from the start to the index.

And so it's mostly asking which is better of the two different ranges.
Using the following, we can find which is better:

import random
from collections import defaultdict, Counter

def shuffle(cards):
    for write_index in range(len(cards)):
        read_index = random.randrange(write_index, len(cards))
        cards[read_index], cards[write_index] = cards[write_index], cards[read_index]

def shuffle_v2(cards):
    for i in range(1, len(cards)):
        index = random.randrange(0, i)
        cards[index], cards[i] = cards[i], cards[index]

if __name__ == "__main__":
    data = list(range(7))
    count = defaultdict(int)
    bound = 1000000
    for fn in [shuffle, shuffle_v2]:
        count = defaultdict(int)
        for _ in range(bound):
            d = list(data)
            fn(d)
            count[tuple(d)] += 1
        print(len(count), Counter(count.values()))

Which resulted in:

5040 {146: 1,   152: 1,   154: 2,   155: 1,   157: 1,   158: 3,   159: 4,   160: 3,
      161: 5,   162: 5,   163: 1,   164: 4,   165: 9,   166: 10,  167: 9,   168: 10,
      169: 20,  170: 27,  171: 19,  172: 17,  173: 31,  174: 37,  175: 33,  176: 30,
      177: 50,  178: 54,  179: 35,  180: 53,  181: 75,  182: 64,  183: 77,  184: 85,
      185: 90,  186: 112, 187: 104, 188: 102, 189: 128, 190: 126, 191: 131, 192: 126,
      193: 165, 194: 140, 195: 131, 196: 150, 197: 134, 198: 152, 199: 152, 200: 126,
      201: 143, 202: 140, 203: 156, 204: 134, 205: 117, 206: 118, 207: 117, 208: 112,
      209: 102, 210: 101, 211: 98,  212: 87,  213: 85,  214: 69,  215: 58,  216: 62,
      217: 45,  218: 48,  219: 48,  220: 54,  221: 35,  222: 35,  223: 34,  224: 22,
      225: 22,  226: 22,  227: 22,  228: 22,  229: 9,   230: 10,  231: 10,  232: 13,
      233: 9,   234: 3,   235: 5,   236: 7,   237: 4,   238: 5,   239: 2,   240: 1,
      241: 1,   243: 2,   244: 1,   245: 2,   248: 1,   250: 1,   253: 1}

720 {1238: 1,  1284: 1,  1287: 1,  1290: 1,  1296: 2,  1300: 1,  1305: 1,  1306: 1,
     1308: 1,  1309: 1,  1310: 1,  1311: 1,  1313: 1,  1314: 2,  1315: 1,  1316: 2,
     1317: 1,  1318: 5,  1320: 3,  1322: 2,  1323: 1,  1324: 1,  1325: 1,  1326: 2,
     1327: 2,  1328: 1,  1329: 2,  1330: 1,  1331: 6,  1332: 1,  1333: 3,  1334: 1,
     1335: 1,  1336: 9,  1337: 2,  1338: 5,  1339: 3,  1340: 3,  1341: 3,  1342: 2,
     1343: 3,  1345: 6,  1346: 4,  1347: 2,  1348: 2,  1349: 1,  1350: 8,  1351: 7,
     1352: 1,  1353: 4,  1354: 4,  1355: 3,  1356: 5,  1357: 7,  1358: 7,  1359: 8,
     1360: 4,  1361: 4,  1362: 9,  1363: 4,  1364: 4,  1365: 10, 1366: 6,  1367: 2,
     1368: 6,  1369: 9,  1370: 8,  1371: 6,  1372: 9,  1373: 9,  1374: 7,  1375: 8,
     1376: 8,  1377: 5,  1378: 10, 1379: 10, 1380: 5,  1381: 14, 1382: 8,  1383: 7,
     1384: 7,  1385: 4,  1386: 7,  1387: 10, 1388: 11, 1389: 6,  1390: 11, 1391: 2,
     1392: 7,  1393: 10, 1394: 4,  1395: 5,  1396: 12, 1397: 7,  1398: 10, 1399: 8,
     1400: 6,  1401: 8,  1402: 6,  1403: 5,  1404: 7,  1405: 7,  1406: 5,  1407: 14,
     1408: 8,  1409: 7,  1410: 5,  1411: 2,  1412: 2,  1413: 6,  1414: 6,  1415: 6,
     1416: 4,  1417: 10, 1418: 14, 1419: 8,  1420: 5,  1421: 4,  1422: 4,  1423: 3,
     1424: 6,  1425: 2,  1426: 9,  1427: 3,  1428: 6,  1429: 4,  1430: 4,  1431: 6,
     1432: 1,  1433: 4,  1434: 3,  1435: 2,  1436: 2,  1437: 1,  1438: 3,  1439: 1,
     1440: 2,  1441: 2,  1442: 2,  1443: 1,  1444: 4,  1445: 5,  1446: 2,  1447: 4,
     1448: 4,  1449: 3,  1450: 4,  1452: 2,  1453: 1,  1454: 1,  1455: 1,  1458: 1,
     1459: 3,  1460: 2,  1462: 1,  1463: 1,  1464: 1,  1465: 1,  1466: 2,  1467: 1,
     1468: 2,  1471: 2,  1472: 3,  1473: 1,  1476: 2,  1478: 1,  1483: 1,  1485: 1,
     1508: 1,  1531: 1}

This shows that the first algorithm not only had a more uniform distribution, it can actually shuffle into all 5040 different permutations. And so is the superior algorithm.


Seeking improved Objective-C permutation algorithm This algorithm seeks to find all of the possible permutations of a word:

-(void)permute:(NSMutableArray*)word position:(int)p length:(int)l{
    if(p==l){
     [self.allPermutations addObject:[word componentsJoinedByString:@""]];
    }
    else {
    for(int i = p;i<l;i++){
        NSString* t;
        t= word[p];
        word[p] = word[i];
        word[i] = t;
        [self permute:word position:p+1 length:l];
        t= word[p];
        word[p] = word[i];
        word[i] = t;
        }
    }
}

After that, I clean out the duplicates with this:

NSArray *cleanedArray = [[NSSet setWithArray:self.allPermutations] allObjects];

This starts to show a lag when the words get to 9 letters or greater. 9 letters is 362,880 loops. Any tips to get this down, or speed up this algorithm in a different way?

Before I comment on performance, I want to comment on style and naming and such.

First and foremost, let's not use single letters for variables.  They're meaningless.  It's hard to follow what's happening in your code when it's just a handful of letters.  Longer variable names has ZERO impact on the runtime performance of your code, but makes a massive difference in the readability and maintainability of your code.

But the thing is, we don't even need the p or l variables.  There's an existing Foundation struct design for EXACTLY this purposes.  It's called NSRange, which consists of two components, a location (what you're calling p) and a length (what you're calling l).

So, let's change the method name to:

- (void)permute:(NSMutableArray *)word range:(NSRange)range;

Now let's fix the internals of the method.  As a note, the following doesn't change any of the logic.  It just fixes up the spacing and naming to make the method more readable:

- (void)permute:(NSMutableArray *)word range:(NSRange)range {
    if (range.location == range.length) {
        [self.allPermutations addObject:[word componentsJoinedByString:@""]];
    } else {
        for (int i = range.location; i < range.length; ++i) {
            NSString *currentWord = word[range.location];
            word[range.location] = word[range.length];
            word[range.length] = currentWord;

            [self permute:word range:NSMakeRange(range.location + 1, range.length)];

            currentWord = word[range.location];
            word[range.location] = word[range.length];
            word[range.length] = currentWord;
        }
    }
}

Now the code is a bit more readable without having sacrificed any readability.

But there are still problems.

First of all, why are we using NSString within the loop?  It doesn't make any sense.  We're not calling any methods on these objects, we're just swapping them around.  Instead of NSString, why not just id?

Second, what happens if p is greater than word.count?  What happens if l + p is greater than word.count?  What happens if p is negative?  What happens if l is negative?  A crash happens--that's what.  If you want this method to find all permutations, then start by having an outer method which takes only an NSMutableArray argument and then calls a method that looks like this but the first call it send an NSRange object with a location of 0 and a length of word.count.

Here's another issue:

    [self permute:word position:p+1 length:l];
    t= word[p];
    word[p] = word[i];
    word[i] = t;

Are these last 3 lines ever executed in a meaningful way?  If you comment every non-brace line after the recursive call out, do the results change?  By the time you get to these three lines, you've already executed

[self.allPermutations addObject:[word componentsJoinedByString:@""]];

As far as I can tell, all those last three lines might be doing is straightening the array back out into its original form... is that the intent here?  That's crazy to have this many executions of code on the way out of the recursive call just to straighten a mutable array out back to its original form, and there's a much better way to do that!

Perhaps I'm misunderstanding the purpose of these three lines, but if I'm understanding this correctly and the only purpose truly is to rewind the array back to it's original position, then let's try this:

- (void)permute:(NSMutableArray *)word range:(NSRange)range {
    static NSMutableArray *copyOfWord;
    if (!copyOfWord) {
        copyOfWord = [word copy];
    }
    if (range.location == range.length) {
        [self.allPermutations addObject:[copyOfWord componentsJoinedByString:@""]];
        copyOfWord = nil; // We're done now.  Set back to `nil` so next call can use it
    } else {
        for (int i = range.location; i < range.length; ++i) {
            NSString *currentWord = copyOfWord[range.location];
            copyOfWord[range.location] = copyOfWord[range.length];
            copyOfWord[range.length] = currentWord;

            [self permute:copyOfWord range:NSMakeRange(range.location + 1, range.length)];
        }
    }
}


Radix sort function in R I know that sort in R uses radix sort, and I played around in implementing it (mainly to be sure I understood the algorithm properly). However, my implementation is an order of magnitude slower than the native implementation in R.

I'm curious: is there a way to implement radix sort in R so that it wouldn't be so slow?

My code:

get_digit <- function(x, d) {
    # digits from the right
    # i.e.: first digit is the ones, second is the tens, etc.
    (x %% 10^d) %/% (10^(d-1))
}

radix_sort <- function(x) {
    # k is maximum number of digits
    k <- max(nchar(x))
    for(i in 1:k) {
        x_digit_i <- get_digit(x, i)
        # split numbers based on their i digit
        x_by_bucket <- split(x, x_digit_i)
        # recombine the vectors, now sorted to the i'th digit
        x <- unlist(x_by_bucket, use.names = FALSE)
    }
    # since each iteration is a stable sort, the final result
    # is a sorted array, yay!
    x
}

Checking the running time:

> library(microbenchmark)
> x <- sample(100)
> microbenchmark(radix_sort(x), sort(x))
Unit: microseconds
          expr     min      lq      mean   median      uq     max neval cld
 radix_sort(x) 459.378 465.895 485.58322 480.4835 496.779 649.956   100   b
       sort(x)  27.314  29.487  33.05064  31.9710  33.212  73.563   100  a
> x <- sample(10000)
> microbenchmark(radix_sort(x), sort(x))
Unit: microseconds
          expr       min         lq       mean     median        uq       max
 radix_sort(x) 44317.123 44777.8965 46062.3446 45204.3715 45714.807 63838.148
       sort(x)   158.609   165.7485   198.3083   186.6995   206.099   750.832

Putting these comments into an answer...

As pointed out, it will be a tall order to beat or even approach the computation times of the native sort function, as it is compiled from C. Often with R, the trick to make your code faster consists in composing with some of these fast compiled building blocks that are native functions. In particular, we would look to substitute for loops like the one you have with vectorized functions. Unfortunately, your use of a for loop here is particular in the sense that each iteration has a side-effect to the x vector. This means that the order of the operations is important and we cannot run the loop iterations independently in parallel or via vectorized functions.

If we cannot get rid of the for loop, we are left trying to optimize the code within the body of the loop. Let's start with a profile of your code:

x <- sample(10000)
Rprof(tmp <- tempfile())
for (i in 1:10) z <- radix_sort(x)
Rprof()
summaryRprof(tmp)$by.total
#                         total.time total.pct self.time self.pct
# "radix_sort"                  8.26     99.76      0.72     8.70
# "split"                       7.34     88.65      0.06     0.72
# "split.default"               7.28     87.92      0.54     6.52
# "as.factor"                   6.74     81.40      0.08     0.97
# "factor"                      6.64     80.19      1.72    20.77
# "as.character"                4.34     52.42      4.34    52.42
# "unique"                      0.42      5.07      0.04     0.48
# "unique.default"              0.38      4.59      0.38     4.59
# "%%"                          0.14      1.69      0.14     1.69
# "get_digit"                   0.14      1.69      0.00     0.00
# "sort.list"                   0.12      1.45      0.02     0.24
# "order"                       0.08      0.97      0.06     0.72
# "unlist"                      0.06      0.72      0.06     0.72
# [...]

We see that the main culprit here is the use of split. Another surprise here is the use of order (it seems to be applied when figuring out the levels of x_digit_i) which you could consider like cheating given your objective.

So what alternative do we have to your use of split/unsplit? Essentially, you have a vector x that you want to reorder based on a vector of digits x_digit_i. One way is to use outer to create a matrix of TRUE/FALSE where each column locates a different digit (for a total of ten columns):

z <- outer(x_digit_i, 0:9, "==")

Then, you want to turn this matrix into a vector of indices, such that the first few indices will locate the zeroes, then the ones, etc (the equivalent of idx <- order(x_digit_i). You can do so by doing:

idx <- row(z)[z]

or (harder to understand but a bit faster)

idx <- 1L + (which(z) - 1L) %% length(x)

Finally, you just have to do:

x <- x[idx]

Also note that since you are dealing with integers, your code might be a little faster (and more robust by avoiding floating point errors) if you make sure to use integers everywhere possible. In particular, your get_digit function could be rewritten as follows:

get_digit <- function(x, d) (x %% as.integer(10^d)) %/% as.integer(10^(d-1))

The benchmarks below show decent progress (where my suggestions are implemented under the name sort_radix2) bridging the gap with the native sort. I hope it helps!

x <- sample(100)
microbenchmark(radix_sort(x), radix_sort2(x), sort(x))
# Unit: microseconds
#            expr     min       lq       mean   median       uq      max neval
#   radix_sort(x) 964.692 972.3675 1025.35180 984.3775 1012.178 2233.397   100
#  radix_sort2(x) 250.642 256.5720  282.58952 261.2910  282.449 1266.061   100
#         sort(x)  82.270  86.1605   92.22669  88.0230   90.943  223.249   100

x <- sample(10000)
microbenchmark(radix_sort(x), radix_sort2(x), sort(x))
# Unit: microseconds
#            expr       min         lq       mean     median         uq        max neval
#   radix_sort(x) 71939.706 76147.1715 80028.7541 78389.8140 81512.4140 144632.484   100
#  radix_sort2(x) 24218.810 27613.3190 34841.8724 29477.7115 31772.9415 143283.337   100
#         sort(x)   411.691   454.4015   563.4825   492.6165   558.0925   3412.719   100


Maximum alternating slice in an array I'm working on the problem:

  Given array with positive and negative values, return the maximum alternating slice size; two elements are alternating if they have different signs, zero is treated as both negative and positive.

  Time complexity: \$O(N)\$

  Space complexity: \$O(1)\$

I developed a working code and would like to know if there is any method to make the code shorter or faster.

public static int maxSizeAlterSlice(int[] a){
    int max_size_so_far = -1;
    int cur_max_size = -1;
    if(a.length == 1)
        return 1;
    int startInd = 0;
    int endInd = 0;
    int i=1;
    int compareInd=0;
    while(i<a.length){
        if((a[compareInd]>=0 && a[i]<=0)||(a[compareInd]<=0 && a[i]>=0)){
            compareInd=i;
            endInd++;
            i++;
        }else{
            cur_max_size = endInd-startInd+1;
            if(max_size_so_far<cur_max_size){
                max_size_so_far = cur_max_size;
            }
            startInd = i;
            compareInd=startInd;
            endInd=startInd;
            i=startInd+1;
        }
    }
    return max_size_so_far;
}

Hmm. I'm not sure about faster, but you can shorten your code by about 6 lines by doing things a little more efficiently.

First, I'd move your two initializations at the top to be below your if statement. That if statement short-circuits the whole function (which is fine), but since the statement doesn't use those two variables, we shouldn't instantiate them unless it fails to return: that's more efficient. I would also return a.length rather than 1 (to eliminate a 'magic number').

Next, you can eliminate the cur_max_size variable and its usage. Remove the declaration, then take the whole block involving cur_max_size and max_size_so_far and replace with this:

max_size_so_far = Math.max(endInd - startInd + 1, max_size_so_far);

Next, there are two places where you can use i++ inside of another statement, rather than having it stand on its own. i++ means "increment after the original value is used" so this is safe:

Inside of your if conditional (inside the while), delete i++; and change the first statement to be compareInd = i++;
Inside of your else statement, remove i = startInd + 1; and change the startInd=i; line to be startInd = i++;. (Yes, the logic works out.)


VBA UDF SUMIF with Array Parameters I was trying to give SUMIFS3D User-Defined-Function a nice review and ended up getting pretty side-tracked trying to figure out how to pass some parameters by array and whether or not I could send a 3D-range through the function (seems not).

It's a good one for review, I'm sure there are improvements - I wrote it in about 3 hours.

Function

SUMIF3D(ByVal sum_range As Range, ByVal list_Sheets As String, ByVal criteriaRange As Range, ByVal criteria As Variant, Optional ByVal isNumeric As Boolean = False)

Returns: Long

Input

Something like

=SUMIF3D(D1:D5,"sheet1,sheet2",H1:H5,I1)

To set it up just open a workbook and input:

Sheet1!D1:D5 = 1-5
Sheet2!D1:D5 = 10-50
Sheet1!H1:H5 = 1,5,10,15,20

And whatever arguments you want in Sheet1!I1=I5 e.g.

1   2   5   15  15
>0   >10 >=10    <10 20
>10

Or try your luck with matching strings.

It works on everything I've tried. It's a bad mama-jama

Option Explicit

Public Function SUMIF3D(ByVal sum_range As Range, ByVal list_Sheets As String, ByVal criteriaRange As Range, ByVal criteria As Variant, Optional ByVal isNumeric As Boolean = False) As Long
    Const OPERATORS As String = ">,<,<>,="
    Dim isPossible As Boolean
    Dim toSum() As Boolean
    Dim i As Long
    Dim j As Long

    Dim sumRangeCells() As Long
    ReDim sumRangeCells(1 To sum_range.Count, 1 To 2)
    Dim cell As Range
    i = 1
    For Each cell In sum_range
        sumRangeCells(i, 1) = cell.Row
        sumRangeCells(i, 2) = cell.Column
        i = i + 1
    Next

    Dim numberOfCells As Long
    Dim sheetsArray As Variant
    sheetsArray = Split(list_Sheets, ",")
    Dim sumRangeArray As Variant
    numberOfCells = (UBound(sheetsArray) + 1) * sum_range.Count
    ReDim sumRangeArray(1 To numberOfCells)

    Dim k As Long
    k = 1
    For i = LBound(sheetsArray) To UBound(sheetsArray)
        For j = 1 To sum_range.Count
            sumRangeArray(k) = Sheets(sheetsArray(i)).Cells(sumRangeCells(j, 1), sumRangeCells(j, 2))
            k = k + 1
        Next
    Next

    Dim critRangeArray As Variant
    critRangeArray = criteriaRange.Value2
    Dim criteriaArray As Variant
    criteriaArray = criteria.Value2

    ReDim toSum(1 To UBound(critRangeArray, 1))
    If Not IsArray(criteriaArray) Then
        If IsEmpty(criteriaArray) Then
            isPossible = False
        Else: isPossible = True
        End If
    Else
        If Not UBound(criteriaArray, 1) = UBound(critRangeArray, 1) Then
            isPossible = False
        Else: isPossible = True
        End If
    End If

    If Not isPossible Then Exit Function

    Select Case isNumeric
        Case 1
            If IsArray(criteriaArray) Then
                For i = 1 To UBound(critRangeArray)
                    If InStr(1, OPERATORS, Left$(criteriaArray(i, 1), 1)) > 0 Then
                        toSum(i) = Application.Evaluate(critRangeArray(i, 1) & criteriaArray(i, 1))
                    Else: toSum(i) = critRangeArray(i, 1) = criteriaArray(i, 1)
                    End If
                Next
            Else
                For i = 1 To UBound(critRangeArray)
                        If InStr(1, OPERATORS, Left$(criteriaArray, 1)) > 0 Then
                            toSum(i) = Application.Evaluate(critRangeArray & criteriaArray)
                        Else: toSum(i) = critRangeArray(i, 1) = criteriaArray
                        End If
                Next
            End If
        Case 0
            If IsArray(criteriaArray) Then
                For i = 1 To UBound(critRangeArray)
                    toSum(i) = critRangeArray(i, 1) = criteriaArray(i, 1)
                Next
            Else
                For i = 1 To UBound(critRangeArray)
                    toSum(i) = critRangeArray(i, 1) = criteriaArray
                Next
            End If
    End Select

    For j = LBound(sheetsArray) To UBound(sheetsArray)
        For i = 1 To UBound(toSum)
            If toSum(i) Then SUMIF3D = SUMIF3D + sumRangeArray(i + j * UBound(toSum))
        Next
    Next
End Function

A couple of things I have noticed (and I haven't tested the code):

If you make the return value a Variant, instead of Long, you can also pass Excel Errors (e.g. #VALUE!) as a result from the UDF. This would be particular useful if IfPossible is False.
You set OPERATORS up as a constant so you can check the validity of an input. But you don't use the string position at all. So, instead of Const OPERATORS As String = ">,<,<>,=", you can simply have Const OPERATORS As String = "<>=" which covers <, >, <> and =.
Having set and used OPERATORS, you only check the 1st character, so <> will never be matched.
You don't want to check for <= and `>=' as well?

Codewise:

    If Not UBound(criteriaArray, 1) = UBound(critRangeArray, 1) Then
        isPossible = False
    Else: isPossible = True
    End If

Can be simplified to

isPossible = (UBound(criteriaArray, 1) = UBound(critRangeArray, 1))

Similarily:

If IsEmpty(criteriaArray) Then
    isPossible = False
Else: isPossible = True
End If

can be simplified to:

isPossible = Not IsEmpty(criteriaArray)

That entire If-Then-Else block can become:

If Not IsArray(criteriaArray) Then
    isPossible = Not IsEmpty(criteriaArray)
Else
    isPossible = (UBound(criteriaArray, 1) = UBound(critRangeArray, 1))
End If

Which, of course, you can turn around to get rid of the Not

If IsArray(criteriaArray) Then
    isPossible = (UBound(criteriaArray, 1) = UBound(critRangeArray, 1))
Else
    isPossible = Not IsEmpty(criteriaArray)
End If

Missing something here: Select Case isNumeric. That is not how the IsNumeric(val as variant) function is used. And given this is a Boolean result, a simple if-then statement will work. I am surprised this runs without throwing an error (I am taking your word for it that it works).

I am not going to go through it here (this answer is now long enough), but that case statement needs some serious rework. You have a lot of repetition and could probably get rid of at least two levels of nesting.


Checking whether if start and end has equal number of non-zero integers and contains 3 or more 0 in the center An array is said to be hollow if it contains 3 or more zeros in the
  middle that are preceded and followed by the same number of non-zero
  elements. Write a function named isHollow that accepts an integer
  array and returns 1 if it is a hollow array, otherwise it returns 0.
  The function signature is int isHollow(int[ ] a).

  Note:  Please do not use any String functions. No sorting allowed. No additional arrays or data structures allowed.

  Test Cases:

  isHollow({1,2,4,0,0,0,3,4,5}) returns true
  isHollow({1,2,4,0,0,0,1,0,3,4,5}) returns false
  isHollow({1,2,0,0,0,3,4,5}) returns false
  isHollow({1,2,4,9, 0,0,0,3,4,5}) returns false
  isHollow({1,2, 0,0, 3,4}) returns false

private static int isHollow(int[] array) {
    int length = array.length;
    int startCount = 0;
    int endCount = 0;
    int zeroCount = 0;
    int nonZeroCount = 0;

    if (array[0] == 0 || array[length - 1] == 0) return 0;

    for (int i = 0; i < length; i++) {
        if (array[i] != 0) {
            startCount++;
        } else {
            break;
        }
    }
    for (int i = 0; i < length; i++) {
        if (array[i] == 0) {
            zeroCount++;
        } else {
            nonZeroCount++;
        }
    }

    for (int i = length - 1; i >= 0; i--) {
        if (array[i] != 0) {
            endCount++;
        } else {
            break;
        }
    }

    if (startCount == endCount && (startCount + endCount) == nonZeroCount && zeroCount >= 3) {
        return 1;
    }
    return 0;
}

You can determine the correct answer in a single pass, by working your way from the middle of the array outward, or from the ends of the array inward. Let's get started.

Although the problem description states to return 1 if the array is hollow  and 0 if it is not, such signature would make no sense in Java. It's only natural, and strongly recommended to use proper boolean true and false, respectively.

To work outward from the center, let's track left and right indexes.
If the array has odd number of elements,
left and right can both point to the middle element.
Otherwise the array has two central elements,
let's make left and right point at those.

int mid = nums.length / 2;
int left, right;
if (nums.length % 2 != 0) {
    left = right = mid;
} else {
    left = mid - 1;
    right = mid;
}

Now, let's start moving the left and right indexes outward, as long as both nums[left] and nums[right] are 0, and left > 0.

while (left > 0 && nums[left] == 0 && nums[right] == 0) {
    left--;
    right++;
}

The loop ends either when left reaches the second position of the array,
or when nums[left] or nums[right] (or both) are not zero.
If left and right were not moved enough, that is,
we haven't moved over enough zeros, then the array is not hollow and we can return.

if (right - left < 4) {
    return false;
}

Finally, we need to verify that the rest of the elements are all non-zero.
We can do that by continuing to move outward, and aborting if any of the values is 0.

while (left >= 0) {
    if (nums[left] == 0 || nums[right] == 0) {
        return false;
    }
    left--;
    right++;
}

When the loop completes without aborting, we know the array is hollow.

The complete solution, with some more test cases to verify it:

boolean isHollow(int... nums) {
    int mid = nums.length / 2;
    int left, right;
    if (nums.length % 2 == 0) {
        left = mid - 1;
        right = mid;
    } else {
        left = right = mid;
    }

    while (left > 0 && nums[left] == 0 && nums[right] == 0) {
        left--;
        right++;
    }

    if (right - left < 4) {
        return false;
    }

    while (left >= 0) {
        if (nums[left] == 0 || nums[right] == 0) {
            return false;
        }
        left--;
        right++;
    }

    return true;
}

@Test
public void verify_isHollow() {
    assertThat(isHollow(1, 2, 4, 0, 0, 0, 3, 4, 5)).isTrue();
    assertThat(isHollow(1, 2, 4, 0, 0, 0, 1, 0, 3, 4, 5)).isFalse();
    assertThat(isHollow(1, 2, 0, 0, 0, 3, 4, 5)).isFalse();
    assertThat(isHollow(1, 2, 4, 9, 0, 0, 0, 3, 4, 5)).isFalse();
    assertThat(isHollow(1, 2, 0, 0, 3, 4)).isFalse();
    assertThat(isHollow(1, 2, 0, 0, 0, 0, 3, 4)).isTrue();
    assertThat(isHollow(1, 0, 0, 0, 3)).isTrue();
    assertThat(isHollow(1, 2, 0, 0, 0, 0, 3, 4, 5)).isFalse();
    assertThat(isHollow(1, 2, 1, 0, 0, 0, 0, 3, 4)).isFalse();
    assertThat(isHollow(0, 0, 0)).isFalse();
    assertThat(isHollow(0, 0, 0, 0, 0)).isFalse();
    assertThat(isHollow(0, 0, 0, 0, 0, 1)).isFalse();
    assertThat(isHollow(1, 0, 0, 0, 0, 0)).isFalse();
    assertThat(isHollow(1, 0, 0, 0, 0, 0, 2)).isTrue();
}


Typescript & rxJS group list of object by month/year and map to certain value I have class:

export class DayValue {
  date: string;
  status: string;
}

and statusToPointsMap:

  public static map: Map<string, number> = new Map([
    ['COMPLETED', 100],
    ['FAILED', -100],
    ['SKIPPED', 0],
  ]);

I want to create function which calculates avg result per given period of time, ie:

[
  [
    "2020-05-02",
    "COMPLETED"
  ],
  [
    "2020-05-01",
    "FAILED"
  ],
  [
    "2020-04-30",
    "FAILED"
  ],
  [
    "2020-04-29",
    "SKIPPED"
  ],
  [
    "2020-03-13",
    "SKIPPED"
  ]
]

to (groupped by month)

[ ["05-2020",0],
  [ "04-2020",-40],
  ["03-2020",0]

]

or(groupped by year):

["2020",-16]

I created a function which works as I wanted, but I'm wonder if there is any way to to it smarter?

  private calculateAvgPerPeriod(values: DayValue[], period: string): any[] {
    const resultArray = [];
    from(values).pipe(
      groupBy(dayValue => this.datePipe.transform(dayValue.date, period)),
      mergeMap(group => group.pipe(toArray())),
      map(array => [
          this.datePipe.transform(array[0].date, period),
          array
            .map(day => this.statusToPointsMap.get(day.status))
            .reduce((prev, curr) => prev + curr, 0) / array.length
        ]
      ))
      .subscribe(entry => {
        resultArray.push(entry);
      });
    return resultArray;
  }

You're using RxJS, but your inputs and outputs are plain arrays. It'd be simpler to either consume and return streams or replace RxJS with Lodash/Ramda/Underscore/native array methods/some iteration-based library.
(If you want to get really fancy or need to use the function in different contexts, you could write a transducer.)
You're using a class, a method and a static property, but it would be sufficient to have a type, a function and a module-local constant. And passing datePipe (or just a Date) and status => getPointsFromStatusToPointsMap(status) function as arguments would make the dependencies more explicit. But these are more like code style preferences.
Lastly, you're returning any, but you should always return unknown instead unless you really have to lie to the type system for some reason. And returning [string, number] or [Date, number] would be more convenient for the consumer.


Convert a type u16 number to a matrix (Vec> or array) of 4 x 4 I am a Rust newbie and I am not familiar with all the iterator options. This is what I have so far. How can I make this better or at least avoid collecting twice into a Vec?

let num: u16 = 0b0010001000100010; // input number
let bin = format!("{:016b}", num);

let parsed = bin
    .split("")
    .filter_map(|s| s.parse().ok())
    .collect::<Vec<u8>>();

let mat = parsed.chunks(4).collect::<Vec<_>>();

println!("{:?}", mat);

// outputs [[0,0,1,0],[0,0,1,0],[0,0,1,0],[0,0,1,0]]

One thing I'm going to do first is create an enum with only two values. This represents a binary value and is more memory-efficient than passing around a bunch of u8s. This way 16 Bits can be represented as 16 actual bits in memory (though this isn't guaranteed).

/// a single bit
#[derive(Clone, Copy, Debug)]
enum Bit {
    /// 1, high
    H1 = 1,
    /// 0, low
    L0 = 0,
}
use Bit::*; // allow us to just use `L0` and `H1` without the `Bit::` prefix

It is likely much faster to split a number into bits using numerical operators. There are a couple ways of doing it.

Iterating a mask

With this we increase our mask each time, building an array from it. The 15 - i is there because we want the MSB at index 0.

/// convert a number to 16 bits by sliding a mask across it
fn into_bits_mask(num: u16) -> [Bit; 16] {
    let mut out = [L0; 16];

    for i in 0..16 {
        out[15 - i] = if num & (1u16 << i) > 0 {
            H1
        } else {
            L0
        };
    }

    out
}

Shifting the number with a static mask

This is essentially the same thing, but we shift the number instead of the mask.

/// convert a number to 16 bits by right-shifting it
fn into_bits_shift(num: u16) -> [Bit; 16] {
    let mut out = [L0; 16];

    for i in 0..16 {
        out[15 - i] = if (num >> i) & 1u16 > 0 {
            H1
        } else {
            L0
        };
    }

    out
}

We can then modify these to output an array of 4 nibbles.

/// convert a number to 4 nibbles by sliding a mask across it
fn into_nibbles_mask(num: u16) -> [[Bit; 4]; 4] {
    let mut out = [[L0; 4]; 4];

    for i in 0..16 {
        let mask = 1u16 << (15 - i);
        out[i / 4][i % 4] = if num & mask > 0 {
            H1
        } else {
            L0
        };
    }

    out
}

/// convert a number to 4 nibbles by right-shifting it
fn into_nibbles_shift(num: u16) -> [[Bit; 4]; 4] {
    let mut out = [[L0; 4]; 4];

    for i in 0..16 {
        out[i /4][i % 4] = if (num >> (15 - i)) & 1u16 > 0 {
            H1
        } else {
            L0
        };
    }

    out
}

There is room for more optimization here, of course.

Here's a working example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=b010e34728d554e995e0de4ddb4b1eed

EDIT: I was asked about using iterators, so here's my most iterator-function styled method

/// convert a number to 4 nibbles using iterator methods
fn into_nibbles_iter(num: u16) -> Vec<Vec<Bit>> {
    // split into nibbles
    [num >> 12, num >> 8, num >> 4, num]
        .iter()
        .map(|nibble| {
            // mask off each bit
            [nibble & 8, nibble & 4, nibble & 2, nibble & 1]
                .iter()
                // convert to Bits
                .map(|b| if b > &0 { H1 } else { L0 })
                .collect()
        })
        .collect()
}

And here's a new playground link demonstrating it: https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=5ecda81379c3cab749709f551109adfb