Imperative code in R keeping track of state A friend of mine has a spreadsheet where she coded a classroom video for various teaching events, which I needed to help her plot. For each recording, she used several video cassettes, and her time codes were relative to each video cassette (ie. they always start on 0). To be able to plot these on a timeline, I needed to convert the times into a relative (continuous) time. The code below works perfectly (and I've solved the problem), however it doesn't look good to me. It looks like the code I used to write in Ruby, before I began playing with R, Haskell etc.

This is not the first time I've come across this kind of a problem - iterating through rows, or lines of text, needing to keep track of several different kinds of "state", etc. I would be curious if this code could be rewritten to look cleaner, more idiomatic.

Example dataset

structure(list(code = structure(c(4L, 3L, 1L, 1L, 5L, 4L, 3L,
1L, 5L, 2L), .Label = c("CC-NoZZ", "CC-ZZ", "FBP", "KK-NoZZ",
"KK-ZZ"), class = "factor"), src = structure(c(1L, 1L, 1L, 2L,
2L, 3L, 3L, 4L, 4L, 4L), .Label = c("alfred1.mov", "alfred2.mov",
"claire1.mov", "claire2.mov"), class = "factor"), class = structure(c(1L,
1L, 1L, 1L, 1L, 2L, 2L, 2L, 2L, 2L), .Label = c("alfred's class",
"claire's class"), class = "factor"), start = c(0L, 2L, 6L, 0L,
10L, 1L, 5L, 0L, 6L, 14L), end = c(2L, 4L, 8L, 4L, 12L, 3L, 9L,
4L, 10L, 20L)), .Names = c("code", "src", "class", "start", "end"
), class = "data.frame", row.names = c(NA, -10L))

Example code

src = ""
class = ""
delta = 0
db$corr.start = NA
db$corr.end = NA
db$delta = NA

for(i in 1:nrow(db)) {
    row <- db[i,]
    if(row$src != src) {
      delta = db[i-1,]$end + delta
      src = row$src
    }

    if(row$class != class) {
      delta = 0
      class = row$class
    }

    db[i,]$corr.start = db[i,]$start + delta
    db[i,]$corr.end = db[i,]$end + delta

    db[i,]$delta = delta
}

Example output

      code         src          class start end corr.start corr.end delta
1  KK-NoZZ alfred1.mov alfred's class     0   2          0        2     0
2      FBP alfred1.mov alfred's class     2   4          2        4     0
3  CC-NoZZ alfred1.mov alfred's class     6   8          6        8     0
4  CC-NoZZ alfred2.mov alfred's class     0   4          8       12     8
5    KK-ZZ alfred2.mov alfred's class    10  12         18       20     8
6  KK-NoZZ claire1.mov claire's class     1   3          1        3     0
7      FBP claire1.mov claire's class     5   9          5        9     0
8  CC-NoZZ claire2.mov claire's class     0   4          9       13     9
9    KK-ZZ claire2.mov claire's class     6  10         15       19     9
10   CC-ZZ claire2.mov claire's class    14  20         23       29     9

R encourages vector-based operations instead of loops. But in this case, I don't think there's a good way around tracking the state of delta.

This might be a slightly better way to create a vector delta:

delta <- 0
vdelta <- c(0)
for (i in 2:nrow(db)) {
  row <- db[i,]
  prev <- db[i-1,]
  if (row$class != prev$class) {
    delta <- 0
  } else if (row$src != prev$src) {
    delta <- prev$end + delta
  }
  vdelta <- c(vdelta, delta)
}

Based on vdelta you can easily create the columns you need:

db$corr.start <- db$start + vdelta
db$corr.end <- db$end + vdelta
db$delta <- vdelta


StreamIterable - create an iterable from a Java 8 Stream This seems like an easy thing to mess up, so I'd be grateful if you could think of any edge-cases that I've missed.

public class StreamIterable<T> implements Iterable<T> {

  private final Stream<T> stream;

  public StreamIterable(Stream<T> stream) {
    this.stream = stream;
  }

  @Override
  public Iterator<T> iterator() {
    return new StreamIterator<>(stream);
  }
}

where

public class StreamIterator<T> implements Iterator<T> {
  private final Spliterator<T> spliterator;

  private boolean nextIsKnown = false;
  private T next = null;

  public StreamIterator(Stream<T> stream) {
    this.spliterator = stream.spliterator();
  }

  @Override
  public boolean hasNext() {
    if (nextIsKnown)
        return true;
    return spliterator.tryAdvance(t -> {next = t; nextIsKnown = true;});
  }

  @Override
  public T next() {
    if (nextIsKnown) {
        return resetAndReturnNext();
    }
    if (!hasNext())
        throw new NoSuchElementException();
    return resetAndReturnNext();
  }

  private T resetAndReturnNext() {
    T result = next;
    nextIsKnown = false;
    next = null;
    return result;
  }
}

Tests

public class StreamIteratableTest {

    @Test
    public void empty() {
        assertThat(new StreamIterable<>(Stream.<String>empty()), Matchers.emptyIterable());
    }

    @Test
    public void matcher() {
        Stream<String> stream = Stream.of("1", "2", "3");
        assertThat(new StreamIterable<>(stream), Matchers.contains("1", "2", "3"));
    }

    @Test
    public void torture_empty() {
        StreamIterator<Object> empty = new StreamIterator<>(Stream.empty());

        assertFalse(empty.hasNext());
        try {
            empty.next();
            fail();
        } catch (NoSuchElementException expected) {
        }

        assertFalse(empty.hasNext());
        assertFalse(empty.hasNext());
        try {
            empty.next();
            fail();
        } catch (NoSuchElementException expected) {
        }
        try {
            empty.next();
            fail();
        } catch (NoSuchElementException expected) {
        }
    }

    @Test
    public void dont_call_hasNext() {
        StreamIterator<Object> twoItems = new StreamIterator<>(Stream.of("1", "2"));

        assertEquals("1", twoItems.next());
        assertEquals("2", twoItems.next());
        try {
            twoItems.next();
            fail();
        } catch (NoSuchElementException expected) {
        }
    }

    @Test
    public void repeat_call_hasNext() {
        StreamIterator<Object> twoItems = new StreamIterator<>(Stream.of("1", "2"));

        assertTrue(twoItems.hasNext());
        assertTrue(twoItems.hasNext());
        assertEquals("1", twoItems.next());
        assertTrue(twoItems.hasNext());
        assertEquals("2", twoItems.next());
        assertFalse(twoItems.hasNext());
        assertFalse(twoItems.hasNext());
    }
}

Two things:

I think you have missed the native implementation. Are you intentionally re-inventing the wheel? Streams have an iterator() method.
Note that streams cannot be reversed, so, while you can create an iterator once, from the stream, you cannot create a second iterator.... In other words, you cannot loop more than once through your iterable.

Consider reworking your class as:

public class StreamIterable<T> implements Iterable<T> {

  private final Stream<T> stream;

  public StreamIterable(Stream<T> stream) {
    this.stream = stream;
  }

  @Override
  public Iterator<T> iterator() {
    return stream.iterator();
  }
}

That's it, no additional code needed. You can still only iterate it once, but, you can add the above to an enhanced-for loop:

for (String v : new StreamIterable(Files.lines(...)) {
    ....
}


Edit distance function Could you please help me to review my first function (modified from an example) in terms of best practice, coding style suggestions and any obvious language mistake/improvements?

unsigned int edit_distance(const vector<string> &s1, const vector<string> &s2)
{
    const size_t len1 = s1.size(), len2 = s2.size();
    vector<vector<unsigned int>> d(len1 + 1, vector<unsigned int>(len2 + 1));

    d[0][0] = 0;
    for(unsigned int i = 1; i <= len1; ++i) d[i][0] = i;
    for(unsigned int i = 1; i <= len2; ++i) d[0][i] = i;

    for(unsigned int i = 1; i <= len1; ++i)
            for(unsigned int j = 1; j <= len2; ++j)
            {
                unsigned int a = d[i - 1][j] + 1;
                unsigned int b = d[i][j - 1] + 1;
                unsigned int c = d[i - 1][j - 1] + (s1[i - 1] == s2[j - 1] ? 0 : 1);
                        d[i][j] = std::min( std::min(a,b), c);
            }
    return d[len1][len2];
}

Do you want unsigned int or do you want std::size_t?

unsigned int edit_distance(const vector<string> &s1, const vector<string> &s2)

It comes down to what you are trying to convey.
The use of string and vector suggests you are doing using namespace std; try not to do this. Prefer to qualify your types: std::string and std::vector.

Personally, I prefer const on the right. But it's only a preference thing.

Only declare one variable per line:

    const size_t len1 = s1.size(), len2 = s2.size();

Come up with better names. d is not very descriptive.

    vector<vector<unsigned int>> d(len1 + 1, vector<unsigned int>(len2 + 1));

I would prefer std::size_t here:

    for(unsigned int i = 1; i <= len1; ++i) d[i][0] = i;


First index of number in the arithmetic progression (which is multiple of prime) Things to deal with this problem.

  An infinite Arithmetic progression.
  A prime number. - p.
  The starting number of an Arithmetic Progression. - a.
  Common difference in the arithmetic progression given to him. - d.

  You have to print the first index
  of a number in that Arithmetic Progression, which is a multiple of the
  given prime number, p.

  Input format: The first line contains a number, tc, denoting the
  number of test cases. After that follow tc number of test cases, each is 2 lines - the first contains two integers, a and d -
  a depicts the first term in the AP, d depicts the common difference.
  The next line contains the prime number.

  Output format: You have to print the FIRST index (0-based) of the
  multiple of the given prime number in the given AP. If no such element
  exists in this infinite AP, then print -1.

  Constraints: 0 <= a, d, <= 10^18 1 <= p <= 10^9

My code:

public static void main(String[] args) throws NumberFormatException, IOException{
StringBuilder output = new StringBuilder();
        BufferedReader reader= new BufferedReader(new InputStreamReader(System.in));
            int noOfTestCaseT=Integer.parseInt(reader.readLine().trim());

            while (noOfTestCaseT != 0){
                noOfTestCaseT--;
                String[] inputAandD = reader.readLine().split(" ");
                long firstElement = Long.parseLong(inputAandD[0].trim());
                long commonDifference = Long.parseLong(inputAandD[1].trim());
                long primeNo = Long.parseLong(reader.readLine().trim());
                firstElement %= primeNo;
                commonDifference %= primeNo;
                int result = 0;
                if(commonDifference == 0) output.append(-1);
                else if (firstElement == 0) output.append(0);
                else{
                    long inverseMod =  getPowerValue(commonDifference, primeNo);
                    result = (int) ((inverseMod * (primeNo-firstElement)) % primeNo) ;
                    output.append(result);
                }
                output.append("\n");

            }

            System.out.println(output);
    }

    private static long getPowerValue(long base, long primeNo) {
        long exp = primeNo -2;
        long powerResult = 1;
        while (exp > 0){
            if ((exp & 1) == 1) powerResult = (powerResult * base) % primeNo;
            base = (base * base) % primeNo;
            exp = exp >> 1 ;
        }
        return powerResult;
    }

How could I can optimize this code further so that its performance improves (though the solution is within the given time limit and I really want to know what are the possible improvement I can make)?

Time elapsed is almost all in overhead
I'm not sure you can do any better than what you've done.  I tested your program and it ran extremely fast.  In fact, I determined that most of the time was probably being spent bringing up the JVM.  I ran your program with varying test inputs (up to 100000 test case input), and also ran your program without computing the result (just reading the input and outputting one number per test case).  The timings showed that most of the time was being spent just bringing up the JVM (or some other Java related overhead):
100000 test case (times are in seconds)
0.14: bringing up the JVM
0.19: parsing the input and generating output
0.12: calculating the answer
0.45: total time

100 test case
0.14: bringing up the JVM
0.02: parsing the input and generating output
0.01: calculating the answer
0.17: total time

So I believe that your actual algorithm runs for about 0.01 seconds out of 0.17 total time.
C program was faster (less overhead?)
I converted your program to C and tested it to see if it would run any faster.  It did run faster, probably because of the lack of overhead:
100000 test case (C program)
0.29: total time

100 test case (C program)
0.01: total time

Here is the C program, just to show you that it is exactly the same as your java program:
#include <stdio.h>

static long long getPowerValue(long long base, long long primeNo);

int main(void)
{
    int noOfTestCaseT = 0;
    long long firstElement, commonDifference, primeNo;

    scanf("%d", &noOfTestCaseT);

    while (noOfTestCaseT != 0){
        noOfTestCaseT--;
        scanf("%lld %lld %lld", &firstElement, &commonDifference, &primeNo);
        firstElement %= primeNo;
        commonDifference %= primeNo;
        if(commonDifference == 0) puts("-1\n");
        else if (firstElement == 0) puts("0\n");
        else {
            long long inverseMod = getPowerValue(commonDifference, primeNo);
            int result = (int) ((inverseMod * (primeNo-firstElement))%primeNo);
            printf("%d\n", result);
        }
    }
    return 0;
}

static long long getPowerValue(long long base, long long primeNo)
{
    long long exp = primeNo -2;
    long long powerResult = 1;
    while (exp > 0){
        if ((exp & 1) == 1) powerResult = (powerResult * base) % primeNo;
        base = (base * base) % primeNo;
        exp = exp >> 1 ;
    }
    return powerResult;
}

A slightly faster inverse mod function
From wikipedia, I copied this algorithm for finding the inverse mod.  Supposedly, it is slightly faster than the algorithm you are using.  Using the 100000 test case input, I found that it was a little bit faster, but not by much (maybe 10% faster).  Here is the function (it replaces your getPowerValue function):
private static long inverse(long a, long n)
{
    long t    = 0;
    long newt = 1;
    long r    = n;
    long newr = a;

    while (newr != 0) {
        long quotient = r / newr;
        long tmp;

        tmp  = newt;
        newt = t - quotient * newt;
        t    = tmp;

        tmp  = newr;
        newr = r - quotient * newr;
        r    = tmp;
    }
    if (t < 0)
        t += n;
    return t;
}


Optimal change with only with two euros coins and five and ten euros bills We assume we have unlimited number of coins and bills. My code works fine but the method optimalChange() has a Cyclomatic Complexity of 8.

import java.util.*;
import java.math.*;

class Change {
   long coin2 = 0;
   long bill5 = 0;
   long bill10 = 0;
}

class Solution {

  static Change optimalChange(long s) {
    long change = s;
    Change c = new Change();
    if (change >=10) {
        if (change % 2 != 0 && change % 5 !=0) {
            return dealWithChangeLike31Euros(change);
        }
        else {
            c.bill10 = (long) change / 10;
            change = change % c.bill10;
        }
    }

    if (change <10 && change >=5) {
        if (change % 2 == 0) {
            c.bill5 = 0;
            c.coin2 = change/2;
        }
        else {
            change = change - 5;
            c.bill5 = 1;
        }
    }

    else if (change %2 ==0) {
        c.bill5 = 0;
        c.coin2 = change/2;
    } else {
        return null;
    }
    return c;
  }

  static Change dealWithChangeLike31Euros(long s) {
    Change c = new Change();
    c.bill10 = ((long) s / 10) - 1;
    long change = (long) s % (c.bill10 * 10);
    if (change > 5) {
        change = change - 5;
        c.bill5 = 1;
    }
    if (change % 2 == 0) {
        c.coin2 = change / 2;
    }
    else {
        return null;
    }
    return c;
  }
}

Can the implementation of the dealWithChangeLike31Euros() method be merged into the other method?

The only way of making an odd total is with a five euro bill.  There is no point ever having more than 1 five euro bill, as you could replace pairs with a ten euro bill.  So, you should start off checking for the odd total.

if (change >= 5  &&  change % 2 == 1) {
    c.bill5 = 1;
    change -= 5;
}

At this point, you can simply compute the number of ten euro bills, and remaining 2 euro coins.

c.bill10 = change / 10;
change = change % 10;

c.coin2 = change / 2;

The only thing you have to watch out for is a total change amount of 1 or 3 euros (or negative), which is not possible to make.  Handle those as a special case.

Your optimalChange() creates a change object, and if dealWithChangeLike31Euros() methods is called, it creates its own Change object, and the first object is abandoned.  You shouldn't create the object if it isn't going to be used.  Options include passing the change object to the dealWithChangeLike31Euros() method, or delaying the creation.

Multiple return points is sometimes a code smell.  optimalChange() has 3:

return dealWithChangeLike31Euros(change);
return null;
return c;

If you initialized Change c = null;, and only created the Change object when needed, you could have just the one return c; at the end.  Less returns may reduce the complexity.


NS.log (ES5 logger) - v0 I don't want to extend the functionality of this method to include say library objects.  I'm just looking for feedback on what it currently does.

ES5, section 8.6.2 "exposes" these global objects.  I added a few Browser or "Host" objects which I needed for my purposes.

If needed I can add some more Browser or Host objects but no library objects as there is too much variety here. (i.e. jQuery, Backbone, etc.).

/*log
**  dependencies - none
**  browser - N/A
**
*/
$A.error = $A.report = $A.log = function (obj, note) {
    var logger,
        temp,
        type,
        ES5a = ['Arguments', 'Array', 'Object'],
        ES5b = ['Boolean', 'Date', 'Error', 'Function', 'JSON', 'Math',
            'Number', 'Null', 'RegExp', 'String', 'Undefined'],
        kindex,
        length;
    note = note || '';
    if (window.console) {
        logger = window.console.log.bind(window.console);
    } else {
        return;
    }
    type = Object.prototype.toString.call(obj).slice(8, -1);
    if (!type) {
        logger("Object type not found: ");
        logger(obj);
        return;
    }

    // Language Objects

    for (kindex in ES5a) {
        if (type === ES5a[kindex]) {
            try {
                temp = JSON.stringify(obj, null, 1);
            } catch (error) {
                temp = false;
            }
            if (temp) {
                logger("LOG|" + ES5a[kindex] + ">" + note + ">" + temp);
            } else {
                logger("LOG|" + ES5a[kindex] + ">" + note + ">");
                logger(obj);
            }
            return;
        }
    }
    for (kindex in ES5b) {
        if (type === ES5b[kindex]) {
            logger("LOG|" + ES5b[kindex] + ">" + note + ">" + obj);
            return;
        }
    }

    // Host Objects

    if (type === 'Event') {
        logger("LOG|B|Event>" + note + ">");
        if (obj.type) {
            logger("LOG|Event.type>> " + obj.type);
        }
        temp = obj.target || obj.srcElement;
        if (temp && temp.id) {
            logger("LOG|Event.target.id>> " + temp.id);
        }
        return;
    }
    if (type === 'HTMLDivElement') {
        logger("LOG|B|Div>" + note + ">");
        if (obj.id) {
            logger("LOG|Div.id>> " + obj.id);
        }
        return;
    }
    if (type === 'Storage') {
        for (kindex in obj) {
            if (obj.hasOwnProperty(kindex)) {
                logger("LOG|Storage>" + note + ">" + kindex + " | " + obj[kindex]);
            }
        }
        return;
    }
    if (type === 'HTMLCollection') {
        for (kindex = 0, length = obj.length; kindex < length; kindex++) {
            logger("LOG|HTMLCollection>" + note + ">" + kindex + " | " + obj[kindex]);
        }
        return;
    }

    // Library Objects

    if (win.jQuery && (obj instanceof win.jQuery)) {
        logger('LOG|jQuery object>');
        return;
    }

    // Not Found

    logger("LOG|B|NotFound>" + note + "> obj: " + obj + " | str: " + type);
};

JSON.stringify is a dangerous function. It fails, and thus your function too, on cyclic objects, on very deep or big objects, or simply on objects having some forbidden accessor deeply cached.

This object can't be logged by your function, for example :

var a = {}; a.b=a;

A related problem is that you only test the type of the external object. You don't detect the problem on this one :

var a = {parent:window};

What I would suggest is

either the addition of a try/catch (which may not be totally satisfying as some stringifications take time and memory before they fail),
to use or build another stringification function (see related SO question)
or simply to avoid the JSON format (of course it depends on your goal)


Proper action when a Java program fails I have a program that has to initialize a few big things (connect to a few databases, parse some XML) and without the initialization being successful the program would not be able to continue.
Right now I have my main method throwing just a general Exception

public static void main(String[] args) throws Exception
{
    //Throws numerous types of Exceptions
    WhateverObject we = WhateverObject.getInstance();

    we.doSomething();
}

My question is, is there a better way to handle this? Should I catch the exception and then print out that it failed an exit? Something else maybe? Note, there's no hope of program recovery at this point.

What would the users like? It's usually an error message and a log file with the details and stacktraces for bug reports/support calls/debugging. You can throw exceptions out from main but it's not user-friendly (unless the users are the people who develop the software).
Please note that singleton nowadays is rather an antipattern. (WhateverObject.getInstance() looks like a singleton.) They make testing harder and often hide dependencies which leads to spaghetti code which is really hard to work with.

What is so bad about singletons?
TotT: Using Dependency Injection to Avoid Singletons
Eliminating static helper classes
The Singleton Pattern


C++ implementation of Hackerrank's "Maximum Element in a Stack" I just attempted the Maximum Element Challenge on Hackerrank. This question has been posted before in swift Swift Hackerrank Maximum Element in a Stack but it has attracted low views/no answers. Moving forward, based on the requirements of finding the maximum element, I used a vector<int> as my underlying data structure as opposed to using Stack as I can't iterate through a stack. This exercise is meant to be a simple one but the time limit gets exceeded for larger input.

How do I improve the performance? Please feel free to criticise my approach as I'm new to C++ with 5 days of experience.

  You have an empty sequence, and you will be given \$N\$ queries. Each
  query is one of these three types:

  1 x  -Push the element x into the stack
  2    -Delete the element present at the top of the stack
  3    -Print the maximum element in the stack

  Input Format

  The first line of input contains an integer, \$N\$ . The next \$N\$
  lines each contain an above mentioned query. (It is guaranteed that
  each query is valid.)

  Constraints

  \$1 \leq N \leq 10^5 \$
  \$1 \leq x \leq 10^9 \$
  \$1 \leq \text{type} \leq 3 \$

  Output Format

  For each type \$3\$ query, print the maximum element in the stack on a
  new line.

  Sample Input

10
1 97
2
1 20
2
1 26
1 20
2
3
1 91
3

  Sample Output

26
91

#include <vector>
#include <iostream>
#include <algorithm>

//using namespace std;
bool compareValues(int i, int j) { return i<j; }

int main() {
    /* Enter your code here. Read input from STDIN. Print output to STDOUT */
    int N;
    int queryType;
    int element;
     std::vector<int> stackOfNumbers;

    std::cin >> N;
    for(int i =0; i < N ; i++){
        std::cin >> queryType;

        switch(queryType) {
      case 1 :
            std::cin >> element;
            stackOfNumbers.push_back(element);
         break;
      case 2 :
            stackOfNumbers.erase(stackOfNumbers.end()-1);
            break;
      case 3 :
           std::cout<< *std::max_element(std::begin(stackOfNumbers), std::end(stackOfNumbers),compareValues ) << std::endl;
         break;
      default :
        std::cout << "Invalid queryType" << std::endl;
   }

    }
    return 0;
}

I believe to finish this within the time limit, you need to complete each of the three operations with \$O(1)\$ time complexity.

There are a number of variations in how you do that, but nearly all of them involve keeping track of the maximum value on the stack as you build the stack, rather than searching through all the existing stack items each time you receive a query. Given that this is an online challenge, I hesitate to go into a lot more detail on this point than that.

As far as the code style goes, the most obvious point would be that your compareValues isn't really necessary--std::less<T> will be used by default, and your comparison adds nothing it doesn't already do perfectly well. In fact, std::less<T> may be superior in one respect: it's defined as a function object--a class that overloads operator() to do the comparison. You'r defining an actual function, and passing a pointer to a function. Passing a function object tends to make it easier for a compiler to generate inline code, so letting std::less<T> do the job may well improve speed at least slightly.

One more (somewhat less obvious) point is that in the process of improving efficiency to the point that you can finish within the time limit, I'm (reasonably) confident that you'll eliminate the need to traverse the stack, so you should be able to use std::stack<int>.


A Tiny Text File Read/Write class Implementation in C# I am trying to implement a tiny text file read / write class FileIO in order to make the operations of reading / writing pure text files easily. The usage is like:
FileIO.Instance.FileWrite("TextFile1.txt", "This is a test1.");
FileIO.Instance.FileWrite("TextFile2.txt", "This is a test2.");
Console.WriteLine(FileIO.Instance.ReadTxTFile("TextFile1.txt", System.Text.Encoding.Default));
Console.WriteLine(FileIO.Instance.ReadTxTFile("TextFile2.txt", System.Text.Encoding.Default));

The experimental implementation
The experimental implementation of FileIO class is as below.
class FileIO
{
    public readonly static FileIO Instance = new FileIO();

    public void FileWrite(string filename, string inputString)
    {
        FileStream file_stream = new FileStream(filename, FileMode.Append);
        byte[] Input_data = System.Text.Encoding.Default.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }

    public void FileWrite(string filename, string inputString, FileMode fileMode)
    {
        FileStream file_stream = new FileStream(filename, fileMode);
        byte[] Input_data = System.Text.Encoding.Default.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }

    public void FileWrite(string filename, string inputString, FileMode fileMode, Encoding encoding)
    {
        FileStream file_stream = new FileStream(filename, fileMode);
        byte[] Input_data = encoding.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }

    public string ReadTxTFile(string filename, Encoding encoding)
    {
        System.IO.StreamReader textreader;
        string inputString;
        inputString = "";
        try
        {
            textreader = new System.IO.StreamReader(filename, encoding);
        }
        catch (Exception)
        {
            return "";
            throw;
        }
        inputString = textreader.ReadToEnd();
        textreader.Close();
        return inputString;
    }
}

If there is any possible improvement about:

Performance: including data reading/writing speed things

The naming and readability

Potential drawbacks of the implemented methods

, please let me know.

Naming

FileIO: The File class resides inside the System.IO namespace. FileIO is quite a misfortune chose of name. TextFileWrapper, TextFileUtil or TextFileManager would be a more appropriate name for this in my opinion.
FileWrite: The general guidance is to start your methods with a verb. So, WriteFile would be a better name for this.
ReadTxtFile: This name is a bit misleading. txt can be considered as a file extension, so the caller may no specify the filename as you expect.
inputString: Please try to avoid Hungarian notation.

Please try to chase consistency (or symmetry).
Try to name your Read-Write method pair in the same fashion:

ReadTextFile, WriteTextFile
ReadFile, WriteFile
or simply just Read and Write

Stuttering:

Maybe it's just for me, but FileIO.Instance.FileWrite seems too repetitive.
TextFileManager.Write seems way more natural for me.

Robustness
Your implementation is quite naive. It does not deal with a lot of different cases:

What if the fileName points to a non-existing file?
What if the inputString is enormously large?
What if one of the parameters is null?
What if the file has been opened by another method and has not been closed yet?

Your implementation also relies on Encoding.Default, which is computer specific.
Which means you can end up in the following situation:

FileA.txt has been written to the disk on MachineA
FileA.txt has been copied from MachineA to MachineB

where the default encoding is different

MachineB could not read FileA.txt

Thread-safety

Your instance property does not make your class a singleton.

Please read thoroughly Jon Skeet's guidance.

If you make your class a singleton you have to make sure that your operations are thread-safe.

Reading and writing the same file from different threads are handled in a proper way.

As pointed out by others you have to do some clean-up as well.

To make sure that if something goes wrong then your file handles are closed properly.

Performance

Try to take advantage of async I/O
Try to take advantage of ArrayPool when you are calling the GetBytes method.

It has an overload which accepts a byte array

As always, measure, measure and measure.


Checking if a binary tree is balanced class BinaryTreeNode {

  private BinaryTreeNode left;
  private BinaryTreeNode right;
  private int data;

  BinaryTreeNode(int d) {
    data = d;
  }

  public void insertLeft(BinaryTreeNode n) {
    this.left = n;
  }

  public void insertRight(BinaryTreeNode n) {
    this.right = n;
  }

  public int height() {
    // height = Max(Hl, Hr) + 1
    int leftHeight = this.left != null ? left.height() : -1;
    int rightHeight = this.right != null ? right.height() : -1;
    return Math.max(leftHeight, rightHeight) + 1;
  }

  public String toString() {
    String leftStr = this.left == null ? "" : this.left.toString();
    String rightStr = this.right == null ? "" : this.right.toString();
    return leftStr + " : " + data + " : " + rightStr;
  }

  public static void main(String[] s) {
    BinaryTreeNode n  = new BinaryTreeNode(0);//             0         l0
                                              //            / \
    BinaryTreeNode l1 = new BinaryTreeNode(1);//           1   4       l1
                                              //          /
    BinaryTreeNode l2 = new BinaryTreeNode(2);//         2             l2
                                              //        /
    BinaryTreeNode l3 = new BinaryTreeNode(3);//       3               l3
    l2.insertLeft(l3);
    l1.insertLeft(l2);
    n.insertLeft(l1);
    n.insertRight(new BinaryTreeNode(4));
    System.out.println(n.height());
    System.out.println(isBalanced(n));
  }

  public static boolean isBalanced(BinaryTreeNode n) {
    // if height = (|hl - hr|) <=1
    int leftHeight = n.left != null ? n.left.height() : -1;
    int rightHeight = n.right != null ? n.right.height() : -1;
    return leftHeight - rightHeight <= 1;
  }
}

Your question is unclear, though I can tell that your code is broken.

Your code here:

  public static boolean isBalanced(BinaryTreeNode n) {
    // if height = (|hl - hr|) <=1
    int leftHeight = n.left != null ? n.left.height() : -1;
    int rightHeight = n.right != null ? n.right.height() : -1;
    return leftHeight - rightHeight <= 1;
  }

Should return true if the tree is balanced, but, it takes a node in, computes the height of the left and right sides, and compares the differences.

This method is broken.... Just because the height of each side of the tree (left, and right) are similar, does not mean that each side is also balanced. Consider a tree like:

    //             0
    //            / \
    //           1   4
    //          /   /
    //         2   5
    //        /   /
    //       3   6

When you test the root, you will determine that both sides have the same height, yet, the tree is far from balanced.


Project Euler, problem # 9, Pythagorean triplet Project Euler #9, Pythagorean triplet is

  A Pythagorean triplet is a set of three natural numbers \$a < b < c\$
  for which \$a^2 + b^2 = c^2\$.

  For example, \$3^2 + 4^2 = 9 + 16 = 25 = 5^2\$.

  There exists exactly one Pythagorean triplet for which \$a + b + c = 1000\$.   Find the product \$a b c\$.

Here is my implementation in Python, awaiting your feedback.

def get_triplet():
    for c in range(2, 1000):
        for b in range(2, c):
            a = 1000 - c - b
            if a ** 2 + b ** 2 == c ** 2:
                return 'n1 = %s\nn2 = ' \
                       '%s\nn3 = %s\nproduct = %s' \
                       % (a, b, c, a * b * c)

if __name__ == '__main__':
    print(get_triplet())

Search Space

Your search space is too large.  Since 0 < a < b < c and a + b + c = 1000, you can put hard limits on both a and c.  The maximum a can be is 332, since 333 + 334 + 335 > 1000.  Similarly, the minimum value c can be is 335, since 332 + 333 + 334 < 1000.  As well, the maximum value c can have is 997 (1 + 2 + 997 = 1000).

Based on the above, if you wish your outside loop to loop over c values, your first loop should be:

for c in range(335, 997 + 1):
    # ...

And the inner loop should be over a values:

    for a in range(1, 332 + 1):
        b = 1000 - c - a
        # ...

But now that we’ve selected a c value, can tighten the limits on a even further.  When c is 600, then a+b must be 400, and since a < b must hold, the maximum a can have is 400 / 2.  So:

    for a in range(1, min(332, (1000 - c) // 2) + 1):
        b = 1000 - c - a
        # ...

Moreover, since we’ve selected a c value, we have established a maximum value for b as well, since b < c must hold.  If c is 400, then b can’t be larger than 399, so a must be at least 1000 - 400 - 399 = 201, establishing a lower limit for our inner loop:

    for a in range(max(1, 1000 - c - (c - 1)), min(332, (1000 - c) // 2) + 1):
        b = 1000 - c - a
        # ...

Geometry

As pointed out in a comment by @Peter Taylor below, we can tighten the limits on c much further.

First, triangles must be triangular.  In every triangle, the length of any side must be less than the sum of the lengths of the other two sides.  In particular, \$c < a + b\$ must be true.  Since \$a + b + c = 1000\$, we can conclude \$c < 1000 - c\$, or more simply, \$c < 500\$, establishing a much smaller upper limit.

The smallest value c could obtain would be when a and b are at their largest.  If we relax the conditions slightly, allowing \$a \leq b\$ and \$a, b, c, \in \Re\$; we have an isosceles right-angle triangle, \$a = b = c / \sqrt 2\$.  Then:

\$a + b + c = 1000\$

\$c = 1000 \sqrt 2 / (2 + \sqrt 2) \approx 414.2\$

giving us a larger lower limit.

Thus, the range of the outer loop can be reduced to:

for c in range(413, 500):
    # ...

Repeated Calculations

When searching for the Pythagorean triplet, you are calculating c ** 2 each and every iteration of the inner loop.  The value of c is constant during execution of the inner loop, so c ** 2 is also constant.  Since exponentiation is a relatively expensive operation, you should get a speed improvement by moving this calculation out of the inner loop:

for c in range(413, 500):
    c2 = c ** 2
    for a in range(max(1, 1000 - c - (c - 1)), min(332, (1000 - c) // 2) + 1):
        b = 1000 - c - a
        if a ** 2 + b ** 2 == c2:
            return a, b, c

1000 - c is also repeated in the inner loop, so you could compute this outside the loop also; it should provide a slight performance improvement.

See other answers for important PEP8, f-string, docstring, and return value improvements not duplicated here.


Split the restaurant bill I'm trying to learn Javascript and I've set up a bill calculator. Most of the lines in the code are to deal with rounding. It has four main components:

Allows you to enter the bill amount and tip percentage.
Calculates the total bill divided by the number of guests.
Calculates the total tip.
Splits the bill by the number of guests.

billAmount = 134.93
const enterTipPercent = 10
const guests = ["Person1", "Person2", "Person3", "Person4"]
const noOfGuests = guests.length;
console.log('Chosen tip: ' + enterTipPercent + '%')

function splitBill() {
  var split = billAmount / noOfGuests;
  var rounding = split.toFixed(2);
  var elseIf = rounding * noOfGuests;

if (rounding == billAmount/noOfGuests) {
  console.log('Each person must pay: £' + rounding +' with no remainder');

}

else if (elseIf * noOfGuests > billAmount){
  var roundDown = Math.floor(split*100) /100;
  var remainingAmount = billAmount-roundDown*noOfGuests;
  var roundedRemainder = remainingAmount.toFixed(2);
  console.log('Each person must pay: £' + roundDown +' with £' + roundedRemainder + ' remaining');
  }
}

function calculateTip() {
  var tip = billAmount/100*enterTipPercent;
  var tipRound = tip.toFixed(2);
  return tipRound;
}

function splitTip() {

  if (calculateTip() * 10 == billAmount) {
    var tipShare = calculateTip();
    var tipDivide = tipShare/noOfGuests;
    var tipRound = tipDivide.toFixed(2);
    console.log('Each person must tip: £' + tipRound);
}
  else  {
  var tipShare = calculateTip();
  var tipDivide = tipShare/noOfGuests;
  var tipRound = Math.floor(tipDivide *100) /100;
  var tipremainder = calculateTip()-tipRound*noOfGuests;
  var tipRoundRemainder = tipremainder.toFixed(2);
  console.log('Each person must tip: £' + tipRound + ' with £' + tipRoundRemainder + ' remaining');
  }
}

splitBill();
console.log('total tip £' + calculateTip());
splitTip();const

The code I've written works fine, but I'm sure this isn't the best, or most efficient way I could have done it. Does anyone have any advice on how I can improve?

Short review;

Use a beautifier, the code is not properly indented or spaced out
You use var, try to use const and let
The code already assigned billAmount / noOfGuests to split, so you can compare rounding and split
£ should probably be a single string constant
elseIf is a terrible variable name
elseIf is already multiplied by noOfGuests, why would you multiply it again in the if statement?
billAmount and noOfGuests should be parameters of splitBill and splitTip
tipremainder should be tipRemainder


Review on code for duplicate removal I am trying to remove duplicates from a string without using additional buffers.
This seems to work correct, I think. Can this be improved? Should I be doing it differently?

private static String removeDuplicates(char[] chars) {

        if(chars == null)
            return null;

        int len = chars.length;
        for(int i = 0; i < len; i++){
            for(int j = i + 1; j < len;){
                if(chars[i] == chars[j]){
                    int temp = j;
                    while(temp < len - 1){
                        chars[temp] = chars[temp + 1];
                        temp++;
                    }
                    len--;
                }
                else
                    j++;
            }
        }

        return new String(chars, 0, len);
    }

A few stylistic comments :

I'd change the while loop for a for loop. It doesn't matter at all but I feel it better that way.
You could perform len--; before the while (or for) loop so that you can go til len instead of len - 1.

Then, as duplicates don't need to be close to each other (from what I understand), wouldn't it be easier to do something like (NOT TESTED AT ALL) :

private static String removeDuplicates(char[] chars) {
    if(chars == null)
        return null;
    int len = chars.length;
    int idx = 0;
    for (int i = 0; i < len; i++) {
        char c = chars[i];
        bool dup = false;
        for (int j = 0; j < idx; j++) {
            if (c == chars[j]) {
                dup = true;
                break;
            }
        }
        if (!dup) {
            chars[idx] = c;
            idx++;
        }
    }

    return new String(chars, 0, idx);
}

The point would be to go through the string and for each characters, try to see if we have seen it already. If we haven't we store it, otherwise, we don't.


Script to drop HTML part of multipart/mixed e-mails I have a working script that I wrote in Python which for e-mail messages that consist only of a plain text part + an HTML part, discards the HTML and keeps only the plain text part.

The script is not exactly elegant and, as can be seen from the code, it smells like it is C (in particular, I simulate the use of bitmasks) and I am not exactly satisfied with some points of it.

I know that the script has some issues (like code duplication and the hack mentioned above), but I don't know the idiomatic Pythonic way of writing it and I would appreciate any kind of criticism to improve it in any way to make the code elegant.

#!/usr/bin/env python
# -*- coding: utf-8 -*-

"""
Author: Rogério Theodoro de Brito <rbrito@ime.usp.br>
License: GPL-2+
Copyright: 2010-2012 Rogério Theodoro de Brito

drop-alternatives is a simple Python script for those who hate emails in
HTML and who prefer their inbox to have as many messages in pure text as
feasible. This script is generally meant to be run as a filter with procmail
or some other mail delivery agent.

It tries to be moderately conservative and only act when things are
moderately safe:

* If the message is `multipart` and has a `text/plain` and a `text/html`
  part, keep the `text/plain` part only.

* In all other cases keep the message intact.
"""

import email
import email.message

def compose_message(orig, body):
    """
    Create new message with headers from `orig` and body from `body`.

    * `orig`: The original message.
    * `body`: The body that we want the new message to have.
    * Returns a new message.

    `compose_message` creates a new message with most of the fields from
    `orig`, with fields from `body` (if any) and with the payload of
    `body`. The fields excluded from `orig` are the following:

    * `content-length`
    * `content-type`
    * `lines`
    * `status`
    """
    wanted = email.message.Message()
    wanted.set_payload(body.get_payload())

    unwanted_fields = ["content-length", "content-type", "lines", "status"]

    # The dictionaries `orig` and `body` have only headers as their items.
    for field in unwanted_fields:
        del orig[field]

    for k, v in orig.items():
        wanted[k] = v
    for k, v in body.items():
        wanted[k] = v

    return wanted

def sanitize(msg):
    """
    Given an RFC-2822 message `msg`, generate its 'sanitized' version.

    * `msg`: The message to be sanitized.
    * Returns a sanitized version of `msg`.

    `sanitize` tries to be moderately conservative and only act when things
    are moderately safe:

    * If the message is multipart and has a `text/plain` and a `text/html`
      part, keep the `text/plain` part only.

    * In all other cases keep the message intact.
    """
    if not msg.is_multipart():
        return msg

    # 'composition' is a bitmask containing the kind of the parts
    TEXTPLAIN = 1  # text/plain
    TEXTHTML = 2  # text/html
    MISCPARTS = 4  # anything else

    composition = 0
    text_taken = False

    for part in msg.walk():
        if (part.get_content_maintype() == "multipart" or
            part.get_content_type() == "message/external-body" or
            part.get_payload() == ""):
            continue
        elif part.get_content_type() == "text/plain":
            if not text_taken:
                text_taken = True
                body = part
                composition |= TEXTPLAIN
            else:
                # if we are seeing a second text/plain part, stop throwing
                # things
                composition |= MISCPARTS
                break
        elif part.get_content_type() == "text/html":
            composition |= TEXTHTML
        else:
            composition |= MISCPARTS

    if composition == (TEXTPLAIN + TEXTHTML) or composition == TEXTPLAIN:
        return compose_message(msg, body)
    else:
        return msg

if __name__ == "__main__":
    import sys
    res = sanitize(email.message_from_file(sys.stdin))
    print res.as_string(unixfrom=False),

I think the code is quite fine as it is. Some small notes:

if composition == (TEXTPLAIN + TEXTHTML) or composition == TEXTPLAIN:

Just to make sure: this code tests that TEXTPLAIN is set and that MISCPARTS is not set. I would make that explicit, otherwise this code is hard to understand (in particular, it’s easy to miss that MISCPARTS mustn’t be set):

if (composition & TEXTPLAIN) != 0 and (composition & MISCPARTS) == 0:

If you’re not happy with bit operations, you can a set instead.

composition = set()

# …

if TEXTPLAIN in composition and MISCPARTS not in composition:

… and define TEXTPLAIN etc. as simple consecutive constants rather than bitmasks.

This is more pythonic certainly, but I actually find the use of bit masks entirely appropriate here.


Finding all the prime factors of a number I want to calculate all the prime factors of a number in the range 2 ≤ N ≤ 107. Here is my function:

public static long cal_me(int a, long[] A){

     long ans=1;
     int count=0;
     while(a%2==0){
         a/=2;
       count++;
     }
     if(count!=0)  // I am calculating value which depend on Number of prime Number
     ans = (long) ((Math.pow(2,2*count+1)+1)/3);

     if(A[a]!=-1) return ans*A[a];

     for(int i=3;i<=Math.sqrt(a) && a!=1;i+=2)
     {
         if(A[a]!=-1){

             return ans*A[a];
         }
         count=0;
           while(a%i==0){
               a/=i;
               count++;

           }

           if(count!=0)
               ans *=(long) ((Math.pow(i,2*count+1)+1)/(i+1));

     }

     if(a>2)
        ans *=  (long) ((Math.pow(a,3)+1)/(a+1));

     return ans;
}

My calling function:

for(int i=2;i<=10000000;i++)
A[i] = cal_me(i,A);

The above program me took 10s. I want something faster — how should I improve this?

Quick observation: you use Math.pow() to calculate powers of two. This is highly inefficient, use bit shifting to prevent long -> double conversion

For example, 2^10 can be calculated very quicky as 1 << 10. See for example here (https://stackoverflow.com/q/141525/461499)

Also you divide a double by a long and then convert to long (in effect flooring), you can skip conversion to double entirely to speed things up.


Wrapping IndexDB with RxJs I find IndexedDB cumbersome and want to wrap the functionality with rxjs observables.  I've come up with an angular2 service that seems to fit the bill, but I'm wondering if I'm using the observables correctly or if there might be any subscriptions I'm missing (plnkr):

Usage:

this.localDbService.set('storeName', key, value)
  .subscribe(
    x => console.log('stored key ', x),
    err => console.error('error storing value', err)
  );

this.localDbService.get('storeName', key)
  .subscribe(
    x => console.log('got value', x),
    err => console.error('error getting value', err)
  );

The service opens the database in the constructor and provides it using a ReplaySubject.  This is the first thing I think might have a better solution.  I want some sort of observable that will provide me with a single value but only after initialization has finished, and provide the same value any time it is called.  I use a ReplaySubject(1) and use .take(1) on it, so any subscribers will not get a value until one is provided, and will only get a single value and won't have to unsubscribe.  Should I perhaps use an AsyncSubject?

Also I'm catching errors in storing and querying and passing any errors from db creation up to the caller, am I missing anything here?:

store(storeName: string, key: any, value: any): Observable<any> {
  return Observable.create((observer: Observer<any>) => {
    try {
      this.db.take(1).subscribe(db => {
        if (!db) {
          observer.error("IndexDB not supported!");
          return;
        }

        var txn = db.transaction([storeName], "readwrite");
        var store = txn.objectStore(storeName);
        var req = store.add(value, key);
        req.onerror = function(err) {
          observer.error(err);
          return;
        };
        req.onsuccess = function(e: any) {
          observer.next(e.target.result);
          observer.complete();
        };
      });
    } catch(err) {
      observer.error(err);
    }
  });
}

updated version of the code, based on @MarcoTerzolo 's answer

import { Injectable } from '@angular/core';

import { Observable, Observer, ReplaySubject, Subject } from 'rxjs';
import { take } from 'rxjs/operators';

const VERSION = 2;

// good tutorial: https://code.tutsplus.com/tutorials/working-with-indexeddb--net-34673
// reference: https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/Using_IndexedDB

@Injectable()
export class LocalDbService {
  db: Subject<IDBDatabase> = new ReplaySubject<IDBDatabase>(1);

  constructor() {
    window['SLOCALDB'] = this;

    if (!window.indexedDB) {
      this.db.next(undefined);
      this.db.complete();
    } else {
      console.log(`localdb - requesting open of 'swbuddy' version ${VERSION}`);

      const openRequest = indexedDB.open('swbuddy', VERSION);
      openRequest.onerror = err => {
        console.error('localdb - open has error:', err);

        this.db.error(err);
        this.db.complete();
      };

      openRequest.onupgradeneeded = function(e: any) {
        console.log('localdb - upgrade needed!');

        // create object stores for 'rawLogin' and 'rawVisit'
        const db: IDBDatabase = e.target.result;
        if (!db.objectStoreNames.contains('rawLogin')) {
          db.createObjectStore('rawLogin');
        }

        if (!db.objectStoreNames.contains('rawVisit')) {
          db.createObjectStore('rawVisit');
        }

        // for testing any type of object setup
        if (!db.objectStoreNames.contains('test')) {
          db.createObjectStore('test');
        }
      };

      openRequest.onsuccess = (e: any) => {
        console.log('localdb - open success!', e.target.result);

        const db: IDBDatabase = e.target.result;
        this.db.next(db);
      };
    }
  }

  get(storeName: string, key: any): Observable<any> {
    console.log('localdb.query()');
    return Observable.create((observer: Observer<any>) => {
      try {
        console.log('localdb.query() - subscribed!');
        this.db.pipe(
          take(1)
        ).subscribe(db => {
          console.log('localdb.query() got db:', db);
          if (!db) {
            observer.error('IndexDB not supported!');
            return;
          }

          const txn = db.transaction([storeName], 'readonly');
          const store = txn.objectStore(storeName);
          const req = store.get(key);
          req.onerror = function(e: any) {
            observer.error(e.target.error);
            return;
          };
          req.onsuccess = function(e: any) {
            observer.next(e.target.result);
            observer.complete();
          };
        });
      } catch (err) {
        observer.error(err);
      }
    });
  }

  put(storeName: string, key: any, value: any): Observable<any> {
    return Observable.create((observer: Observer<any>) => {
      try {
        this.db.pipe(
          take(1)
        ).subscribe(db => {
          if (!db) {
            observer.error('IndexDB not supported!');
            return;
          }

          const txn = db.transaction([storeName], 'readwrite');
          const store = txn.objectStore(storeName);
          const req = store.put(value, key);
          req.onerror = function(e: any) {
            console.log('store error event:', e);
            observer.error(e.target.error);
            return;
          };
          req.onsuccess = function(e: any) {
            console.log('store success:', e);
            observer.next(e.target.result);
            observer.complete();
          };
        });
      } catch (err) {
        observer.error(err);
      }
    });
  }

  delete(storeName: string, key: any): Observable<any> {
    console.log('localdb.delete()');
    return Observable.create((observer: Observer<any>) => {
      try {
        console.log('localdb.delete() - subscribed!');
        this.db.pipe(
          take(1)
        ).subscribe(db => {
          console.log('localdb.delete() got db:', db);
          if (!db) {
            observer.error('IndexDB not supported!');
            return;
          }

          const txn = db.transaction([storeName], 'readwrite');
          const store = txn.objectStore(storeName);
          const req = store.delete(key);
          req.onerror = function(e: any) {
            observer.error(e.target.error);
            return;
          };
          req.onsuccess = function(e: any) {
            observer.next(e.target.result);
            observer.complete();
          };
        });
      } catch (err) {
        observer.error(err);
      }
    });
  }

  query(storeName: string): Observable<any> {
    console.log('localdb.query()');
    return Observable.create((observer: Observer<any>) => {
      try {
        console.log('localdb.query() - subscribed!');
        this.db.pipe(
          take(1)
        ).subscribe(db => {
          console.log('localdb.query() got db:', db);
          if (!db) {
            observer.error('IndexDB not supported!');
            return;
          }

          const txn = db.transaction([storeName], 'readonly');
          const store = txn.objectStore(storeName);
          const req = store.openCursor();
          req.onerror = function(e: any) {
            observer.error(e.target.error);
            return;
          };
          req.onsuccess = function(e: any) {
            const cursor = e.target.result;
            if (cursor) {
              observer.next({key: cursor.key, value: cursor.value});
              cursor.continue();
            } else {
              observer.complete();
            }
          };
        });
      } catch (err) {
        observer.error(err);
      }
    });
  }
}


Am I on the right track with this random number array? Is this the best way of doing this? I wanted to make sure the grammar is correct when the array is output.

1 car, 2 cars.

The output is correct but is there an easier way generally? I just want to make sure I am doing this in a proper way. I don't have the benefit of the experience of the majority of you guys so I am asking for general guidance.
 Or is it a case of "if the code works then that's all that matters".

<?php
//generates a random number between 1 -10.
function randomnumber(){
    $randomnumber = mt_rand(1, 10);
    return $randomnumber;
}

//Creates array.
//Checks to see if the random number is greater than 1
//Adds an 's' to the entry if greater than one
for ($i = 0; $i < 10; $i++){
        $rannum = randomnumber();
        if ($rannum > 1){
            $adds = "s";
        }else{
            $adds = "";
        }
        $products[$i] = $rannum." car".$adds;
}

//Outputs the final array.
foreach ($products as $current){
    echo $current."<br />";
}
?>

Just a little detail, to kick off: you're not declaring $products as an array anywhere. PHP will create a new variable, and assign it an empty array for you, true enough, but if you change your ini settings to E_STRICT | E_ALL, you'll notice that it doesn't do this without complaining about it.
It's always better to declare and initialize your variables beforehand. writing $products = array(); isn't hard, nor is it very costly. At any rate it's less costly than producing a notice.

Anyway, about your actual code: There is room to make your code more efficient way, so let's look at it piecce by piece:

function randomnumber(){
    $randomnumber = mt_rand(1, 10);
    return $randomnumber;
}

This function is just calling mt_rand, and returning the result. But only after the return value of mt_rand has been assigned to a local variable. That's just wasteful. Look at it from a machine's standpoint:

randomnumber();//call <name:randomnumber>
  \\
   \==> lookup function in memory, goto, and start
   $randomnumber = mt_rand(1,10);
     \\                   \\
      \=> allocate memory  \=> lookup function, pass 2 arguments & call
        ||     /\                                                 //
        ||     ||=======then, assign return valueof: =============/
        ||
        \== return COPY of assigned value
            then flag $randomnumber for GC (Garbage Collection)

That's just a hell of a lot of work, for a simple call to mt_rand. You can easily omit the allocation and garbage collection steps by changing the randomnumber function to this:

function randomnumber()
{
    return mt_rand(1,10);
}

This does exactly the same thing, without the overhead of an assignment. Still, a function call that just calls another function is a bit silly. If anything, it's an alias to an existing function, with default params. But you're still returning a copy of the return value of the core function (mt_rand). Just ditch the function, and in-line mt_rand(1,10);
If you insist, keep the alias, but define default arguments:

function randomnumber($from = 1, $to = 10)
{
    $from = (int) $from;//make sure we're dealing with ints
    $to = (int) $to;
    if ($from > $to)
    {//check order of params
        $tmp = $from;
        $from = $to;
        $to = $tmp;
    }
    return mt_rand($from, $to);
}
randomnumber();//returns mt_rand(1,10)
randomnumber(10,20);//returns mt_rand(10,20)
randomnumber(200);//returns mt_rand(10, 200)
randomnumber(null, 5);//returns mt_rand(0,5)

But that's only of any use if you're going to use this alias function throughout, and don't want to bother passing those 2 arguments all the time, or (in case of passign variables) you're not sure what they might contain (null, 'a string', (float) '0.123',...).
But we're drifting off topic. Next, the array-constructing loop:

//Creates array.
//Checks to see if the random number is greater than 1
//Adds an 's' to the entry if greater than one
for ($i = 0; $i < 10; $i++){
        $rannum = randomnumber();
        if ($rannum > 1){
            $adds = "s";
        }else{
            $adds = "";
        }
        $products[$i] = $rannum." car".$adds;
}

This isn't too bad, really, just remove the function-call, and you're good, and perhaps shorten the if-else to a ternary or a single if-statement. you could argue that assigning the random number to a variable isn't really necessairy. You're using $i as an array-key, so you could assign the value to the array directly, and check the value there

for($i=0;$i<10;$i++)
{
    $products[$i] = mt_rand(1,10);//$i is the key, so we don't need any other vars
    $products[$i] .= 'car '.($products[$i] > 1 ? 's' : '');
}
//without ternary:
for($i=0;$i<10;$i++)
{
    $products[$i] = mt_rand(1,10).' car';//just concat car already
    if ($products[$i] > 1)
    {//comparing to int will cast $products[$i] to int
     //since the string starts with an int, it'll compare that int
     //'4 car' > 1 ~~> 4 > 1 ~~> true ==> '4 car' .= s ===> 4 cars
        $products[$i] .= 's';
    }
}

Then, the output loop:

//Outputs the final array.
foreach ($products as $current){
    echo $current."
";
}

Ok, that's perfectly valid code, too, but wouldn't it be a lot shorter just writing this:

echo implode('
', $products);

This turns an array into a string, and separates all values with a
. To add another break at the end (and/or beginning) of this string:

echo '
', implode('
', $products), '
';

I use this all the time if I want to dump a quick
 to the screen:

echo '

', implode('

', $array), '

';

So, your code ends up looking like:

for($i=0;$i<10;$i++)
{
    $products[$i] = mt_rand(1,10);//$i is the key, so we don't need any other vars
    $products[$i] .= 'car '.($products[$i] > 1 ? 's' : '')
}
echo implode('
', $products);

Or, if regexes don't scare you and you like some unmaintainable code, you could even do this:

$arr = array();
$i=0;
while($i++<10)
{
    $arr[] = mt_rand(1,10);
}
echo preg_replace('/(([2-9]+|10)\s+car)(?!s)/','$1s',implode(' car
',$arr).' car');

But that's just terrible code to maintain...

If the array you're echo-ing isn't of any use to you except for your printing it out, you could just as well drop the array:

for($i=0, $j=mt_rand(1,10);$i<10;$i++, $j=mt_rand(1,10))
{
    echo $j, ' car', $j > 1 ? 's' : '', '
';
}

Which, if you're a massochist, you can turn into a one-liner quite easily:

for($i=0, $j=mt_rand(1,10);$i<10;$i++, $j=mt_rand(1,10))  echo $j, ' car', $j > 1 ? 's' : '', '
';

As you can see, I'm using $i to count the number of iterations, while at the same time $j is (re-)assigned a new random value upon each iteration, too.
Using multiple variables in a for-loop construct is most commonly done to avoid calling count too much when for-looping a numerically indexed array:

for($i=0, $j = count($array); $i<$j;$i++)
{
    var_dump($array[$i]);
}

instead of:

for($i=0;$i<count($array);$i++)
{
    var_dump($array[$i]);
}

Both of the loops above behave in the same way (provided $array doesn't change inside the loop), but the first one is more performant, because, as you probably know, after each iteration the conditional expression (ie $i<$j or $i<count($array)) is evaluated. In the second case, this means the length of $array will be counted time and time again, whereas in the first case, this is only done once.


Path validation helper in a 'path input box' I have a helper extension method that I use for path string validation. It looks as below. It's purpose is to work with network paths as well, so in case a network resource is not available, I do not want my applications to freeze, hence the specified timeout etc.

public static bool IsExistingFilePath(this string source, int numberOfAttempts = 3, int timeout = 300)
{
    for (int i = 0; i < numberOfAttempts; i++)
    {

        var task = new Task<bool>(() => File.Exists(source));
        task.Start();

        bool exists = task.Wait(timeout) && task.Result;
        if (exists)
        {
            return true;
        }
        else
            continue;
    }
    return false;
}

I also have a generic text box control, where I want to validate the provided path - and indicate if path is not correct. I want this auto text box validation to happen as infrequently as possible, because I don't want to run this loop of File.Exists() tasks when the path is being provided at the moment (e.g. user typing) etc.

My current approach is that I have a property that is bound to textbox text, and whenever it is updated, the validation is performed (update source trigger = property changed, mode = two way).

However, there is a problem, because property setter is not accessed when I paste into the text box nor when I write in it. I had set up a bunch of event handlers (OnPaste and LostFocus), but it's becoming unmanageable and appears not correct - and also appears to fire more times then necessary.

Then I thought about data validation from XAML with a ValidationRule class, but that actually works more or less like with KeyUp.

What would be the correct way to perform validation only when needed?

When text is pasted into box
When property is changed for whatever reason
When textbox is cleared
When text is changed (for instance, a proper path is pasted and a user decides to change it manually, e.g. use different drive)

Start a Timer.

Whenever the text box changes, the user presses a key, the TextChanged event occurs, etc, start the timer. If it changes again, reset it. Create some timeout that, after which, the file validation would occur. (200ms would be plenty)

This has the obvious benefit of reducing the number of IO requests you do, and it helps keep responsiveness up. You can also make it asynchronous, and only hold the UI when doing actual updating.

As far as your code goes...

I would add braces in your else condition. I used to omit braces whenever possible, but I encountered more than one bug as a result, and it's just as easy to add them.

There's really not much else to comment on; there's not a lot going on here and it's quite clean to begin with.


Find minimal number of days to meet all your friends I was asked this in an interview yesterday:

You get an array of segments (segment contains 2 numbers, representing days count (say in a month, but could be in a year, etc.): start, end). You need to find the minimum number of days where you can meet as many of your friends.

For example, if you get [2, 5], [4, 8], [6, 10] - then one possible answer would be  4 and 6. If you get [1,10], [2,9], [3,7] one possible answer would be 5, as in day 5 everyone is available and you can meet them all.

My way of solving was to order the segments by start, and then look for the biggest intersection. Though I was told this is not the most optimal solution. So  I wonder - what is the optimal solution (i.e. more efficient)?

Here's my code:

def optimal_dates(segments):
    # sort the segments
    sorted_segments = sorted(segments_list, key=lambda var: var[0])

    # perform actual algorithm
    final_intersections = set()         # stores the final return result
    working_intersection = set()        # stores the working accumulative intersections
    for i in range(len(sorted_segments) - 1):
        seg_b = set(range(sorted_segments[i + 1][0], sorted_segments[i + 1][1] + 1))
        if not working_intersection:
            seg_a = set(range(sorted_segments[i][0], sorted_segments[i][1] + 1))
            working_intersection = seg_a.intersection(seg_b)
            # if empty, seg_a doesn't intersects forward, so add any element from it
            if not working_intersection:
                final_intersections.add(seg_a.pop())
        else:
            temp = working_intersection.intersection(seg_b)
            # if empty, this was end of intersection, so add any element from the previous accumulative intersections
            if not temp:
                final_intersections.add(working_intersection.pop())
            working_intersection = temp

    # add the final element
    if working_intersection:
        final_intersections.add(working_intersection.pop())
    else:
        final_intersections.add(sorted_segments[-1][0])

    return final_intersections

Making the set out of an interval is an immediate red flag. set is a heavyweight data structure, and making it is a heavyweight operation. It is also much easier to compute an intersection of two intervals, than of two sets.

Making the same set twice is another red flag.

All that said, it seems that you really overthink the problem. It admits a much simpler solution. Consider the friend whose window of opportunity closes the earliest. You want to meet him anyway, so make the most of it: select every friend whose window opens prior to it, schedule a party, and discard all the participants from further consideration; rinse and repeat.

Hint: do not sort the list of intervals. Sort the list of events (interval openings and closings).

Two things I intentionally don't want to spell out:

Prove that this algorithm does produce an optimal solution, and
How to efficiently discard the friends you've already met.


Find the probability of picking a prime number in a range The task is:

From a sub-sequence of integers between 2 inputted numbers A and B, we need to find the probability of picking a prime number. I have written something to accomplish the task-

import java.util.*;
class Sample{
    public static void main(String[] args){
        Scanner sc = new Scanner(System.in);
        int cases = sc.nextInt();
        for(int i = 0; i<cases; i++){
            int l = sc.nextInt();
            int r = sc.nextInt();

            double ans = 0.0;
            for(int j = l; j<=r;j++){
                if(isPrime(j)){
                    ans++;
                }
            }
            System.out.println(String.format("output: %.6f",ans/(r-l+1)));

        }

    }
    public static boolean isPrime(int n){
        boolean ans = true;
        if(n == 1){
            ans = false;
        }
        if(n>5&& n%5==0){
            ans = false;
        }
        if(n>2&&n%2 == 0){
            ans = false;
        }
        else{
        for(int i = 3; i<(Math.sqrt(n)+1);i=i+2){
            if(n%i == 0){
                ans = false;
                break;
            }
        }}
        return ans;
    }
}

We also input an integer (cases) to represent the number of test cases.
This gives a correct answer on my machine, but gets a time limit exceeded verdict when I upload it. I thought that maybe using c++ would solve the issue.

So I made a similar program in c++, but still it does not match the time limit of 1 second. The C++ code is:

#include <iostream>
#include <math.h>
using namespace std;
bool isPrime(int n){
    bool ans= true;
    if(n == 1){
        ans = false;
    }
    if(n > 2 && n%2 == 0){
        ans = false;
    }
    for (int i = 3; i < (pow(n,0.5)+1); ++i)
    {
        if(n%i == 0){
            ans = false;
            break;
        }
    }
    return ans;
}
int main(){
    int cases;
    cin>> cases;
    for(int i = 0;i<cases;i++){
        int a, b , ans = 0;
        cin>> a,b;
        for (int j = a; j <= b; ++j)
        {
            if(isPrime(j)){
                ans ++;
            }
        }
        double d = ans/(r-l+1);
        cout<<d;
    }

}

Constraints:

1 ≤ Cases ≤ 100000

1 ≤ A≤ B ≤ 1000000

How can I optimize it further?

What I researched -

I should check if a i divides n only up till Math.sqrt(n)+1. Also I should do i = i+2 as I have checked for even numbers before only. I should also break the loop once ans becomes false so as to stop any further unnecessary iterations.

for(int i = 3; i<(Math.sqrt(n)+1);i=i+2){
        if(n%i == 0){
            ans = false;
            break;
        }
    }

Here is the link to the question: codechef.

Your question contains 2 pieces of code which makes it hard to review.

Issue specific to the C++ code

Your are using ++i in your code despite having handled the even numbers explicitely before. It should be i+=2 like in your Java code.

Issue in both implementations

Instead of maintaining an ans boolean value, you can simply return false whenever you know that the number can't be prime.

You write your loop like this for(int i = 3; i<(Math.sqrt(n)+1);i=i+2) which is likely to compute the square root at each iteration. The compiler may be able to optimize this out but just in case, it is probably better to store it in a variable beforehand.

Different algorithm

To know whether many numbers are primes, a convenient and efficient algorithm is to use The Sieve of Eratosthenes to build a table to check primes.

You could build it one and for all to get the primes up to the maximum r value.

Going further, you could also build (once) a table giving for each number n the number of primes numbers smaller or equal to n. Then, for each input (a, b), the frequency is something along the lines of ( primes_up_to(b) - primes_up_to(a) / (b - a).


Append the contents of latest Stackage nightly build to the global cabal config file I've made the following python script to to append the contents of the latest Stackage nightly build to the global cabal config file (instead of going to the site, copying the page and appending it manually...). I would appreciate your feedback:

"""
Appends the latest stackage nightly sources from Stackage(http://www.stackage.org/nightly)
to your global cabal config file

It also makes two backups: one of the appended file and one of the unappended file

Stackage(http://www.stackage.org/) is a stable source of Haskell packages
"""

import requests
import shutil
from os.path import expanduser
from os.path import join

cabal_config_path = expanduser('~/.cabal/config')
stackage_nightly_url = 'https://www.stackage.org/nightly/cabal.config?global=true'

def write_cabal_config_backup(filename):
    """
    Writes a backup of the current global cabal config file in the ~/.cabal directory
    """
    shutil.copyfile(cabal_config_path,
        join(expanduser('~/.cabal'), filename))

def unappend_stackage():
    """
    Unappend stackage sources from the global cabal config file. Be careful that
    the sources must be at the end of the file, or this function will delete things
    that you don't want it to.
    """
    def unappended_cabal_config():
        # Searches for the string 'Stackage' and returns a list made of the lines
        # of the file from there up, excluding the 'Stackage' line and everything below.
        with open(cabal_config_path) as f:
            cabal_config = f.readlines()
            for i in range(len(cabal_config)):
                if 'Stackage' in cabal_config[i]:
                    return cabal_config[:i]
        return cabal_config
    def write_unappended_cabal_config():
        cabal_config = unappended_cabal_config()
        with open(cabal_config_path, 'wt') as f:
            for line in cabal_config:
                f.write(line)
    write_unappended_cabal_config()

def append_stackage_nightly():
    """
    Appends stackage nightly sources to the global cabal config file
    """
    def get_stackage_nightly():
        r = requests.get(stackage_nightly_url)
        if r.status_code == 200:
            return r.text
    stackage_nightly = get_stackage_nightly()
    if stackage_nightly:
        with open(cabal_config_path, 'a') as f:
            f.write(stackage_nightly)

if __name__ == '__main__':
    write_cabal_config_backup('config_backup_appended')
    unappend_stackage()
    write_cabal_config_backup('config_backup_unappended')
    append_stackage_nightly()

The unappend_cabal_config can be more efficient and more elegant using a generator:

Reading all the lines from the file when you may only need the ones until one containing "Stackage" is wasteful
It's good to avoid the index variable in loops when possible

Like this:

def unappend_cabal_config():
    with open(cabal_config_path) as fh:
        for line in fh:
            if 'Stackage' in line:
                return
            yield line


Approximating Pi with Polygons I wrote a program that approximates Pi by using polygons. I used the formulars in the picture beyond.
In my code they are called innerPoly (\$c_{2n}\$) and outerPoly (\$C_{2n}\$).

And since you can calculate the circumference of a 2n-polygon with knowing the circumference of a n-polygon you will get the circumferences \$C_8\$, \$C_{16}\$, \$C_{32}\$ etc., knowing \$C_4\$.

$$
\begin{array}{l}
c_{2n} =& 2 \sqrt{2n^2-n\sqrt{(2n)^2-c_n^2}} \qquad&\textrm{for the inner polygon, with}\ c_4=4\sqrt{2} \\
C_{2n} =& \frac{4 n C_n}{2n + \sqrt{(2n)^2 + C_n^2}} &\textrm{for the outer polygon, with}\ C_4=8
\end{array}
$$

My thoughts are:

Would it make it anyhow better when I have a function called void PiApproximation() that writes my values to stdout already? I mean technically you can always put your full code into the main()-function, but when you have to use the same code-parts over and over again you should make a own function of it and call it, when u need it.
So I guess in  this case it will make no difference if I use an own function or calculate the circumferences and print it in the main-function.
What else can I improve?

pi_approx.c

#include <stdio.h>
#include <math.h>

#define INNER_FOUR 4*sqrt(2); //circumference c_4 of the inner tetragon(square)
#define OUTER_FOUR 8 //circumference C_4 of the outer tetragon

int power(int n, int p);

int main(void)
{
  int n = 4;
  double innerPoly = INNER_FOUR;
  double outerPoly = OUTER_FOUR;

  printf("PI-APPROXIMATION USING POLYGONS\n");
  printf("===============================\n\n");
  printf("    n    I      c_n/2      I       C_n/2     I\n");
  printf("---------I-----------------I-----------------I\n");

  for (int i=3; n<=8192; n=power(i,2), i++)
  {
    printf("   %4d  I   %1.8lf    I    %1.8lf   I\n", n, innerPoly / 2, outerPoly / 2);

    innerPoly = 2 * sqrt(2 * n*n - n*sqrt(4 * n*n - innerPoly*innerPoly)); //formular c_2n
    outerPoly = (4 * n * outerPoly) / (2 * n + sqrt(4 * n*n + outerPoly*outerPoly)); //formular C_2n
  }

  return 0;
}

int power(int n, int p)
{

  int pBuffer = 1;

  for (int i=1; i <= n; i++)
  {
    pBuffer *= p;
  }

  return pBuffer;
}

You asked,

  Would it make it better if I had a function called PiApproximation?

The code in main does just one thing so it would not be an improvement to introduce a new function like this — the question would then be, what's the point of main?
There is a dependency between the initial value of n and the initial values of innerPoly and outerPoly. It would make sense to put all the initialization code together.
Because n can never be negative it could be unsigned.
The loop:

for (int i=3; n<=8192; n=power(i,2), i++)

is quite hard to follow because the loop variable is i but the termination condition is on a different variable n. Because the loop variable i is not used, it would be easier to follow the logic if you wrote:

for (; n <= 8192; n *= 2)

(This also avoids the need for the power function.)
The l modifier has no effect on the printf format specifier %f, and so should be omitted.


Validate Move Pattern Chess A while ago, I was working on a chess project with a couple other people.  In the end, I wrote all but a couple of the lines of code, including these:

class Piece(list):

    def IsValidMovePattern(self, FromCoord, ToCoord):
        print(self, "Validation not done")
        return False

class King(Piece):

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [abs(ToCoord[0] - FromCoord[0]), abs(ToCoord[1] - FromCoord[1])]

        # One square in any direction
        if move[0] <= 1 and move[1] <= 1:
            return True

        # Castling - AND has higher priority than OR
        if move[0] == 0 and move[1] == 2:
            if self[1] == Player(1) and FromCoord == (0, 4) or self[1] == Player(2) and FromCoord == (7, 4):
                return True

        return False

class Queen(Piece):

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [abs(ToCoord[0] - FromCoord[0]), abs(ToCoord[1] - FromCoord[1])]

        # Diagonal and straight moves
        if move[0] == move[1] or move[0] == 0 or move[1] == 0:
            return True

        return False

class Rook(Piece):

    HasMoved = False

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [ToCoord[0] - FromCoord[0], ToCoord[1] - FromCoord[1]]

        if move[0] == 0 or move[1] == 0:
            return True

        return False

class Knight(Piece):

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [abs(ToCoord[0] - FromCoord[0]), abs(ToCoord[1] - FromCoord[1])]

        # L-shaped moves
        if move[0] == 1 and move[1] == 2 or move[0] == 2 and move[1] == 1:
            return True

        return False

class Bishop(Piece):

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [abs(ToCoord[0] - FromCoord[0]), abs(ToCoord[1] - FromCoord[1])]

        # Can only move diagonal
        if move[0] == move[1]:
            return True

        return False

class Pawn(Piece):

    # Set to True when a pawn moves two squares past
    # Pawn must be on square 3 for PlayerOne or square 4 for PlayerTwo
    EnPassant = False

    def IsValidMovePattern(self, FromCoord, ToCoord):

        move = [ToCoord[0] - FromCoord[0], ToCoord[1] - FromCoord[1]]

        # Player(1) capture
        if self[1] == Player(1) and abs(move[1]) == move[0] == 1:
            return True

        # Player(2) capture
        if self[1] == Player(2) and abs(move[1]) == 1 and move[0] == -1:
            return True

        # Player(1) first move
        if self[1] == Player(1) and FromCoord[0] == 1 and move[1] == 0 and move[0] == 2:
            return True

        # Player(2) first move
        if self[1] == Player(2) and FromCoord[0] == 6 and move[1] == 0 and move[0] == -2:
            return True

        # Player(1) move
        if self[1] == Player(1) and move[1] == 0 and move[0] == 1:
            return True

        # Player(2) move
        if self[1] == Player(2) and move[1] == 0 and move[0] == -1:
            return True

        return False

This is called like this (where self is similar to this in C#; it is being called from the Board class):

if not self.data[FromCoord[0]][FromCoord[1]].IsValidMovePattern(FromCoord, ToCoord):

FromCoord and ToCoord are tuples (similar to immutable arrays) like (FromX, FromY).

Is this a good implementation?  Would this be a slow implementation, or is it reasonable?  The rest of the code (with Checkmate and the AI incomplete) can be found at Github.

Don't extend list when your class isn't actually a list. It makes every instance that much more complicated when there is no reason to. What does it mean when you call .insert() on a Piece? It breaks all of your code and should not be possible in the first place.

Note: I wrote the following section because I thought you were using the base class to represent the coordinate of the specific piece. After being confused at why you where comparing the y coordinate to Player(1), I had to look at your linked code. This showed that you were doing something entirely different. The fact that I got that far with a completely wrong understanding of how your code works is a huge indication that the design is bad.

A list is good for doing list type things. Don't use it as solution to everything and treat it like a bag you can do whatever you want with. There are many different core data structures that do different things. You should learn how they work and when to use them.

I'm going to leave the following section as it still has valid points about giving things names and how classes should be structured.

What you want is a Coordinate structure that represent a location on the board. A Coordinate has an x value and a y value for the corresponding axises. Using a collection where the index is significant hides information and requires readers to know that x comes before y. If you don't want to write the small amount of code it takes to create the fields, you can use namedtuple to generate the class for you. I would do this for the tuples you are getting passed in as arguments, as well. from_coord.x is much clearer than from_coord[0].

Taking this a step forward into object oriented design. A chess piece is not a Coordinate. A chess piece has a location on the board, which is a Coordinate. Based on this, it might makes sense for Piece to have a coord instance variable.

move is a bad variable name. By the name, it is not clear what it represents. When you look at the code, you see that is change in respective coordinate values between the from coordinate and the to coordinate. Since you are using lists for both your coordinates and these move delta values, things become harder to follow as a specific index is related, but not the same thing. At first glance, it looks like move is just a coordinate.

Using Player(1) to access the enum value (code that should have been included in the question directly, not linked to) can be confusing. At first glance it looks like you are creating an instance of a class. Instead Player.One looks like you are referencing a constant. Accessing the value programmatically is supported for:

  situations where Color.red won't do because the exact color is not known at program-writing time. Source

This is not that situation. You know exactly which value you wan to be accessing.

# Castling - AND has higher priority than OR
if move[0] == 0 and move[1] == 2:
    if self[1] == Player(1) and FromCoord == (0, 4) or self[1] == Player(2) and FromCoord == (7, 4):
        return True

You can use parenthesis to group boolean expressions in order to ensure the indented order of operations. Doing this would mean you wouldn't need a nested if clause.

Even though you are familiar with C# conventions, when you are working in Python, you should use Python conventions. In Python, method and variable names are in snake_case, not CapWords. Class names do stay as CapWords. (PEP8: Naming Convetions)


Simple Go REST API that returns dummy messages I am a learner in Go and wrote example below in a day so I would like to get some suggestions to improve it. It is a simple rest api that currently just accepts request and returns a dummy message. My main aim here is to get comments and suggestions on how I structured the flow/logic. Although it currently works fine and easy to follow (at least to me), could it be done in a cleaner/scalable way?

cmd/server.go

package main

import (
    "fmt"
    "football-api/internal/app/route"
    "log"
    "net/http"
)

type server struct {}

func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    response, err := route.Handler(r)
    if err != nil {
        w.WriteHeader(http.StatusNotFound)
    } else {
        w.WriteHeader(http.StatusOK)
    }

    w.Write([]byte(response))
}

func main() {
    fmt.Println("Server is running on port 8080 ...")

    err := http.ListenAndServe(":8080", &server{})
    if err != nil {
        log.Fatalf("Couldn't start server on :8080. [%s]", err.Error())
    }
}

internal/app/route/handler.go

package route

import (
    "errors"
    "net/http"
    "path/filepath"
)

func Handler(r *http.Request) (response string, err error) {
    switch filepath.Clean(r.URL.Path) {
    case "/":
        return Home(r), nil
    case "/comments":
        return Comments(r), nil
    default:
        return "", errors.New("route was not found")
    }
}

internal/app/route/home.go

package route

import "net/http"

func Home(r *http.Request) string {
    return "Home"
}

internal/app/route/comments.go

package route

import "net/http"

func Comments(r *http.Request) string {
    return "Comments"
}

There's not so much code that it would be hard to read, looks clean enough to me.

Regarding scalable ... in what way?  The switch in Handler will be the limiting factor since adding another route will always have to have a corresponding edit in that function.  Some frameworks will have ways around that (for better or worse), but as long as there are just a handful of routes I doubt it's a real issue (though if this is more of an exercise, go for it: if you have a hundred handlers, how could you register them instead of having a single point like here?).

In ServeHTTP there'll definitely be an issue with the need for additional, more nuanced error codes.  I'm not suggesting a particular solution, but I can see that happening quite quickly ... plus, 404 for an internal error looks wrong too, that should be 500 most likely, only if the route couldn't be found 404 would be appropriate.

Okay two more, returning a string instead of directly writing output will be a scalability problem, it'd be much better to avoid additional memory allocations to construct that string and simply write to the output buffer / writer directly.

And, the result of a errors.New can be stored in a global variable as long as the message doesn't change - one more memory allocation gone.


Ashton and String Hackerrank Online coding challenge Hacker Rank.

I tried to solve it using the naive appraoch first but its failing on some of the inputs and rest its getting timed out. How to get started in problem like these so that I could reduce the time complexity involved?

public class Solution {
    private static Set<String> substrings(String text) {
        int length = text.length();
        Set<String> set = new TreeSet<>();

        for (int i = 0; i < length; i++) {
            for (int j = i + 1; j <= length; j++) {
                set.add(text.substring(i, j));
            }
        }
        return set;
    }

    public static void main(String[] args) {
        Scanner scanner =  new Scanner(System.in);
        int T = scanner.nextInt();
        String s = scanner.next();
        int K = scanner.nextInt();
        StringBuilder sb = new StringBuilder();
        for (String sub : substrings(s)) {
            sb.append(sub);
        }
        System.out.println(sb.charAt(K - 1));
    }
}

###Program only handles one testcase###
Your program reads in T but doesn't iterate T times, so it only handles the first test case.  This could be the cause of the "wrong result" error.
###Memory limit exceeded###
Your program solves the problem correctly, but it uses \$O(n^3)\$ memory, so on large inputs it will run out of memory (possibly giving you a wrong answer instead of a timeout).  For a string of size 100000 (which is the maximum according to the problem description), the total length of all substrings is:  166671666700000, or \$1.7 * 10^{14}\$, which would require 170 terabytes of memory.
###Solution using: Suffix array + LCP array###
In the problem description, you will notice on the "Need Help?" section on the right hand side that there are links to two topics:

Suffix array
LCP array

You probably will want to read those links later, because they will give useful information on how to generate the two arrays in the most efficient way possible.  But for now, I will explain how, if you have already generated the suffix array and LCP array, you can solve the rest of the problem in \$O(n)\$ time and no additional space other than the \$O(n)\$ space used for the two arrays.
###Definitions###
First, some quick definitions of my own (if you haven't read through those above links yet):

The suffix array for a string of length \$n\$ is just a sorted array of the \$n\$ suffixes of the string.  To save space, the array contains integers instead of strings, where the integer is the index of the starting character of the suffix in the original string.  For example, if suffix[3] = 5, that is equivalent to suffix[3] = original_string.substring(5).

The LCP array holds the length of the longest common prefix between two successive strings of the suffix array.  For example, if suffix[5] = "abcd" and suffix[6] = "abyz", then LCP[6] = 2 because the two strings have a common prefix of length 2.

###Generating substrings in order###
The key to finding the kth character in the concatenated substrings is that you can generate all substrings in lexicographical order by iterating across the suffix array.  Let's take an example of the string "abcd".  The suffix array for this string is:
suffix[0] = "abcd"
suffix[1] = "bcd"
suffix[2] = "cd"
suffix[3] = "d"

If you take each possible prefix string of each possible suffix string, you will have generated all possible substrings of the original string.  For example, if you take suffix[0] = "abcd", there are four possible prefix strings:
"a", "ab", "abc", "abcd"

If you take suffix[1] = "bcd", there are three possible prefix strings:
"b", "bc", "bcd"

Now, if you notice, the order in which I just generated the substrings are the first 7 substrings of the original string in correct lexicographical order.  If you continue through the suffix array like this, all of the strings you generate will be in the order that you are supposed to concatenate to the final string (except for duplicate substrings, which I will talk about later).
You will also notice that for a particular length suffix, it is easy to compute how long all of its substrings will be.  For example, if the length of the suffix is 4 (such as "abcd"), then the length of the substrings it generates is 1 + 2 + 3 + 4.  In general, if a suffix has length n, then the length of the substrings it generates is n * (n+1) / 2.
###Solving the problem (no duplicate substrings)###
This gives us the following code to find the kth character, assuming that there are no duplicate substrings.  For each suffix in the suffix array, we find out if K falls within the substrings for that suffix.  If it does, we find out which substring it is and print out the required character.  If it does not, we quickly move on to the next suffix.
    int     n         = s.length();
    Integer [] suffix = GenerateSuffixArray(s);

    for (int i=0;i<n;i++) {
        long len   = n - suffix[i];
        long count = len * (len + 1) / 2;

        // If K is not found within the substrings generated by this
        // suffix, then go on to the next suffix.
        if (K > count) {
            K -= count;
            continue;
        }

        // K must be found within this suffix.  Now iterate through its
        // substring to find the exact substring and character.
        for (long j=1;j<=len;j++) {
            if (K <= j) {
                System.out.println(s.charAt((int) (suffix[i] + j - 1)));
                break;
            }
            K -= j;
        }
        break;
    }

###Handling duplicate substrings###
Duplicate substrings are handled by skipping past them.  For example, suppose you had two suffixes in your suffix array like this:
suffix[0] = "abcd"
suffix[1] = "abxyabcd"

Notice that the substrings generated would be:
"a", "ab", "abc", "abcd"
"a", "ab", "abx", "abxy", "abxya", "abxyab", "abxyabc", "abxyabcd"

Because there is a common prefix of length 2, the first 2 substrings generated are the same.  So when we handle the second suffix, we should skip those because they were already handled previously.
###Sample solution###
Here is a sample solution to the problem, that can get the correct answer for a string of 100000 length, but not in the most time efficient manner.  The reason is that I generated the suffix array and the LCP array in the simplest way possible, requiring \$O(n^2)\$ time.  To really solve the problem, you should read those links above and generate the suffix array and LCP array in a more efficient manner.  I'll leave that as an exercise to the reader.
import java.util.*;

public class Solution {

    public static Integer[] GenerateSuffixArray(final String s)
    {
        int        len = s.length();
        Integer [] ret = new Integer[len];

        for (int i=0; i < len; i++) {
            ret[i] = i;
        }
        Arrays.sort(ret, new Comparator<Integer>() {
            public int compare(Integer i1, Integer i2) {
                return s.substring(i1).compareTo(s.substring(i2));
            }
        });
        return ret;
    }

    public static int[] GenerateLCP(String s, Integer [] suffix)
    {
        int    len     = s.length();
        int [] ret     = new int[len];
        String prevStr = s.substring(suffix[0]);

        ret[0] = 0;
        for (int i=1; i < len; i++) {
            String str  = s.substring(suffix[i]);
            int    j    = 0;
            int    jMax = Math.min(prevStr.length(), str.length());

            // Find the longest common prefix of prevStr and str.
            for (j=0; j < jMax; j++) {
                if (prevStr.charAt(j) != str.charAt(j))
                    break;
            }
            ret[i] = j;
            prevStr = str;
        }
        return ret;
    }

    public static void main(String[] args) {
        Scanner scanner =  new Scanner(System.in);
        int T = scanner.nextInt();

        while (T-- > 0) {
            String s = scanner.next();
            long K = scanner.nextLong();

            int     n         = s.length();
            Integer [] suffix = GenerateSuffixArray(s);
            int     [] lcp    = GenerateLCP(s, suffix);

            for (int i=0;i<n;i++) {
                long len    = n - suffix[i];
                long count  = len * (len + 1) / 2;
                long prefix = lcp[i];

                count -= prefix * (prefix + 1) / 2;

                // If K is not found within the substrings generated by this
                // suffix, then go on to the next suffix.
                if (K > count) {
                    K -= count;
                    continue;
                }

                // K must be found within this suffix.  Now iterate through its
                // substring to find the exact substring and character.
                for (long j=prefix+1;j<=len;j++) {
                    if (K <= j) {
                        System.out.println(s.charAt((int) (suffix[i] + K - 1)));
                        break;
                    }
                    K -= j;
                }
                break;
            }
        }
    }
}