Suffix Tree in Python I was wondering why the following implementation of a Suffix Tree is 2000 times slower than using a similar data structure in C++ (I tested it using python3, pypy3 and pypy2). I know the bottleneck is the traverse method, do you spot chance of optimisation in the following code? Thanks!

from sys import stdin, stdout, stderr, setrecursionlimit
setrecursionlimit(100000)

def read():
    return stdin.readline().rstrip()

def readint():
    return int(read())

def make_node(_pos, _len):
    global s, n, sz, to, link, fpos, slen, pos, node
    fpos[sz] = _pos
    slen[sz] = _len
    sz += 1
    return sz-1

def go_edge():
    global s, n, sz, to, link, fpos, slen, pos, node
    while (pos > slen[to[node].get(s[n - pos], 0)]):
        node = to[node].get(s[n - pos], 0)
        pos -= slen[node]

def add_letter(c):
    global s, n, sz, to, link, fpos, slen, pos, node
    s[n] = c
    n += 1
    pos += 1
    last = 0
    while(pos > 0):
        go_edge()
        edge = s[n - pos]
        v = to[node].get(edge, 0)
        t = s[fpos[v] + pos - 1]
        if (v == 0):
            to[node][edge] = make_node(n - pos, inf)
            link[last] = node
            last = 0
        elif (t == c):
            link[last] = node
            return
        else:
            u = make_node(fpos[v], pos - 1)
            to[u][c] = make_node(n - 1, inf)
            to[u][t] = v
            fpos[v] += pos - 1
            slen[v] -= pos - 1
            to[node][edge] = u
            link[last] = u
            last = u
        if(node == 0):
            pos -= 1
        else:
            node = link[node]

def init_tree(st):
    global slen, ans, inf, maxn, s, to, fpos, slen, link, node, pos, sz, n

    inf = int(1e9)
    maxn = len(st)*2+1 #int(1e6+1)
    s = [0]*maxn
    to = [{} for i in range(maxn)]
    fpos, slen, link = [0]*maxn, [0]*maxn, [0]*maxn

    node, pos = 0, 0
    sz = 1
    n = 0

    slen[0] = inf
    ans = 0
    for c in st:
        add_letter(ord(c))

text = "TTTTTTTTTT" + 1777*"A" + "$"
query = 1750*"A"

def traverse_edge(st, idx, start, end):
    global len_text, len_st
    k = start
    while k <= end and k < len_text and idx < len_st:
        if text[k] != st[idx]:
            return -1
        k += 1
        idx += 1
    if idx == len_st:
        return idx
    return 0

def edgelen(v, init, e):
    if(v == 0):
        return 0
    return e-init+1

def dfs(node, leafs, off):
    if not node:
        return
    k, tree = node
    if not isinstance(tree, int): # it is a node pointing to other nodes
        for kk, value in tree.items():
            if slen[value] > 190000000:
                leafs.append(fpos[value]-off)
            else:
                dfs((kk, to[value]), leafs, off+slen[value])

def traverse(v, st, idx, depth=0):
    global len_st
    result = cache.get((v, st), -2)
    if result != -2:
        return result
    r = -1
    init = fpos[v]
    end = fpos[v]+slen[v]
    e = end-1
    if v != 0:
        r = traverse_edge(st, idx, init, e)
        if r != 0:
            if r == -1:
                cache[(v, st)] = []
                return []
            depth += r
            # Here is when we found a match
            leafs = []
            dfs((v, to[v]), leafs, depth)
            #return reversed(leafs)
            return leafs
    idx = idx + edgelen(v, init, e)
    if idx > len_st:
        cache[(v, st)] = []
        return []
    k = ord(st[idx])
    children = to[v]
    if k in children:
        vv = children.get(k, 0)
        matches = traverse(vv, st, idx, depth)
        cache[(v, st)] = matches
        return matches
    return []

def main():
    global text, len_st, len_text, cache
    init_tree(text)
    len_text = len(text)
    len_st = len(query)
    r = []
    #cache = {}
    for runs in range(1000):
        cache = {}
        matches = traverse(0, query, 0)
        r.append("\n".join(str(m) for m in matches))
    stdout.write("\n".join(r)+"\n")

main()

So, about all those globals. They make your code really hard to read, I have almost no idea what is going on (after having read it twice). The general recommendation is to only use global objects if you really have to. Otherwise pass the objects either as arguments to the function, or write a class which has the relevant variables as attributes and keep the state like this.

Another thing that makes your code a lot easier to understand (both for other people and for yourself in two months time), are docstrings, which document what each function does, the parameters it takes and what it returns or modifies.

I finally gave up trying to optimize Pypy and wrote the code I needed just in C++. Here is it in case somebody is curious:

#include <bits/stdc++.h>

using namespace std;

#define fpos adla
const int inf = 1e9;
const int maxn = 1e6+1;
//#define maxn 100005

char s[maxn];
map<int,int> to[maxn];
int slen[maxn], fpos[maxn], link[maxn];
int node, pos;
int sz = 1, n = 0;

int text_len;
char text[maxn];
int query_len;
char query[maxn];

int make_node(int _pos, int _len) {
    fpos[sz] = _pos;
    slen[sz] = _len;
    return sz++;
}

void go_edge() {
    while(pos > slen[to[node][s[n - pos]]]) {
        node = to[node][s[n - pos]];
        pos -= slen[node];
    }
}

void add_letter(int c) {
    s[n++] = c;
    pos++;
    int last = 0;
    while(pos > 0) {
        go_edge();
        int edge = s[n - pos];
        //int &v = to[node][edge];
        int v = to[node][edge];
        int t = s[fpos[v] + pos - 1];
        if(v == 0) {
            //v = make_node(n - pos, inf);
            to[node][edge] = make_node(n - pos, inf);
            link[last] = node;
            last = 0;
        }
        else if(t == c) {
            link[last] = node;
            return;
        }
        else {
            int u = make_node(fpos[v], pos - 1);
            to[u][c] = make_node(n - 1, inf);
            to[u][t] = v;
            fpos[v] += pos - 1;
            slen [v] -= pos - 1;
            // v = u;
            to[node][edge] = u;
            link[last] = u;
            last = u;
        }
        if(node == 0)
            pos--;
        else
            node = link[node];
    }
}

int traverse_edge(char st[], int st_len, int idx, int start, int end) {
    int k = start;
    while (k <= end && k < text_len && idx < st_len) {
        if (text[k] != st[idx]) {
            return -1;
        }
        k += 1;
        idx += 1;
    }
    if (idx == st_len) {
        return idx;
    }
    return 0;
}

int edgelen(int v, int init, int e) {
    if (v == 0) {
        return 0;
    }
    return e-init+1;
}

void dfs(map<int, int> tree, vector<vector<int>> leafs, int off) {
    if (tree.empty()) {
        return;
    }
        /* for ( auto [k, value] : tree) { */ // C++17
        for ( auto it : tree) {
            int value = it.second;
            if (slen[value] > maxn / 10) {
                vector<int> leaf;
                leaf.push_back(fpos[value]);
                leaf.push_back(slen[value]);
                leaf.push_back(fpos[value]-off);
                leafs.push_back(leaf);
            } else {
                dfs(to[value], leafs, off+slen[value]);
            }
        }
}

pair<bool, vector<int>> traverse(int v, map<int,int> tree, char st[], int idx, int depth) {
    int r = -1;
    int init = fpos[v];
    int end = fpos[v]+slen[v];
    int e = end - 1;
    if (v != 0) {
        r = traverse_edge(st, query_len, idx, init, e);
        if (r != 0) {
            if (r == -1) {
                return {false, vector<int>{}};
            }
            vector<int> matches;
            return {true, matches};
        }
    }
    idx = idx + edgelen(v, init, e);
    if (idx > query_len) {
        return {false, vector<int>{}};
    }
    int k = int(st[idx]);
    map<int,int> children = to[v];
    /* if (children.contains(k)) { */ // C++20
    if (children.find(k) != children.end()) {
        int vv = tree[k];
        return traverse(vv, to[vv], st, idx, depth);
    }
    return pair<bool, vector<int>>(false, vector<int>{});
}

int main() {
    int t;
    scanf("%d", &t);
        while (t--) {
            scanf("%s", text);
            text_len = strlen(text);

            for (int i=0; i<maxn; i++) {
                to[i] = map<int,int>{};
            }
            sz = 1;
            n = 0;
            pos = 0;
            node = 0;
            slen[0] = inf;

            for (int i=0; i<text_len; i++) {
                add_letter((int)text[i]);
            }
            add_letter((int)'$');

            int q;
            scanf("%d", &q);
                while (q--) {
                    scanf("%s", query);
                    query_len = strlen(query);
                    pair<bool, vector<int>> results = traverse(0, to[0], query, 0, 0);
                    printf("%s\n", results.first ? "y" : "n");
                }
        }
}
```


Finding the n th term in Fibonacci series Solving a problem in coding contest I came across a problem, which on digging a bit lead me to the data set in this question . Since the program to enumerate and finding the solution was too complex and execution time was below par I jot down the logic to the equation of curve (a single line)

Upon further digging deep I found that I missed the series which was forming a Fibonacci Series, hence by Binet's fourmla I found the nth term of the series which was even efficient. Here is the code

import math
import sys

def powLF(n):
    if n == 1:     return (1, 1)
    L, F = powLF(n//2)
    L, F = (L**2 + 5*F**2) >> 1, L*F
    if n & 1:
        return ((L + 5*F)>>1, (L + F) >>1)
    else:
        return (L, F)

def fib(n):
    if n & 1:
        return powLF(n)[1]
    else:
        L, F = powLF(n // 2)
        return L * F

def sum_digits(n):
   r = 0
   while n:
       r, n = r + n % 10, n // 10
   return r

def _print(string):
    fo = open("output.txt", "w+")
    fo.write(string)
    fo.close()

try:
    f = open('input.txt')
except IOError:
    _print("error")
    sys.exit()
num = f.readline()
try:
   val = int(num)
except ValueError:
    _print("error")
    sys.exit()

sum = sum_digits(int(num))
f.close()

if (sum == 2):
    _print("1")
else:
    _print(str(int(math.ceil(fib(sum)))))

Although still the code doesn't seem to match the par criteria, how can I optimize the code further ?

The code looks fine to me apart from the fact that it can throw an exception. The function fib() powLF() are giving you the complexity of O(log(n)) plus the execution time of the function sum_digits() is 479 ns per loop which is perfectly fine.

> %timeit sum_digits(n)
1000000 loops, best of 3: 479 ns per loop

Take a look at Lecture 3 of the MIT Open Courseware course on algorithms for a good analysis of the matrix approach.

>>> timeit.timeit('fib(1000)', 'from __main__ import fibM as fib', number=10000)
0.40711593627929688
>>> timeit.timeit('fib(1000)', 'from __main__ import fibL as fib', number=10000)
0.20211100578308105

This is not fibonacci series.  It's also recursively calculating the result which is almost certainly a bad idea if performance is at all important.  For calculating fibonacci, it would be better to use some sort of list, for example:

def fib(n):
    f = [0,1,1]
    for i in xrange(3,n):
        f.append(f[i-1] + f[i-2])
    return 'The %.0fth fibonacci number is: %.0f' % (n,f[-1])

If continued calls were to be made, then you should consider saving f and calculating only what is required to arrive at n.


Renaming all files/folders in first children folders but not sub-children Let's say I have this folder tree:

├──FolderParent
  ├── FolderA
      ├── FolderAA_IMG             -- should be renamed
          ├── file_IMG_AA1.txt     -- should NOT be renamed
          ├── file_VID_AA1.txt     -- should NOT be renamed
      ├── file_IMG_A1.txt          -- should be renamed
      ├── file_PANO_A1.txt         -- should be renamed
  ├── FolderB
      ├── file_IMG_B1.txt          -- should be renamed
      ├── file_PANO_B1.txt         -- should be renamed

As you can see, only the files/folders in the first children folder should be renamed, not the other ones. I follow this base code and then I added a second loop above but I wonder if the double for loop is the right way to go.

import os

# custom var
path= r"C:\Users\user\FolderParent"

# other cvar
number_modified_files= 0

# get all path of subfolder
all_subfolders = [f.path for f in os.scandir(path) if f.is_dir() ]
print(all_subfolders)

for folder in all_subfolders:
    # set the path to the folder: otherwise the rename file won't work
    os.chdir(folder)
    #won't rename files in subfolder but will rename folder in the path
    for filename in os.listdir(folder):
        print("new file:", filename)
        if "IMG_" in filename:
            os.rename(filename, filename.replace('IMG_', ''))
            number_modified_files +=1
        elif "PANO_" in filename:
            os.rename(filename, filename.replace('PANO_', ''))
            number_modified_files +=1
        elif "VID_" in filename:
            os.rename(filename, filename.replace('VID_', ''))
            number_modified_files +=1

print(f"End : {number_modified_file} files renamed")

You should rethink your solution in terms of regexes:

#!/usr/bin/env python3

import re
from os import scandir, rename, path

def rename_children(parent):
    n_renamed = 0
    re_fname = re.compile('(IMG|PANO|VID)_')

    for child_dir in scandir(parent):
        if child_dir.is_dir():
            for child in scandir(child_dir):
                renamed = re_fname.sub('', child.name)
                if renamed != child.name:
                    new_path = path.join(child_dir.path, renamed)
                    print(f'Renaming {child.path} to {new_path}')
                    rename(child.path, new_path)
                    n_renamed += 1
    print(f'{n_renamed} files renamed')

Note the following changes:

Only one if to check whether the regex matches
Use scandir instead of listdir
Do not call chdir; there's no point
Don't call replace; the pattern check and the replacement operation can be combined by using sub
Don't store a list of all_subfolders; simply iterate over the results

You should use a recursive function like this:

import os

def rename_files(basedir):
    for f in os.listdir(basedir):
        if os.path.isdir(f):
            rename_files(os.path.join(basedir, f))
        else:
            if "IMG_" in f:
                os.rename(f, f.replace('IMG_', ''))
            else:
                pass

rename_files('C:/Users/user/FolderParent')


Extracting tuples from Excel sheet My code have too much nested option processing which makes it ugly. But it has an advantage it has no side effect and typesafe.

package repos

import java.io.{InputStream, File}
import org.apache.poi.hssf.usermodel.HSSFWorkbook
import org.apache.poi.ss.usermodel.{Cell, Row, Workbook, Sheet}
import org.apache.poi.xssf.usermodel._
import scala.collection.JavaConversions._

object ExcelRepo{

  def read(binary: InputStream, filename: String) = {
    val excelExtension = "([^\\s]+(\\.(?i)(xls|xlsx))$)"
    val excelData = if(filename.matches(excelExtension)) {

      val xlsExtension = "([^\\s]+(\\.(?i)(xls))$)"
      val xlsxExtension = "([^\\s]+(\\.(?i)(xlsx))$)"

      val workbook:Option[Workbook] = if (filename.matches(xlsExtension)) {
        println("XLS")
        Some(new HSSFWorkbook(binary))
      } else {
        println("XLSX")
        Some(new XSSFWorkbook(binary))
      }

      val data = workbook match {
        case Some(wb: Workbook) =>
          val sheet = Option(wb.getSheetAt(0))
          sheet match {
            case Some(s: Sheet) =>
              val titleRow = Option(s.getRow(0))
              titleRow match {
                case Some(tr:Row) =>
                  val titleCell = Option(tr.getCell(0))
                  val priceCell = Option(tr.getCell(1))
                  (titleCell, priceCell) match {
                    case (Some(tc: Cell), Some(pc:Cell)) =>
                      if (pc.getStringCellValue == "price" && tc.getStringCellValue == "title") {
                        val list:List[(String, Double)] = sheet.get.tail.map{
                         r => (r.getCell(0).getStringCellValue, r.getCell(1).getNumericCellValue)}.toList
                        list
                      } else {
                        List()
                      }
                    case (_,_) => List()
                  }
                case None => List()
              }
            case None => List()
          }
        case None => List()
      }
      data
    } else {
      List()
    }
    excelData
  }

}

This function reads Excel file assuming that it is not null, and returning the List of tuples with (price, title), or empty list if workbook or row or cell is empty.

Is it actually ugly? Should I refactor it? If I should what is the clear and concise way to deal with match Option → getting new Option → match it again → getting new Option ...

You are doing the same thing in case of failure at any stage - returning an empty list.  You should use a for comprehension to chain these stages; if any one stage returns None, then the other stages will not be followed.  If you do something like this:

  for {
    wb <- workbook;
    sheet <- Option(wb.getSheetAt(0));
    titleRow <- Option(sheet.getRow(0));
    titleCell <- Option(titleRow.getCell(0)) if titleCell.getStringCellValue == "title";
    priceCell <- Option(titleRow.getCell(1)) if priceCell.getStringCellValue == "price";
  } yield { sheet.get.tail.map{r => (r.getCell(0).getStringCellValue, r.getCell(1).getNumericCellValue)}.toList
  }

This will return Option[List[(String, Double)]], containing either a real list or None.  If any of those stages returns None, the following stages will not be executed and you just get your None back early.  You can pattern match against this output to return the contents of Some(list) or List() if the output is None.

Note: I took the annotated val list out of there, so you would either need to have specified the return type for a val to which you assign the output of the for comprehension or put it back into the yield block.

Note how I turned your if...else expression into guards.  Can you see just how much simpler the code is and how each conditional is paired with the appropriate object? Particularly since the else was just returning another empty list.  This way, it fails and returns None.

"for comprehensions" can be overused in Scala, but this really is where they shine.  You correctly chose Option, but possibly the best thing about Option for any Java coder is the way it can remove boilerplate and defensive coding.  Your code still has the defensive coding, while the for comprehension just throws all that away. Learn to be free ;)

package cs220.ds

import java.io.{InputStream, File, FileInputStream}
import org.apache.poi.hssf.usermodel.HSSFWorkbook
import org.apache.poi.ss.usermodel.{Cell, Row, Workbook, Sheet}
import org.apache.poi.xssf.usermodel._
import scala.collection.JavaConversions._

object ExcelRepo{

  def read(binary: InputStream, filename: String)= {
    val excelExtension = "([^\\s]+(\\.(?i)(xls|xlsx))$)"
    val excelData = if(filename.matches(excelExtension)) {

      val xlsExtension = "([^\\s]+(\\.(?i)(xls))$)"
      val xlsxExtension = "([^\\s]+(\\.(?i)(xlsx))$)"

      val workbook:Option[Workbook] = if (filename.matches(xlsExtension)) {
        println("XLS")
        Some(new HSSFWorkbook(binary))
      } else {
        println("XLSX")
        Some(new XSSFWorkbook(binary))
      }

      val data  = {
        for {
          wb <- workbook
          sheet <- Option( wb.getSheetAt(0) )
        } yield { sheet.tail.map( r => (r.getCell(0), r.getCell(1),r.getCell(2)) ).toList}
      }

        data
      }

print("output:" + excelData)
     excelData
  }

}

This is a working implementation that I realized you can not use the get function on sheet due to tail only applies to Option, and .get will actually pull what's in the Option. which will throw an error, What this code is the combination of the OP's code with the provided solution that has been fixed for minor errors that would make it not work correctly. I removed many unnecessary checks for my use to just pull all but the header row in excel. I used the println in the bottom of the function to make sure I was getting the data I wanted. I also added data at the end of the read FN to make sure that the correct variable is returned from the excelData value. This is all I have done to make this project work.


Custom networked remote manager I've spent my fair share of time criticizing other people's code on this site; but I'm finding out I'm kind of a hypocrite. Almost everything I've written so far has been some program whipped together to do some task for me. I have little experience actually starting out writing maintainable code.

Then today I decided to start a semi-large project. I want to write a sort of custom networked remote manager; a kind of cross-breed between ssh and an all-out remote desktop.

I wrote up the following class while realizing the previous revelations:

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.Socket;
import java.net.UnknownHostException;
import java.util.ArrayList;

public abstract class Connection implements Runnable {
    private final Socket s;
    private final InputStream is;
    private final OutputStream os;
    private final ArrayList<byte[]> posts;
    private boolean run;
    public Connection(String addr, int port) throws UnknownHostException, IOException {
        s = new Socket(addr, port);
        is = s.getInputStream();
        os = s.getOutputStream();
        posts = new ArrayList<>();
        run = true;
        new Thread(this).start();
    }
    public void post(byte data[]) {
        synchronized (posts) {
            posts.add(data);
        }
    }
    public void stop() {
        run = false;
    }
    public void run() {
        while (run) {
            byte send[] = null;
            synchronized (posts) {
                if (posts.size() > 0) {
                    send = posts.get(0);
                    posts.remove(0);
                }
            }
            if (send != null) {
                try {
                    os.write(send);
                } catch (IOException e) {
                    hasError(e);
                }
            }
            try {
                if (is.available() > 0) {
                    byte data[] = new byte[is.available()];
                    receive(data, is.read(data));
                }
            } catch (IOException e) {
                hasError(e);
            }
        }
    }
    public abstract void receive(byte data[], int len);
    public abstract void hasError(Exception e);
}

It occurred to me I should probably be asking here how maintainable code is actually written. I'm kind of ashamed I haven't done this sort of thing before, but as a self-taught programmer I presume this sort of obstacle is common.

Given the above sample of my idea of how to make maintainable modular code, what sort of pitfalls should I look out for while making the excursion into a legitimate program of larger than two or three files?

new Thread(this).start();

I may be wrong about this, but I don't think this is recommended in the constructor.

ExecutorService implementations exist to provider better/consistent handling of asynchronous tasks. Arguably, a better form will be to extend your abstract class and use it with an ExecutorService instance, instead of having it (Connection) decide when to start running.

Inside your run() method, you may want to consider the following rewrite as it is slightly more expressive and reduces one level of nesting:

while (run) {
    byte send[] = null;
    synchronized (posts) {
        if (posts.isEmpty()) {
            continue;
        }
        send = posts.get(0);
        posts.remove(0);
    }
    // send != null probably not required, as long as post() does not accept null
    // if (send != null) {
    try {
        os.write(send);
    } catch (IOException e) {
        hasError(e);
    }
    // }
    // ...
}

Considering you are doing something like:

posts.get(0);
posts.remove(0);

You might actually be looking for a Queue implementation to simplify this approach. In fact, you may be looking for a thread-safe implementation so that you do not need the explicit synchronized blocks, such as something that implements a BlockingQueue.

  Given the above sample of my idea of how to make maintainable modular code, what sort of pitfalls should I look out for while making the excursion into a legitimate program of larger than two or three files?

A thorough answer for this will be out-of-scope here on CR, so my only advice is to perhaps ask that on Programmers.SE (as long as you can make it on-topic there). :)

To me, maintainable code is code that I or someone else will be able to understand and modify in the future, even after I've forgotten the details, or even if I'm not around to explain to someone new trying to learn my source code from scratch.

In practice, this mainly means:

Documenting code. Well-named variables, functions and classes. Copious comments written to be understandable by someone who forgot or never knew what the code did, explaining basic things, thinking, reasons, etc, any place it might not be immediately obvious from the code itself. Nice consistent style.

Your code sample is short and well-formatted and fairly easy to follow, but there are no comments, and some of the variable names are very short (which can be ok if they're only used immediately for a line or two, but would be clearer with explicit names). Those things aren't so important at this scale, but if you're doing something tricky I might not know it, and the longer and more elaborate the project gets, the more documentation can help someone new or returning. Your imports are all self-explainitory by name, but if I'm including something unusual I may say why, e.g:

// Misc imports:
import java.util.concurrent.TimeUnit; // Needed for SpecialSync() function.
import java.wtf.BizarroLib;   // Needed for JimJamFunc();

Depending on the style you choose, the class definition might be commented more like:

  // Definition for class Connection
  //
  // Encapsulates a custom networked remote manager.
  // Throws UnknownHostException if (explain when it can throw).
  // (Maybe explain how to use and clean up, if not trivial.)
  // (Insert class-level notes about when to use it, and any dependencies
  //  or things you might forget in a year, or not know as a new person
  //  seeing this for the first time and wondering what this is.)
  public abstract class Connection implements Runnable {
    private final Socket sock;
    private final InputStream inSock;
    private final OutputStream outSock;
    private final ArrayList<byte[]> posts;
    private boolean shouldRun;
    public Connection(String addr, int port) throws UnknownHostException, IOException { ...

Some IDE's have a format for functions which will cause the comments to appear when the programmer hovers over its name elsewhere in code. Styles vary, but I usually at least like blank lines between function definitions, e.g.:

// Stop processing posts.
public void stop() {
    run = false;
}

// Main processing loop.
// On IO exceptions, (mention what calling hasError does).
public void run() {
…
}

// Defined elsewhere:
// (Maybe say where to find or put a definition for these.)
public abstract void receive(byte data[], int len);
public abstract void hasError(Exception e);

Code written in a way that makes sense and is organized, hopefully with few weird quirks and dependencies, and is written with the idea that its requirements may change over time, so it should be made easy to change things. Break code into a meaningful organization of files with organized file names in organized folders. Encapsulate functionality. Make utility functions. For a larger project, usually I build (or re-use) some sort of framework that helps make coding the meat of the logic easier, usually by abstracting out implementation details into nice well-named functions that say what they do and do what I want to do at the level I want to think about it when I'm doing the project's logic only a level or two down from the requirements level.

Your code sample is short and since you're using Java, you will have one class per file. This one already nicely encapsulates one task. If a class starts to have a lot of different complex functionality, you may want to consider whether it makes sense to divide the class up so that there is at most one dense/complex function per class. I'm more familiar with C, C++ and C#, where a class can be split between files. For Java, it makes sense to organize classes into folders, depending on the project's organization, so I'd expect this class file to be in a place such as org.DomainName.projectName.util. That will start to matter when you have dozens of small classes like this, so you can find this one quickly when you need it (or when you're new and don't what's in the project), but have it out of sight and mind when you don't.
Complete notes about how to set up the build and production environments, so that if someone just had the files and had to get them running on a blank computer starting with installing the OS and tools, they could do that without being stumped for days by obscure system settings and installation steps.

So if this sample were part of a larger project, I'd expect these notes to be at the top level of the project file tree, and/or in the project text documentation. If this is a small module meant to be maintainable, you might put it in a comment block at the top of the file, mentioning the build environment and version you developed it with, etc.
Some design docs that explain how the project files are organized, what the requirements are, and how the operators and end users are supposed to use it, as well as list of current known issues and future plans.

Again, this would be in top-level documentation.


Insertion sort in Common Lisp This is my first bit of significant code in Common Lisp, an implementation of insertion sort. Being new to Lisp in general, I'm wondering if this code follows best Lisp practices in regards to program design.

;;; Return a portion of a list
(defun slice (elements from-index to-index)
  (cond
    ((= from-index to-index) nil)
    (t (cons (nth from-index elements) (slice elements (+ from-index 1) to-index)))))

;;; Return a list omitting a portion
(defun splice (elements from-index to-index)
  (append (slice elements 0 from-index) (slice elements to-index (length elements))))

;;; Move element in list to new index, return new list
(defun move (elements from-index to-index)
  (let ((spliced (splice elements from-index (+ from-index 1))))
    (append (slice spliced 0 to-index) (list (nth from-index elements)) (slice spliced to-index (length spliced)))))

;;; Find the determined (via predicate) correct index for an element in a list
(defun find-ordered-index (elements predicate index &optional (key index))
  (cond
    ((or (= index 0) (= key 0)) 0)
    ((funcall predicate (nth (- key 1) elements) (nth index elements)) (find-ordered-index elements predicate index (- key 1)))
    (t key)))

;;; Return sorted list
(defun insertion-sort (elements predicate &optional (index 0))
  (cond
    ((= index (length elements)) elements)
    (t (insertion-sort (move elements index (find-ordered-index elements predicate index)) predicate (+ index 1)))))

Trivial

Use 1- instead of (- ... 1).

Avoid very long lines (Emacs will indent for you).

Do not use cond when a single if without progn would do.

Memory

Use nconc instead of
append when possible to avoid
unnecessary consing (in your case, splice allocates a fresh list, so
its result can be passed to nconc).

Catastrophic

Whenever you use nth, you are
using the wrong algorithm.

Optimal search is linearithmic: O(n*log(n)).

Insert search is quadratic: O(n^2).

Your implementation is O(n^3):

nth is O(n)
slice is O(n^2) (calls nth * recursion).
splice and move are also O(n^2) (call slice).
find-ordered-index is O(n).
insertion-sort is O(n^3) (calls move * recursion).

The insertion sort problem is an interesting topic for Lispers because of singly-linked lists.

additional descriptions

(defun insertion-sort (list)
  (let ((tmp))
    (do ((i-node list (rest i-node))) ((null i-node) list)
      (do ((j-node (rest i-node) (rest j-node))) ((null j-node))
        (when (> (first i-node) (first j-node))
          (setq tmp (first j-node))
          (rplaca j-node (first i-node))
          (rplaca i-node tmp))))))


Numeric double integration I've made a simple program for numerically aproximating double integral, which accepts that the bounds of the inner integral are functions:

import numpy as np
import time

def double_integral(func, limits, res=1000):
        t = time.clock()
        t1 = time.clock()
        t2 = time.clock()
        s = 0
        a, b = limits[0], limits[1]
        outer_values = np.linspace(a, b, res)
        c_is_func = callable(limits[2])
        d_is_func = callable(limits[3])
        for y in outer_values:
            if c_is_func:
                c = limits[2](y)
            else:
                c = limits[2]
            if d_is_func:
                d = limits[3](y)
            else:
                d = limits[3]
            dA = ((b - a) / res) * ((d - c) / res)
            inner_values = np.linspace(c, d, res)
            for x in inner_values:
                t2 = time.clock() - t2
                s += func(x, y) * dA
            t1 = time.clock() - t1
         t = time.clock() - t
         return s, t, t1 / res, t2 / res**2

This is, however, terribly slow. When res=1000, such that the integral is a sum of a million parts, it takes about 5 seconds to run, but the answer is only correct to about the 3rd decimal in my experience. Is there any way to speed this up?

The code i am running to check the integral is

def f(x, y):
    if (4 - y**2 - x**2) < 0:
        return 0              #This is to avoid taking the root of negarive #'s
    return np.sqrt(4 - y**2 - x**2)

def c(y):
    return np.sqrt(2 * y - y**2)

def d(y):
    return np.sqrt(4 - y**2)
# b d
# S S f(x,y) dx dy
# a c
a, b, = 0, 2
print(double_integral(f, [a, b, c, d]))

The integral is eaqual to 16/9

If you want to use numpy, use numpy properly. Inestead of

for x in inner_values:
    s += func(x, y) * dA

use the more idiomatic, and much faster

s += dA * np.sum(func(inner_values, y))

Note: this requires rewriting f as

return np.sqrt(np.maximum(0, 4 - y**2 - x**2))

so it can accept an array as an input. This does not reduce accuracy, but brings time down to a much more accaptable .04 seconds for a 100x improvement. The takeaway here is Numpy is not magic. It provides quick vectorization.

For a function strickly growing.
Between a and b
Recursively Search at x interssection point between the line a and b and compare the delta y if too much you you add the point c and check the intersection.
After that you have to sum the trapezoid sirfaces


Calculating GCD for 2 integers I have implemented a method which takes 2 numbers and returns the Greatest Common Divisor, using the Euclidean algorithm.
The Euclidean algorithm goes like this:
If we take the numbers 585 and 442:

585 / 442 = 1 (remainder 143)
442 / 143 = 3 (remainder 13)
143 / 13 = 11 (remainder 0)

The process stops here and GCD = 13.
Is there any way I can make this implementation better, considering time, resources and refactoring?
using System;

class Program
{
    static void Main()
    {
        Console.WriteLine("Input first number: ");
        int x = Convert.ToInt32(Console.ReadLine());
        Console.WriteLine("Input second number: ");
        int y = Convert.ToInt32(Console.ReadLine());

        Console.WriteLine(GetGCD(x, y));

        Console.ReadKey();
    }

    static int GetGCD(int x, int y)
    {
        while (Math.Max(x, y) % Math.Min(x, y) != 0)
        {
            int tmp = Math.Max(x, y) % Math.Min(x, y);
            if (x < y) y = tmp;
            else x = tmp;
        }

        return Math.Min(x, y);
    }
}

Recursive Method

You can change your GetGCD() method to be much more intuitive and minimal using a ternary operator and recursion. I recommend also documenting what it's doing for readability purposes:

static int GetGCD(int x, int y)
{
    // If y is equal to 0, return x.
    // If y is not equal to 0, recursive call with x as y and y as the remainder.
    return y == 0 ? x : GetGCD(y, x % y);
}

Can also be written as:

static int GetGCD(int x, int y)
{
    // If y is equal to 0, return x.
    if (y == 0)
        return x;
    // If y is not equal to 0, recursive call with x as y and y as the remainder.
    return GetGCD(y, x % y);
}

Depending on your preference.

Iterative Method

As stated in other answers, you might use an iterative method:

static int GetGCD(int x, int y)
{
    while (y != 0)
    {
        int tmp = x % y;
        x = y;
        y = tmp;
    }
    return x;
}

Entry Point

Use TryParse() for retrieving integer input from the console. We don't want exceptions on user input.

Modular arithmetic should be positive. To ensure this, we use the absolute value of the input (Euclidean division). Here's how I would implement it:

static void Main()
{
    int x, y;

    // Get x. Ensure it's an integer.
    Console.WriteLine("Input first number: ");
    while(!Int32.TryParse(Console.ReadLine(), out x));

    // Get y. Ensure it's an integer.
    Console.WriteLine("Input second number: ");
    while(!Int32.TryParse(Console.ReadLine(), out y));

    // Ensure the values being used are positive.
    Console.WriteLine(GetGCD(Math.Abs(x), Math.Abs(y)));

    Console.ReadKey();
}

Output

As you can see, it will wait for an input that can be parsed to an integer and returns the expected value:

Input first number:
a
b
c
585
Input second number:
d
442
13

Benchmark

Just for fun, I thought I'd test the difference between the recursive and iterative methods.

Benchmarked method: Recursive
Test cases: 20
Bench in MS: 184
Iterations per case: 10000000

Benchmarked method: Iterative
Test cases: 20
Bench in MS: 121
Iterations per case: 10000000

The difference is negligible in real application (unless you plan to run this millions of times). These tests were run from entirely different processes.

By using this, you can pass multiple values as well in the form of array:-

// pass all the values in array and call findGCD function
    int findGCD(int arr[], int n)
    {
        int gcd = arr[0];
        for (int i = 1; i < n; i++) {
            gcd = getGcd(arr[i], gcd);
}

        return gcd;
    }

// check for gcd
int getGcd(int x, int y)
    {
        if (x == 0)
            return y;
        return gcd(y % x, x);
    }


Checks if all items are negative This piece of code checks to see if all the values within an array are all negative and returns true if they all are.

public static boolean allNegatives(int[] a) {

  if(a == null) {

      return false;

   }

    for(int i = 0; i < a.length; i++) {
        if(a[i] > 0)
            return false;
    } return true;
}

With java 8 you can reduce your code using a more functional programming. It would be something like next. You could even reduce it to one line:

public static boolean allNegatives(int[] a) {
     //@bowmore suggestion edit
     return a != null && Arrays.stream(a).allMatch(i -> i < 0);
}

There are 3 exit-points for your method:

if the argument is null (can't judge the sign of elements)
if the argument is empty (can't judge the sign of elements)
if the argument contains at least one element (can easily multiply to get accumulated sign)

Arithmetical (binary) approach
Here is the implementation I would suggest from a different perspective: arithmetical using only the sign, exactly the sign-bit:
static boolean containsOnlyNegatives(int[] integers) {
  // so called guard-statement first as fail-fast
  if (integers == null || integers.length == 0) {
    // though strictly non-negative; consider throwing IllegalArgumentException
    return false;
  }

  // bitwise AND (&) operation results true only if negative sign given
  // put -1 as reference for comparison (negative sign)
  var signedResult = IntStream.of(integers).reduce(-1, (a, b) -> a & b);

  return Integer.signum(signedResult) == -1;
}

Notice how I changed the method-name to sound like a predicate or matcher (idiomatic naming-convention like is/has/contains; compare Hamcrest matchers).
Here is a demo on IDEone which uses a simple for-loop.
Why arithmetical instead of iteration using predicates ?
Bitwise arithmetics are usually the fastest operations for a CPU (hopefully the Java compiler can optimize as such)
When predicate gains performance ?
Assume your list of 1001 integers has the first element as negative and as early as the second element with a positive. Then you could stop after the second element and return false. This is concept is also called short-circuit or fail-fast similar to mathematical/logical dis-prove of an assumption.
Note on Java 8 stream-predicates:

allMatch tests all integers and thus may take longer
anyMatch can stop as soon as first positive detected to prove: not all integers are negative!

So you could improve given functional answer using this  matcher and predicate:
static boolean containsOnlyNegatives(int[] integers) {
  // similar to answer already given, but short-circuiting and bitwise
  return integers != null
         && !Arrays.stream(integers).anyMatch(i -> Integer.signum(-1 & i) != -1);
}

Reads hard and causes mental load, because all that diluting negations made it somewhat complicated.
Simplify towards a positive
In your case the method-name has a negative connotation already: allNegatives.
When verifying that all are negatives, you also verify that no positive exists.
Try to reverse it - it's easier to read and think on a positive expression (of intent):
static boolean containsPositive(int[] integers) {
  // now short-circuiting if the first element was tested positive
  return integers != null
         && Arrays.stream(integers).anyMatch(i -> Integer.signum(-1 & i) == 1);
}

// kept for convenience, if you need that contract
static boolean containsOnlyNegatives(int[] integers) {
  return !containsPositive(integers);
}

Try the demo on IDEone
See also

Bitwise Operators and Negative Numbers
java - How can I check whether an array is null / empty? - Stack Overflow
Java Stream anyMatch() Tutorial With Examples | kheri.net


HackerRank Grid Search challenge I’ve written an answer for The Grid Search challenge on HackerRank.
Please let me know of any improvements that can be made to my implementation (specifically if there's any way to make the algorithm more efficient. I'd like to make this algorithm optimal).

P.S: My implementation is a brute force algorithm, and I reckon that there's a much better way of solving this problem. But, as the grid is unsorted, I'm not exactly sure what that better method would be.

Problem statement (as written on HackerRank):

  Given a 2D array of digits, try to find the location of a given 2D pattern of digits.

Answer:

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class Solution {
    public static void main(String[] args) throws IOException {
        Solution solution = new Solution();
        try(BufferedReader input = new BufferedReader(new InputStreamReader(System.in))){
            int numTestCases = Integer.parseInt(input.readLine());
            assert numTestCases >= 1 && numTestCases <= 5;
            int[][] grid;
            int[][] pattern;

            for(int t = 0; t < numTestCases; t++) {
                grid = solution.buildArray(input);
                pattern = solution.buildArray(input);
                // Gotta STDOUT in HackerRank for problem to be graded.
                System.out.println(solution.findPattern(grid, pattern));
            }
        }catch(IOException e){
            e.printStackTrace();
        }
    }

    /**
     * Determines whether pattern exists in grid.
     *
     * @param grid       2D array of digits
     * @param pattern    the 2D sequence of digits to be found in grid
     * @return "YES" if grid contains pattern. "NO" if not.
     */
    public String findPattern(int[][] grid, int[][] pattern){
        // R, C, r, & c are the same letters used in the problem
        for(int R = 0; R < grid.length - pattern.length + 1; R++){
            for(int C = 0; C < grid[0].length - pattern[0].length + 1; C++){
                outerLoop:
                for(int r = 0; r < pattern.length; r++){
                    for(int c = 0; c < pattern[0].length; c++){
                        if(grid[R + r][C + c] != pattern[r][c]){
                            break outerLoop;
                        }
                    }
                    if(r == pattern.length - 1){
                        return "YES";
                    }
                }
            }
        }
        return "NO";
    }

    /**
     * Builds and returns an array built according to data received from STDIN.
     *
     * @param input    STDIN stream
     * @return a 2D primitive array containing 0-9 digits
     * @throws IOException
     */
    public int[][] buildArray(BufferedReader input) throws IOException {
        String[] sizeParameters = input.readLine().split(" ");
        int rows = Integer.parseInt(sizeParameters[0]);
        int columns = Integer.parseInt(sizeParameters[1]);
        int[][] array = new int[rows][columns];

        for(int i = 0; i < rows; i++){
            String rowOfNumbers = input.readLine();
            for(int j = 0; j < columns; j++){
                array[i][j] = Character.getNumericValue(rowOfNumbers.charAt(j));
            }
        }

        return array;
    }

}

Code organization

This is usually a cause for concern:

Solution solution = new Solution();

Programming contests do not generally require much object design. And in this particular case, instance methods just obfuscate reading. Make them public static. And if it gets so big that you need to divvy it up, do it properly, then.

This is usually a cause for concern, too:

    }catch(IOException e){
        e.printStackTrace();
    }

Just throw the exception, if you cannot do anything with it. And some automated judges show you the STDERR but not STDOUT, so you may be wasting your chance to see whether something went wrong or you returned wrong answer, and what went wrong. (Just knowing if your program has thrown an exception is good. If the automated judge does not support exception but supports process exit codes, you do a System.exit if an exception thrown.)

Performance

Some contests test your code with just small and large data sets, which means you need to find and algorithm with good average complexity. Some contest, such as ACM, tests the code also with the known worst case scenarios of the best known algorithms that could be used to solve the problem. In that case you need to find good worst case complexity.

Some contests have inputs to weed out naive implementations as your current one. One such case would be 1000X1000 grid of 1s, and 500X500 pattern of 1s with a 2 in the bottom right corner. Against such a case you can try an algorithm like :

Scan the pattern, find the row with most diversity (e.g. with the most elements which have a different value to its right)
for each row in the grid do a KMP substring search.
for each match compare the rest of the pattern in the grid, using the row and column offset of the match.

Which, I believe, will have a complexity : O(R(C+c) + m(rc)), where m is the number of the matches for the pattern row used; which can still be bad for large m. In that case you can do a search for a pattern column in parallel (real or simulated).

Before starting with

for (int r = 0; r < pattern.length; r++) {

You can add one more check:

if (grid[R + r][C + c] == pattern[r][c]) {
    //only then start comparing
}

By doing this check, you can avoid unnecessary iterations of the inner for loops(the ones which compare the pattern)


Confirming existence of lapindromes The following description is taken from CodeChef.com's Lapindrome problem.

  Lapindrome is defined as a string which when split in the middle, gives two halves having the same characters and same frequency of each character. If there are odd number of characters in the string, we ignore the middle character and check for lapindrome. For example gaga is a lapindrome, since the two halves ga and ga have the same characters with same frequency. Also, abccab, rotor and xyzxy are a few examples of lapindromes. Note that abbaab is NOT a lapindrome. The two halves contain the same characters but their frequencies do not match.
  Your task is simple. Given a string, you need to tell if it is a lapindrome.

  Input:

  First line of input contains a single integer T, the number of test cases.

  Each test is a single line containing a string S composed of only lowercase English alphabet.

  Output:

  For each test case, output on a separate line: "YES" if the string is a lapindrome and "NO" if it is not.

  Constraints:

  \$1 ≤ T ≤ 100\$
  2 \$≤ |S| ≤ 1000\$, where |S| denotes the length of S.

  Example:

  Input:

6
gaga
abcde
rotor
xyzxy
abbaab
ababc

  Output:

YES
NO
YES
YES
NO
NO

#include<stdio.h>
int main()
{
    int t;
    scanf("%d",&t);
    while(t--)
    {
        char s[1005];
        scanf("%s",s);
         int len,b[strlen(s)],z;
         int i,j,count =0;
         len=strlen(s);
         for(z=0;z<len;z++)
         {
                b[z] =1;
         }
        if(len%2 !=0)
        {
            for(i=0;i<len/2;i++)
            {
                for(j=(len/2)+1;j<len;j++)
                {
                    if(s[i] == s[j])
                    {
                        if(b[i] && b[j] )
                        {
                            b[i]=0;
                            b[j]=0;
                            count+=1;
                        }
                    }
                }
            }
        }
         else if(len%2 ==0)
        {
            for(i=0;i<len/2;i++)
            {
                for(j=(len/2);j<len;j++)
                {
                    if(s[i] == s[j])
                    {
                        if(b[i] && b[j] )
                        {
                            b[i]=0;
                            b[j]=0;
                            count+=1;
                        }
                    }
                }
            }
        }
        if(count==len/2)
            {
                printf("YES\n");
            }
            else
            {
                printf("NO\n");
            }
    }
}

Firstly, there's no reason for us to dump all of this in main.

The logic for determining whether or not a string is a lapindrome can (and should) simply exist in a function that takes a string and returns a bool.

bool isLapindrome(const char *string);

This function has the single responsibility of determining whether or not the input string is a Lapindrome, and it returns a true/false indicating as much.

 if(len%2 !=0)
    {
        for(i=0;i<len/2;i++)
        {
            for(j=(len/2)+1;j<len;j++)
            {
                if(s[i] == s[j])
                {
                    if(b[i] && b[j] )
                    {
                        b[i]=0;
                        b[j]=0;
                        count+=1;
                    }
                }
            }
        }
    }
     else if(len%2 ==0)
    {
        for(i=0;i<len/2;i++)
        {
            for(j=(len/2);j<len;j++)
            {
                if(s[i] == s[j])
                {
                    if(b[i] && b[j] )
                    {
                        b[i]=0;
                        b[j]=0;
                        count+=1;
                    }
                }
            }
        }
    }

Correct me if I'm wrong, but by my eyes, the parts inside each half of these if and else are identical except the starting position of the inner loop.

That looks to be the case, so why don't we refactor, starting from the inside and working out.

Rather than nested if statements, we can simply have a single conditional.

if (s[i] == s[j] && b[i] && b[j]) {
    b[i] = 0;
    b[j] = 0;
    ++count;
}

And rather than effectively copy & pasting our loop, we can instead calculate our starting position of the inner loop more effectively:

for (i = 0; i < len/2; ++i) {
    for (j = (len/2 + len%2); j < len; ++j) {

Notice how this produces the same result?  Your version of the code is effectively...

if (someCalculation == 1) {
    // ...
        for (j = len/2 + 1; // ...
} else if (someCalculation == 0) {
    // ...
        for (j = len/2 + 0; // ...

(Anything % 2 can only be 1 or 0.)

So we've already eliminated two levels of nesting without even moving anything out to its own function.

for (i = 0; i < len/2; ++i) {
    for (j = (len/2 + len%2); j < len; ++j) {
        if (s[i] == s[j] && b[i] && b[j]) {
            b[i] = 0;
            b[j] = 0;
            ++count;
        }
    }
}

Some other comments regarding readability...

All of your variable names are unreadably and meaninglessly short.

What is s?
What is b?
What is i?
What is j?
What is z?
I assume len is length.  Why not just use length?

We also have this mess of variable declarations:

int len,b[strlen(s)],z;
int i,j,count =0;
len=strlen(s);

I'm not familiar enough with C to know how expensive strlen is, and while I'm sure it's not horrendous, I also can't imagine it's a completely free call.  And when it does make sense to avoid duplicating function calls, we definitely should.

So for starters:

int length = strlen(inputString);

Next, I'd use any one of the options in this Stack Overflow answer to get yourself a bool type for what b is supposed to be.

bool characterChecked[length];

And then, while it is generally okay to use 1-letter variables for loop iterators, I think we should make it more clear what's going on.  If you're writing C99 code, you can just declare them straight in the loop:

for(int i = 0; // ...

And this would be ideal, especially when we consider that this limits the variables scope to only within the loop.  A variable's scope should be as limited as possible.  A variable's scope should be no larger than necessary.

But if you're on C89, this isn't an option.  As such, we should declare our variables closer to where the for loop actually exists and potentially even leave a comment.

int i;
for(i = 0; //...

Single letter variable names aren't very self-documenting.  (Loop iterators do have some special historical treatment however.  But for your other single-letter variables, we definitely need better.)

Why don't you treat the array b as a string instead of a hash table, which could cost you a larger amount of memory?

memset(b,0,128*sizeof(int));

for(z=0;z<len/2;z++)
count+=!!(++b[s[z]]);

Don't use nested loops where unnecessary.
Proceed to reverse operation from len/2+1:

for(z=len/2+len%2;z<len;z++)
{
count-=!!b[s[z]]--;
if ((b[s[z]])<0)
   break;
}

Last check:

printf("%s\n",!count?"yes":"no");

Here is a solution that could account for both memory and time.


Widget setup for Tkinter-based game I'm thinking of making a little strategic game using tkinter (player should interact with it using buttons). But my code is very repetitive.  This is a mock-up of my program:

import tkinter as tk
import random

def multi(*args):
    for func in args:
        func()

def show_hide():
    if not button["text"]:
        button.configure(text="Perform action #1", bd=2, bg="white", command=activate_deactivate)
        status_label["text"] = "Status: {}".format(status.get())
        button_2.configure(text="Perform action #2", bd=2, bg="white", command=activate_deactivate_2)
        status_label_2["text"] = "Status: {}".format(status_2.get())
    else:
        button.configure(text="", bd=0, bg="#F0F0F0", command=None)
        status_label["text"] = ""
        button_2.configure(text="", bd=0, bg="#F0F0F0", command=None)
        status_label_2["text"] = ""

def activate_deactivate():
    if status.get() == "Can be done":
        status.set("To be done")
        status_label.configure(text="Status: {}".format(status.get()), fg="blue")
    else:
        status.set("Can be done")
        status_label.configure(text="Status: {}".format(status.get()), fg="black")

def activate_deactivate_2():
    if status_2.get() == "Can be done":
        status_2.set("To be done")
        status_label_2.configure(text="Status: {}".format(status.get()), fg="blue")
    else:
        status_2.set("Can be done")
        status_label_2.configure(text="Status: {}".format(status.get()), fg="black")

def step_forward():
    if status.get() == "To be done":
        button.configure(text="", bd=0, bg="#F0F0F0", state="disabled")
        status_label["text"] = ""
        result = random.choice(["success", "failure"])
        if result == "success":
            status.set("Accomplished")
            status_label["fg"] = "green"
        else:
            status.set("Failed")
            status_label["fg"] = "red"
    else:
        button.configure(text="", bd=0, bg="#F0F0F0", command=None)
        status_label["text"] = ""

def step_forward_2():
    if status_2.get() == "To be done":
        button_2.configure(text="", bd=0, bg="#F0F0F0", state="disabled")
        status_label_2["text"] = ""
        result = random.choice(["success", "failure"])
        if result == "success":
            status_2.set("Accomplished")
            status_label_2["fg"] = "green"
        else:
            status_2.set("Failed")
            status_label_2["fg"] = "red"
    else:
        button_2.configure(text="", bd=0, bg="#F0F0F0", command=None)
        status_label_2["text"] = ""

root = tk.Tk()

status = tk.StringVar()
status.set("Can be done")

status_2 = tk.StringVar()
status_2.set("Can be done")

main = tk.Button(root, text="Show/Hide", bg="white", font="courier 30", command=show_hide)
main.pack()

frame = tk.Frame(root, pady=10)
frame.pack()

frame_1 = tk.Frame(frame, padx=10)
frame_1.pack(side="left")

frame_2 = tk.Frame(frame, padx=10)
frame_2.pack(side="left")

button = tk.Button(frame_1, text="", font="courier 20", bd=0)
button.grid(row=0, column=0)

status_label = tk.Label(frame_1, text="", font="courier 14")
status_label.grid(row=1, column=0)

button_2 = tk.Button(frame_2, text="", font="courier 20", bd=0)
button_2.grid(row=0, column=1)

status_label_2 = tk.Label(frame_2, text="", font="courier 14")
status_label_2.grid(row=1, column=1)

next_day = tk.Button(root, text="Next day", bg="white", font="courier 30",
                     command=lambda: multi(step_forward, step_forward_2))
next_day.pack()

root.mainloop()

This definitely looks terrible. With blank lines, it takes 100+ lines of code just to make 2 buttons (well, it's not only "2 buttons"), but I want to have many of these (a few groups each one containing around 4+ buttons which is a lot). If you know how "this" can be changed, I'd love to hear any suggestions (although this code is bad, it perfectly represents functionality and look of the interactive part of the UI, so these should remain the same).

The main issues, with your code, are that:

you do not factorize part of your code that are similar by correctly parametrizing your functions; and
you do not manage any common state between entities that are related.

But first, let's talk a bit about multi and __name__.

First, you can remove the need for a lambda by having multi return a callable that you will be able to provide to the command parameter. Writting something allong the lines of ..., command=multi(step_forward, step_forward_2)):

def combine(*callables):
    def runner():
        for function in callables:
            function()
    return runner

I also changed names, I feel these are more explicit.

Second, you should wrap your top-level code under an if __name__ == '__main__': clause. That way, if I want to use the building blocks you provide, I can import your file and build around it without having your UI pop up.

Now onto simplifying your code.

There is at least 3 things you are trying to achieve all at once:

Build logical blocks such as a button and a label in a frame;
Interact with such blocks from "the outside"; I believe the show/hide should be more generic than the "next step" one, but not knowing what you want to achieve with such UI, I can be misleading;
Manage states and control flow to have all your parts work together.

The first part is pretty straightforward: if you want several elements that behave like a single entity, you build a class to hold these elements together. Having done that, the second one comes naturally: you provide methods on your class so that "the outside" can interact with it. Managing states are thus nearly done: combine fed by instance method is very helpful. You only need to set it as a command on UI elements once each of them has been built.

In the following improvements, you’ll see that I provide several level of customization to be able to interact with the group of item both from the inside and the outside. Depending on your needs, you can merge some or detail even more:

import tkinter as tk
import random

class ButtonWithStatus:
    def __init__(self, root, pack_side, frame_config=None, button_config=None, label_config=None):
        if frame_config is None:
            frame_config = {}
        self._status_config = {} if label_config is None else label_config
        self._button_config = {} if button_config is None else button_config
        self._button_config['command'] = self.on_click
        self.hidden = True

        frame = tk.Frame(root, **frame_config)
        frame.pack(side=pack_side)
        self._hidden_background_color = frame.cget('bg')
        # Build hidden button
        parameters = self.hidden_parameters(self._button_config)
        self.button = tk.Button(frame, **parameters)
        self.button.grid(row=0, column=0)
        # Build label with hidden text
        parameters = self.hidden_parameters(self._status_config, False)
        self.label = tk.Label(frame, **parameters)
        self.label.grid(row=1, column=0)

    def hidden_parameters(self, configuration, is_button=True):
        parameters = configuration.copy()
        parameters.update(text='', bg=self._hidden_background_color)
        if is_button:
            parameters['command'] = None
        return parameters

    def on_click(self):
        raise NotImplementedError

    def show_hide(self):
        if self.hidden:
            self.button.configure(**self._button_config)
            self.label.configure(**self._status_config)
        else:
            parameters = self.hidden_parameters(self._button_config)
            self.button.configure(**parameters)
            parameters = self.hidden_parameters(self._status_config, False)
            self.label.configure(**parameters)
        self.hidden = not self.hidden

class ActivableButton(ButtonWithStatus):
    STATUS = 'Status: {}'

    def __init__(self, root, button_text, inactive_status, active_status):
        super().__init__(root, 'left', {'padx': 10},
                         {'text': button_text, 'font': 'courier 20', 'bd': 0},
                         {'text': self.STATUS.format(inactive_status), 'font': 'courier 14'})
        self.activated = False
        self._active = active_status
        self._inactive = inactive_status

    def on_click(self):
        self.activated = not self.activated
        if self.activated:
            self._status_config.update(text=self.STATUS.format(self._active), fg='blue')
        else:
            self._status_config.update(text=self.STATUS.format(self._inactive), fg='black')
        self.label.configure(**self._status_config)

class ValidableActivableButton(ActivableButton):
    def __init__(self, root, button_text):
        super().__init__(root, button_text, 'Can be done', 'To be done')
        self._done = False

    def step_forward(self):
        if self._done:
            return

        if self.activated:
            self._button_config.update(text='', bd=0, bg='#F0F0F0', state='disabled')
            if random.randint(0, 1):
                self._status_config.update(text='Accomplished', fg='green')
            else:
                self._status_config.update(text='Failed', fg='red')
            self._done = True
        self.button.configure(**self._button_config)
        self.label.configure(**self._status_config)

def combine(*callables):
    def runner():
        for function in callables:
            function()
    return runner

if __name__ == '__main__':
    root = tk.Tk()
    main = tk.Button(root, text="Show/Hide", bg="white", font="courier 30")
    main.pack()

    frame = tk.Frame(root, pady=10)
    frame.pack()

    button1 = ValidableActivableButton(frame, 'Perform action #1')
    button2 = ValidableActivableButton(frame, 'Perform action #2')

    main['command'] = combine(button1.show_hide, button2.show_hide)

    next_day = tk.Button(root, text="Next day", bg="white", font="courier 30",
                         command=combine(button1.step_forward, button2.step_forward))
    next_day.pack()

    root.mainloop()

Several other improvements:

usage of booleans instead of text to determine toggable state;
usage of the "default" background color of the container to determine the background color of the hidden state;
usage of dictionaries to store/restore widget configuration.

Depending on your needs, you may want to add/remove some of the parameter of the constructors. Or even combine ValidableActivableButton and ActivableButton into a single class.

You also don't really need to manage the hidden state yourself by removing text (and optionnally changing background color) as your can hide widgets pretty easily. This allows you to manipulate your widgets directly or using configure without worying about storing their state for future hide/show:

import tkinter as tk
import random

class ButtonWithStatus:
    def __init__(self, root, pack_side, frame_config=None, button_config=None, label_config=None):
        if frame_config is None:
            frame_config = {}
        if button_config is None:
            button_config = {}
        if label_config is None:
            label_config = {}
        button_config['command'] = self.on_click

        frame = tk.Frame(root, **frame_config)
        frame.pack(side=pack_side)
        self.button = tk.Button(frame, **button_config)
        self.button.grid(row=0, column=0)
        self.label = tk.Label(frame, **label_config)
        self.label.grid(row=1, column=0)

        # Hide label and button
        self.button.grid_remove()
        self.label.grid_remove()
        self.hidden = True

    def on_click(self):
        raise NotImplementedError

    def show_hide(self):
        if self.hidden:
            self.button.grid()
            self.label.grid()
        else:
            self.button.grid_remove()
            self.label.grid_remove()
        self.hidden = not self.hidden

class ActivableButton(ButtonWithStatus):
    STATUS = 'Status: {}'

    def __init__(self, root, button_text, inactive_status, active_status):
        super().__init__(root, 'left', {'padx': 10},
                         {'text': button_text, 'font': 'courier 20', 'bd': 0},
                         {'text': self.STATUS.format(inactive_status), 'font': 'courier 14'})
        self.activated = False
        self._done = False
        self._active = active_status
        self._inactive = inactive_status

    def on_click(self):
        self.activated = not self.activated
        if self.activated:
            self.label['text'] = self.STATUS.format(self._active)
            self.label['fg'] = 'blue'
        else:
            self.label['text'] = self.STATUS.format(self._inactive)
            self.label['fg'] = 'black'

    def step_forward(self):
        if self._done:
            return

        if self.activated:
            self.button['text'] = ''
            self.button['bd'] = 0
            self.button['bg'] = '#F0F0F0'
            self.button['state'] = 'disabled'
            if random.randint(0, 1):
                self.label['text'] = 'Accomplished'
                self.label['fg'] = 'green'
            else:
                self.label['text'] = 'Failed'
                self.label['fg'] = 'red'
            self._done = True

def combine(*callables):
    def runner():
        for function in callables:
            function()
    return runner

if __name__ == '__main__':
    root = tk.Tk()
    main = tk.Button(root, text="Show/Hide", bg="white", font="courier 30")
    main.pack()

    frame = tk.Frame(root, pady=10)
    frame.pack()

    button1 = ActivableButton(frame, 'Perform action #1', 'Can be done', 'To be done')
    button2 = ActivableButton(frame, 'Perform action #2', 'Can be done', 'To be done')

    main['command'] = combine(button1.show_hide, button2.show_hide)

    next_day = tk.Button(root, text="Next day", bg="white", font="courier 30",
                         command=combine(button1.step_forward, button2.step_forward))
    next_day.pack()

    root.mainloop()

I rewrote my code. If you, for any reason, want to see how this can be improved, here's what I've got:

import tkinter as tk
import random

class ActionButton(tk.Button):
    def __init__(self, *args, **kwargs):
        tk.Button.__init__(self, *args, **kwargs)
        self.configure(text="", font="courier 20", bd=0)

class ActionLabel(tk.Label):
    def __init__(self, *args, **kwargs):
        tk.Label.__init__(self, *args, **kwargs)
        self.configure(text="", font="courier 14")

def multi(*args):
    for func in args:
        return func

def show_hide(a, b, c, d):
    if not b[0]["text"]:
        b[0].configure(text="{}".format(d[0]), bd=2, bg="white",
                       command=lambda: activate_deactivate(a[0], c[0]))
        c[0]["text"] = "Status: {}".format(a[0].get())
        b[1].configure(text="{}".format(d[1]), bd=2, bg="white",
                       command=lambda: activate_deactivate(a[1], c[1]))
        c[1]["text"] = "Status: {}".format(a[1].get())
    else:
        b[0].configure(text="", bd=0, bg="#F0F0F0", command=None)
        c[0]["text"] = ""
        b[1].configure(text="", bd=0, bg="#F0F0F0", command=None)
        c[1]["text"] = ""

def activate_deactivate(a, b):
    if a.get() == "Can be done":
        a.set("To be done")
        b.configure(text="Status: {}".format(a.get()), fg="blue")
    else:
        a.set("Can be done")
        b.configure(text="Status: {}".format(a.get()), fg="black")

def step_forward(a, b, c):
    if a.get() == "To be done":
        b.configure(text="", bd=0, bg="#F0F0F0", state="disabled")
        c["text"] = ""
        result = random.choice(["success", "failure"])
        if result == "success":
            a.set("Accomplished")
            c["fg"] = "green"
        else:
            a.set("Failed")
            c["fg"] = "red"
    else:
        b.configure(text="", bd=0, bg="#F0F0F0", command=None)
        c["text"] = ""

root = tk.Tk()

status = tk.StringVar()
status.set("Can be done")

status_2 = tk.StringVar()
status_2.set("Can be done")

main = tk.Button(root, text="Show/Hide", bg="white", font="courier 30",
                 command=lambda: show_hide([status, status_2],
                                           [button, button_2],
                                           [label, label_2],
                                           ["Perform action #1", "Perform action #2"]))
main.pack()

frame = tk.Frame(root, pady=10)
frame.pack()

frame_1 = tk.Frame(frame, padx=10)
frame_1.pack(side="left")

frame_2 = tk.Frame(frame, padx=10)
frame_2.pack(side="left")

button = ActionButton(frame_1)
button.grid(row=0, column=0)

label = ActionLabel(frame_1)
label.grid(row=1, column=0)

button_2 = ActionButton(frame_2)
button_2.grid(row=0, column=1)

label_2 = ActionLabel(frame_2)
label_2.grid(row=1, column=1)

next_day = tk.Button(root, text="Next day", bg="white", font="courier 30",
                     command=lambda: multi(step_forward(status, button, label),
                                           step_forward(status_2, button_2, label_2)))
next_day.pack()

root.mainloop()

CHANGES DONE:

1) I created 2 classes for Label and Button - no need to apply same text, border size, etc. to those.

class ActionButton(tk.Button):
    def __init__(self, *args, **kwargs):
        tk.Button.__init__(self, *args, **kwargs)
        self.configure(text="", font="courier 20", bd=0)

class ActionLabel(tk.Label):
    def __init__(self, *args, **kwargs):
        tk.Label.__init__(self, *args, **kwargs)
        self.configure(text="", font="courier 14")

# code...
# code...
# code...

button = ActionButton(frame_1)
button.grid(row=0, column=0)

label = ActionLabel(frame_1)
label.grid(row=1, column=0)

button_2 = ActionButton(frame_2)
button_2.grid(row=0, column=1)

label_2 = ActionLabel(frame_2)
label_2.grid(row=1, column=1)

2) Now all functions  were changed to use parameters. Instead of rewriting the same, they all take arguments.

def show_hide(a, b, c, d):
    if not b[0]["text"]:
        b[0].configure(text="{}".format(d[0]), bd=2, bg="white",
                       command=lambda: activate_deactivate(a[0], c[0]))
        c[0]["text"] = "Status: {}".format(a[0].get())
        b[1].configure(text="{}".format(d[1]), bd=2, bg="white",
                       command=lambda: activate_deactivate(a[1], c[1]))
        c[1]["text"] = "Status: {}".format(a[1].get())
    else:
        b[0].configure(text="", bd=0, bg="#F0F0F0", command=None)
        c[0]["text"] = ""
        b[1].configure(text="", bd=0, bg="#F0F0F0", command=None)
        c[1]["text"] = ""

def activate_deactivate(a, b):
    if a.get() == "Can be done":
        a.set("To be done")
        b.configure(text="Status: {}".format(a.get()), fg="blue")
    else:
        a.set("Can be done")
        b.configure(text="Status: {}".format(a.get()), fg="black")

def step_forward(a, b, c):
    if a.get() == "To be done":
        b.configure(text="", bd=0, bg="#F0F0F0", state="disabled")
        c["text"] = ""
        result = random.choice(["success", "failure"])
        if result == "success":
            a.set("Accomplished")
            c["fg"] = "green"
        else:
            a.set("Failed")
            c["fg"] = "red"
    else:
        b.configure(text="", bd=0, bg="#F0F0F0", command=None)
        c["text"] = ""

# code...
# code...
# code...

main = tk.Button(root, text="Show/Hide", bg="white", font="courier 30",
                 command=lambda: show_hide([status, status_2],
                                           [button, button_2],
                                           [label, label_2],
                                           ["Perform action #1", "Perform action #2"]))
main.pack()

next_day = tk.Button(root, text="Next day", bg="white", font="courier 30",
                     command=lambda: multi(step_forward(status, button, label),
                                       step_forward(status_2, button_2, label_2)))
next_day.pack()

NOTE:
It's still imperfect ("def show_hide" is still repetitive, and yet idk what can be done with it) but this is definitely a big step forward from what it was.


Checking whether one string matches another This checks whether one string has all the chars in another string to make up that exact string. Counts matter so for, for example, str1 = 'abcdgbff' str2 ='aaabb' should return false because str1 only has one 'a' and 'b'.

How can I optimize my code further?

function scramble(str1, str2) {
  for (let i of str1) {
    if (str2.indexOf(i) != -1) {
      str2 = str2.slice(0, (str2.indexOf(i))) + str2.slice(str2.indexOf(i) + 1);
    }
  }
  return str2.length === 0;
}

// Example:
console.log(scramble('abcdgbff', 'aaabb')); // false

If your input strings are of length n, your current implementation has a cubic runtime complexity O(n³). This could become an issue for longer strings.

I recommend to split counting character frequencies and comparing those counts into two separate steps with nearly linear complexity O(n):

// Count characters in 'str':
function frequencies(str) {
  let result = {};
  for (let chr of str) {
    result[chr] = (result[chr] || 0) + 1;
  }
  return result;
}

// Check if 'str1' contains all characters of 'str2':
function scramble(str1, str2) {
  let frequencies1 = frequencies(str1);
  let frequencies2 = frequencies(str2);
  return Object.entries(frequencies2).every(([chr2, frequency2]) =>
    frequencies1[chr2] >= frequency2
  );
}

// Examples:
console.log(scramble("abc", "cba"));
console.log(scramble("aa", "aaaa"));
console.log(scramble("123", "234"));

I can't program in JavaScript but i will tell you, your dealing with strings, so if you look at the type of input and know that the third letter into a word is usualy a vowl, but the last is usually a consent, then you can try to make your code error/False statement as fast as possible, if you choose to measure length then chose Consent positions to compare first.


HackerRank "Service Lane" in Python Problem statement:

  Calvin is driving his favorite vehicle on the 101 freeway. He notices that the check engine light of his vehicle is on, and he wants to service it immediately to avoid any risks. Luckily, a service lane runs parallel to the highway. The length of the highway and the service lane is N units. The service lane consists of N segments of unit length, where each segment can have different widths.

  Calvin can enter into and exit from any segment. Let's call the entry segment as index i and the exit segment as index j. Assume that the exit segment lies after the entry segment(j>i) and i ≥ 0. Calvin has to pass through all segments from index i to indexj (both inclusive).

  Calvin has three types of vehicles - bike, car and truck, represented by 1, 2 and 3 respectively. These numbers also denote the width of the vehicle. We are given an array width[] of length N, where width[k] represents the width of kth segment of our service lane. It is guaranteed that while servicing he can pass through at most 1000 segments, including entry and exit segments.

  If width[k] is 1, only the bike can pass through kth segment.
  If width[k] is 2, the bike and car can pass through kth segment.
  If width[k] is 3, any of the bike, car or truck can pass through kth segment.

  Given the entry and exit point of Calvin's vehicle in the service lane, output the type of largest vehicle which can pass through the service lane (including the entry & exit segment)

  Input Format

  The first line of input contains two integers - N & T, where N is the length of the freeway, and T is the number of test cases. The next line has N space separated integers which represents the width array.

  T test cases follow. Each test case contains two integers - i & j, where i is the index of segment through which Calvin enters the service lane and j is the index of the lane segment where he exits.

  Output Format

  For each test case, print the number that represents the largest vehicle type that can pass through the service lane.

  Note:
  Calvin has to pass through all segments from index i to index j (both inclusive).

  Constraints:

  2 <= N <= 100000

  1 <= T <= 1000

  0 <= i < j < N

  2 <= j-i+1 <= min(N,1000)

  1 <= width[k] <= 3, where 0 <= k < N

Solution:

N, T = map(int, input().split())
widths = list(map(int, input().split()))
for _ in range(T):
    i, j = map(int, input().split())
    vehicle = 3
    for x in range(i, j+1):
        width = widths[x]
        if width == 1:
            vehicle = 1 # Only a bike can pass
            break
        elif width == 2:
            vehicle = 2 # Only a car or bike can pass
    print(vehicle)

I'm looking for any way to make this shorter, simpler or more Pythonic.

Shorter, simpler, and more Pythonic would be:

N, T = map(int, input().split())
widths = list(map(int, input().split()))
for _ in range(T):
    i, j = map(int, input().split())
    vehicle = min(widths[segment] for segment in range(i, j + 1))
    print(vehicle)

You would miss out on the shortcut when encountering a bike-only segment, though.

I've also renamed x to segment for clarity.

My solution to the problem, provided you don't mind the time complexity and prefer a pythonic(precise) code -

def serviceLane(n, cases):
    for i,j in cases:
        return min(width[i:j+1])


Extracting information from JSON Here is a simplified version of my JSON.

{
    "Relations": [
        {
            "Type": "A",
            "Categories": [
                { "Name": "Identity" }
            ]
        },
        {
            "Type": "B",
            "Categories": [
                { "Name": "Identity" },
                {
                    "Name": "Contact Information",
                    "Type": "Phone"
                },
                { "Name": "Addresses" },
                { "Name": "Bank Accounts" }
            ]
        },
        {
            "Type": "C",
            "Categories": [
                { "Name": "Identity" },
                { "Name": "Contact Information" },
                { "Name": "Service Fields" }
            ]
        }
    ]
}

My code generates an IEnumerable (List, but I am open to suggestions if there is a more compatible option) that holds all distinct categories, meaning, when joining its values (using string.Join("\n", collection)) it contains the following JSON:

{ "Name": "Identity" }
{
    "Name": "Contact Information"
    "Type": "Phone"
}
{ "Name": "Addresses" }
{ "Name": "Bank Accounts" }
{ "Name": "Contact Information" }
{ "Name": "ServiceFields" }

This is the code:

List<dynamic> Method()
{
    string jString = File.ReadAllText(@"file.json");
    dynamic jObject = JsonConvert.DeserializeObject<dynamic>(jString);

    // The variable in questions
    var categories = new List<dynamic>();
    foreach (dynamic relation in jObject.Relations)
    {
        foreach (dynamic category in relation.Categories)
        {
            if (!CollectionContainsItem(categories, category))
            {
                categories.Add(category);
            }
        }
    }

    return categories;

    bool CollectionContainsItem(IEnumerable<dynamic> collection, JToken searchedItem)
    {
        foreach (var item in collection)
        {
            if (JToken.DeepEquals(item, searchedItem))
            {
                return true;
            }
        }
        return false;
    }
}

How to improve my code? It seems very cumbersome to me, especially the local method.

Note: I need a dynamic approach. I do not want to create C# classes for the JSON data.

Thanks!

Yes, your code can indeed be simplified, as Xiaoy312 already demonstrated:

// Obtain the Relations array:
return JObject.Parse(jsonString)["Relations"]
     // Take the categories of all relations, flattening them into a single enumerable:
    .SelectMany(relation => relation["Categories"])
    // Filter out duplicate category objects (JToken.EqualityComparer uses DeepEquals internally):
    .Distinct(JToken.EqualityComparer)
    // Materialize the results into a list:
    .ToList();

This is also more efficient:

Using dynamic means that several things (such as method overload resolution) cannot be done at compile-time but must be done at run-time, which involves additional work.
Distinct uses a set internally, so it doesn't need to compare a new category against all other categories found so far - an internal hashcode lookup is often sufficient.

dynamic can be useful, but it doesn't work with extension methods (which Linq heavily depends on), so in this case it's more of a hindrance. About the only benefit here is that relation.Categories is slightly easier to write than relation["Categories"], but you do pay for that at run-time, and it handicaps your tools (no compile-time checks, no automated refactoring). Also keep in mind that dynamic is 'contagious': any expression involving something dynamic also becomes dynamic.

Another problem is that dynamic code is less self-descriptive. Is category.Name a string, or could it be a { "name": "Identity", "id": "5" } object? That sort of uncertainty makes code more difficult to maintain, at least in my experience. Static typing can be more restrictive and cumbersome, but it gives you more guarantees and tells you more about what sort of data your program expects to handle. Unless you have a good reason to stick with dynamic (in that case, throw a .Cast<dynamic>() in just before the .ToList() call) I would recommend creating a Category class.

Finally, I would make the filename a parameter and rename Method to something more descriptive such as LoadCategoriesFromJsonFile, and I would put some thoughts into error handling (what if the file doesn't exist, or if it's corrupted).

Maybe try a two step process:

First, convert the Json to XML. Various libraries can do the conversion.
Second, use LINQ to query the XML. LINQ documentation has examples.

Expanding upon this answer. The original poster wanted less cumbersome code. Well, why reinvent the wheel?  Converting the Json to XML can be done with one line of code (assuming a library function). LINQ is an query language for accessing data stored in enumerable objects like XML.  All the "name" elements can be extracted in one (or more, depending on code formatting style) line of code. That is less cumbersome than the original posters code.


JavaScript bottom-up tree transformer aimed for performance function transform(tree,fn){
    var root = tree,
        node = tree,
        child,
        parent,
        is_node,
        dir;
    root.dir = 0;
    while(true) {
        is_node = typeof(node)==="object";
        dir = is_node ? node.dir : 2;
        if (dir < 2)
            child = node[dir],
            node.dir++,
            child.parent = parent = node,
            child.dir = 0,
            node = child;
        else if ((changed = fn(node))!==undefined)
            changed.parent = parent,
            changed.dir = 0,
            node = changed;
        else
            if (!parent)
                return node;
            else
                parent[parent.dir-1] = node,
                node = parent,
                parent = node.parent;
    };
};

// TEST
var tree = [[1,2],[[3,4],[5,6]]];
console.log(
    JSON.stringify(transform(tree,function(a){
        if (a[0]===1) return [3,[5,5]];
        if (a[0]===5) return 77;
    })) === "[[3,77],[[3,4],77]]");

I need to make this function as fast as possible, no matter what. I'm aware this is far from the optimal implementation. How could I improve it? Asm.js, maybe?

Hmmm,

I don't think your code works,

var tree = [[1,2,[1,2,[5]]],[[3,4],[5,6]],[1,2,[1,2,[5]]],[[3,4],[5,6]]];

gives

[[3, 77], [[3, 4], 77], [1, 2, [1, 2, [5]]], [[3, 4], [5, 6]]]

I would have expected that you want that last [5,6] to be 77 in your fiddle.

Furthermore, as RoToRa mentioned

It is not clear in your code what you want to achieve
Comma separated statements to avoid curly braces are evil, python much?
Going through a tree pretty much asks for a recursive solution because you need to keep track of where you are

This function seems to do what you want thru recursion:

function transform( node, f )
{
  node = f(node) || node;
  var index = node.length;
  if( index )
  {
    while( index-- )
    {
      node[index] = transform( node[index] , f );
    }
  }
  return node;
}

Or, if you drop those curlies which you seem to like:

function transform( node, f )
{
  node = f(node) || node;
  var index = node.length;
  if( index )
    while( index-- )
      node[index] = transform( node[index] , f );
  return node;
}

I originally wanted to use [].map which would be cleaner, however you insisted speed.

Update: I checked jsperf for the fastest loop and it turns out there is a new loop variant that blows the other approaches out of the water: http://jsperf.com/loops/142

Using this approach the code would look like this:

function transform( node, f )
{
  node = f(node) || node;
  if( node.length )
    for (var i = 0, child; child = node[i]; i++)
      node[i] = transform( child , f );
  return node;
}

Ok, I've decided to write something after all, however not about the performance, but about the coding style.

First off you should give more information what your function actually does. It took me a lot of time to understand that it you are working with binary trees here.

I don't like your use of the comma operator. All code conventions I know discourage its use like this.

Normally I'm fine with declaring all the variables at the start of the function instead of when they are used. However in this case, the declaration makes it look like all of the
variables are used "globally" within the function, although most of them are just temporal use. This makes reading the code very difficult.

Additionally some of your variables (mainly parent and dir) seem to just duplicate the properties of the same name of the current node. This makes tracking the use of the variables and it's matching property very confusing IMHO.

Actually lets go through the variables one by one:

root: With exception of setting the first dir property unused, and thus completely unnecessary.
node: Should be called currentNode.
child: One of the variables that seem to be "global", however is just used temporally in the dir < 2 block. It should be declared there.
parent: I admit I haven't quite undesrtood the point of this (and the corresponding parent property), but I'm quite sure you are just duplicating the value of node.parent. I think it should be possible to get rid of it all together.
is_node: Like child just a temporary variable, so it's shouldn't be declared at the beginning of the function. Also it's named very badly. You are not using it when checking if a node "is a node", but if a node is an array (or has children, or if's not a leaf). So it should be called is_array, has_children or (if logically switched) is_leaf. However at the end, you don't need it at all IMHO. Instead just merge the two lines you are using it in.
dir: Sorry, but it's the worst of them all:

Again just a temporary variable, that shouldn't be declared at the start of the function.
It also just duplicates the value of node.dir.
Very badly named. "dir" normally stands for "direction", which doesn't apply here. It's the "current index".
Worst of all: The use of the "magic" (and yet completely meaningless) number 2.
Conclusion: Get rid of it. It's completely unnecessary.

Instead of:

is_node = typeof(node)==="object";
dir = is_node ? node.dir : 2;
if (dir < 2)
  child = node[dir],

use

if (typeof(node)==="object")
  child = node[node.dir],

There is much more, but I don't have the time right now. Maybe I'll come back.


Two Datasets Same Query I have a form that allows the user to select a server.  This will execute the exact same query just on a different server.

private void QueryButton_Click(object sender, EventArgs e)
{
    switch (this.FindByComboBox.SelectedIndex)
    {
        case 0:
            ByTargetVDN();
            break;
        // ...
    }
}
void ByTargetVDN()
{
    int value = 0;
    if (this.ValueTextBox.Text.Length > 0)
    {
        value = Convert.ToInt32(this.ValueTextBox.Text);
    }
    switch (this.ServerComboBox.SelectedIndex)
    {
        case 0:
            var gr01 = new GR01.TargetVDNDataTable();
            var adapter = new ICMQuery.Models.GR01TableAdapters.TargetVDNTableAdapter();
            if (value == 0)
            {
                adapter.Fill(gr01);
            }
            else
            {
                adapter.FillByPeripheralNumber(gr01, value);
            }
            this.FindScriptsDataGrid.DataSource = gr01;
            break;
        case 1:
            var ds01 = new DS01.TargetVDNDataTable();
            var adapterNew = new ICMQuery.Models.DS01TableAdapters.TargetVDNTableAdapter();
            if (value == 0)
            {
                adapterNew.Fill(ds01);
            }
            else
            {
                adapterNew.FillByPeripheralNumber(ds01, value);
            }
            this.FindScriptsDataGrid.DataSource = ds01;
            break;
    }
}

There has got to be a way I can streamline this.  I was hoping for something along the lines of

switch (this.ServerComboBox.SelectedIndex)
  {
    case 0:
      var ds = new GR01.TargetVDNDataTable();
      var adapter = new ICMQuery.Models.GR01TableAdapters.TargetVDNTableAdapter();
      break;
    case 1:
      ds = new GR01.TargetVDNDataTable();
      adapter = new ICMQuery.Models.GR01TableAdapters.TargetVDNTableAdapter();
      break;
  }
if (value == 0)
  {
    adapter.Fill(ds);
  }
else
  {
    adapter.FillByPeripheralNumber(ds, value);
  }
this.FindScriptsDataGrid.DataSource = ds;

but of course this doesn't work.

if (this.ValueTextBox.Text.Length > 0)
{
    value = Convert.ToInt32(this.ValueTextBox.Text);
}

This will throw a nullpointerexception if Text is null, use TryParse instead

int value;
bool result = Int32.TryParse(ValueTextBox.Text, out value);
if(result){
 // use value
}

I don't recommend you using SelectedIndex, because it breaks your code if the ordering get changed in the future, I would use SelectedItem instead.

Notice that you are repeating code, which is not a brilliant idea.

TheTypeForThis gr01 = null;
TheTypeForThis adapter = null;
switch (this.ServerComboBox.SelectedIndex)
{
    case 0:
        gr01 = new GR01.TargetVDNDataTable();
        adapter = new ICMQuery.Models.GR01TableAdapters.TargetVDNTableAdapter();
        break;
    case 1:
        var ds01 = new DS01.TargetVDNDataTable();
        var adapterNew = new ICMQuery.Models.DS01TableAdapters.TargetVDNTableAdapter();
       break;
}

 if (value == 0)
 {
    adapter.Fill(gr01);
 }
 else
 {
    adapter.FillByPeripheralNumber(gr01, value);
 }
 this.FindScriptsDataGrid.DataSource = gr01;

I assume it's your style to refer to all class using "this.". That's not the usual way to do things but it is acceptable.
It also seems to be your style to capitalize the names of the UI elements. Again, not the usual style.
You really need to separate the UI and the business logic. The way things stand now, it would be impossible to drive the program via an API or a batch process. Especially notable since that's usually the best way to test it.
Especially since you haven't included the definitions of classes GR01 and DS01 are, it's impossible to tell from this code snippet what gr01 and ds01 are.
The biggest problem seems to be that you're declaring adapter and adapternew inside the switch. Move those declarations outside, and you should only need one variable.


Compress svg files in PHP I wrote something to "compress" svg files. The svg files I am using often have comments and empty <g> tags, and I want to remove them. My main goal is not the speed of the compression, but the size of the compressed svg file.

Here is an example svg file: https://image.flaticon.com/icons/svg/222/222436.svg

And here is the code I am using:

public function compress($svg)
{
    $svg = preg_replace('/<!--.*-->/', '', $svg);
    $svg = preg_replace('/<g>[\n\r\s]*<\/g>/', '', $svg);
    $svg = preg_replace('/\n/', ' ', $svg);
    $svg = preg_replace('/\t/', ' ', $svg);
    $svg = preg_replace('/\s\s+/', ' ', $svg);
    $svg = str_replace('> <', '><', $svg);
    $svg = str_replace(';"', '"', $svg);

    return $svg;
}

Do you see any dangers here, perhaps this could ruin some svg files?
Is there something I could do to compress the file even more?
Is there any way to speed this up?

SVG is structured data and should therefore not require regex to work with it.  I would suggest that you work with either an XML or SVG-specific library in PHP which would let you parse in the input SVG, remove any unwanted nodes and then render it back out.

Here is a starting point for common PHP tools for working with XML that should be able to do what you want.

This also looks like a library that could meet your needs.

$iconfile = new DOMDocument();
$iconfile->load($source);

$svg = $iconfile->saveHTML($iconfile->getElementsByTagName('svg')[0]);
$svg = preg_replace('/<!--.*-->/', '', $svg);
$svg = preg_replace('/<g>[\n\r\s]*<\/g>/', '', $svg);
$svg = preg_replace('/\n/', ' ', $svg);
$svg = preg_replace('/\t/', ' ', $svg);
$svg = preg_replace('/\s\s+/', ' ', $svg);
$svg = str_replace('> <', '><', $svg);
$svg = str_replace(';"', '"', $svg);

echo $svg;


Get list of files in directory with exclude option This method returns the list of files (absolute path) in a folder (or tree). It allows filtering by extensions or filenames.

The method receives the following parameters:

string path: folder path to scan for files.
string[] exclude: can contain filenames such as "read.me" or extensions such as "*.jpg".
SearchOption searchOption: TopDirectoryOnly to scan only the specified folder or AllDirectories to scan tree folder under the specified path.

The method return files that doesn't appear in exclude array and a) hasn't extension, or b) extension doesn't appear in exclude array.

    public static IEnumerable<string> GetFiles(string path, string[] exclude, SearchOption searchOption = SearchOption.AllDirectories)
    {
        IEnumerable<string> files = Directory.EnumerateFiles(path, "*.*", searchOption);
        var resultFiles = new List<string>();

        if (exclude.Length > 0)
        {
            foreach (var filename in files)
            {
                string extension = Path.GetExtension(filename);

                if (Array.IndexOf(exclude, Path.GetFileName(filename)) >= 0)
                {
                    continue;
                }

                if (string.IsNullOrEmpty(extension) || Array.IndexOf(exclude, "*" + extension) < 0)
                {
                    resultFiles.Add(filename);
                }
            }
        }

        return resultFiles;
    }

A few things I noticed:

If the exclude array is empty you return an empty list instead of every file.
reversing your logic would allow you to check everything in one statement.
Since you're returning an IEnumerable<string> already, using the yield operator would make sense here.
Accepting an IEnumerable<string> instead of an string[] for the exclude parameter, gives the user more options.  Also this allows you to leverage the LINQ extensions better.

Your code could look something like this:

public static IEnumerable<string> GetFiles(string path, IEnumerable<string> exclude, SearchOption searchOption = SearchOption.AllDirectories)
{
    IEnumerable<FileInfo> files = new DirectoryInfo(path).EnumerateFiles("*.*", searchOption);
    foreach (var filename in files)
    {

        if (!exclude.Any(x => x == filename.Name || (x.StartsWith("*") && x.Contains(filename.Extension))))
        {
            yield return filename.Name;
        }

    }
}

Everything that you have written there can be done with a call to: Directory.GetFiles(string, string, SearchOption)