Calculator that works by compiling and running a .java file I wrote this code for a simple calculator. It creates a java file from the term, compiles and executes it. Now my question is : Is there a better way of doing this ? Would it be more efficient to analyze the string and then evaluate it step by step in the program ? Is this the right way of doing java ? Do I have to improve my 'style' ?

class Calculator {
    public static double evaluate(String term) throws IOException {
        String result=term;
        char[] n={'0','1','2','3','4','5','6','7','8','9','+','-','*','/','(',')'};
        boolean number=false;
        boolean add=true;
        int adds=0;
        for (int i=0; i < term.length(); i++) {
            number=false;
            for (int i2=0; i2 < n.length; i2++) {
                if (term.charAt(i)==n[i2]) {
                    number=true;
                    add=true;
                    break;
                } else {
                }
            }
            if (number==false && add==true)  {
                String p1=result.substring(i+adds*5,result.length());
                String p2=result.substring(0,i+adds*5);
                result=p2+"Math."+p1;
                adds++;
                add=false;
            }
        }
        JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
        FileOutputStream terminus = new FileOutputStream("Term.java");
        terminus.write(new String("import java.lang.Math; import java.io.FileOutputStream; import java.io.IOException; public class Term { public static void main(String[] args) {try {FileOutputStream result = new FileOutputStream(\"result.txt\");result.write(Double.toString("+result+").getBytes());} catch (Exception e) {}}}").getBytes());
        compiler.run(System.in, System.out, System.err, "Term.java");
        Runtime.getRuntime().exec("java Term");
        int i=0;
        FileInputStream resu = new FileInputStream("result.txt");
        String r="";
        i=resu.read();
        while(i != -1) {
            r=r+new String(new char[] {(char)(i)});
            i=resu.read();
        }
        return Double.parseDouble(r);
    }
}

And here's how I use it :

          try {
          System.out.println(Calculator.evaluate("cos(1+1)*sin(7+9)/atan(7)"));
          }
          catch (Exception e) {}

Let me answer your questions first:

Yes, there is a better way of doing this, and you already know it as you have suggested it yourself.
Yes, it would be more efficient to do it manually by analyzing the String and evaluating it part-by-part, like an interpreter would.
This is most definitely the wrong way of doing Java (not mincing my words here, you know that you're doing it wrong, otherwise that question wouldn't have occurred to you).
Yes, you have to improve your style. See below.

My Views:

Your approach to a calculator is certainly novel, for Java. This is highly  inefficient (you have to spawn new processes, which is always slow), as you have suspected. What you do is basically emulating eval of Python. Bad idea, but you've done it. Now how to go forward? (Yes, doing this the "classic" way is the way to do it in Java. Doing it this way feels like you are forcing Java to be more Python-ish.)

Style:

Space around operators, please! Whitespace is free and makes the code that much more readable.
Magic numbers. Instead of 5 everywhere, put

private static String prefix = "Math.";
private static int prefixLength = prefix.length();

at the top of the Calculator class, and use those instead of their values. More self-explanatory and much easier to read and maintain. This prefix business is voided by another suggestion below, though.
Weird variable names. Nested loop indices are usually named i,j,k, etc., like in mathematics. What do adds and add even mean? p1 and p2 are as good as foo and bar in telling what they contain, which is no good.

Code:

Why do you wrap a String literal into a String object using a String(String) constructor? (If that sounded weird to you, better drop the new String("...") for just "..." everywhere.
Where you are creating your Term.java, you don't need the import java.lang.Math, it's imported by default as it is part of the java.lang package.
What you might want to do is have import static java.lang.Math.*, which is a better idea as it allows you to skip all the weird string processing going on in the loop where you prepend "Math." wherever there is a function call in the term.
Why an empty else? If you don't need it, skip it.
Comparing boolean values against boolean literals is not required. For <x> == true, use just <x>, and for <x> == false, use just !<x>
Wrap the possible IOException in a RuntimeException and rethrow it. This prevents the caller from having to rethrow the exception or gobble it (like you do) compulsorily.
Try FileReader and FileWriter, especially wrapped up in a BufferedReader and BufferedWriter, respectively, instead of using FileInputStream and FileOutputStream raw - the former ones will allow you to
read and write whole Strings at a time, getting rid of all the getBytes() calls you have on Strings (horrible idea especially if you don't specify your file encodings)
String.format(String, Object...) is a good idea if you have to create a String with variables interpolated into it.


String self-similarity It's a string problem I have been making on Hackerrank. It is executing fine on all test cases except the last two. These last two test case are declaring it "Terminated due to time out".

C programs are allowed 2 seconds on this site and somehow my program is taking a fraction beyond 2 seconds. How am I supposed to reduce that time taken?

  For two strings A and B, we define the similarity
  of the strings to be the length of the longest prefix common to both
  strings. For example, the similarity of strings "abc" and "abd" is 2,
  while the similarity of strings "aaa" and "aaab" is 3.

  Calculate the sum of similarities of a string S with each of its suffixes.

   #include<stdio.h>
#include<string.h>
void stringMatch(char strMain[]) {
        int similarity = 0,i,j;
        for(i =0;i<strlen(strMain);i++){
            for(j =0;j+i<strlen(strMain);j++){
                if(strMain[j] == strMain[j+i]){
                    similarity++;
                }else{
                    break;
                }

            }
        }
        printf("%d\n",similarity);
}

int main(){
        int T,i;
        scanf("%d",&T);
        char str[1000000];
        for(i =0;i<T;i++){
            scanf("%s",str);
            stringMatch(str);
        }
}

You re-calculate string length every iteration of every loop:

    for(i =0;i<strlen(strMain);i++){
        for(j =0;j+i<strlen(strMain);j++){

It does not look like the string changes length. So calculate it once.

    size_t  size = strlen(strMain);
    for(i =0; i < size; i++){
        for(j =0; j+i < size; j++){

Or don't even calculate it at all. The string is terminated when you reach the '\0' character so just look for that

    for(i =0; strMain[i]; i++){
        for(j =0; strMain[j+i]; j++){

The other thing I noticed is that you are checking the string against itself. Which is not similar to the description you give above (where you are comparing two different strings).


Properly read integer in a range in C I wanted to write a C function C99/POSIX compliant that read an integer from the user input. I wanted this function to be safe and robust but I feel it is way too complex for such simple task.

I am wondering whether this code is optimal.

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

/**
 * Read an integer from `stdin`.
 * @param min Minimum accepted value
 * @param max Maximum accepted value
 * @param nom Default value
 * @return captured integer
 */
int read_integer(char *text, int min, int max, int nom) {
    int n = nom;
    bool failure = false;

    do {
        printf("%s [%d] ? : ", text, nom);

        // Read user input
        char line[24];
        do {
            if (fgets(line, sizeof(line), stdin) != line || feof(stdin)) {
                exit(EXIT_FAILURE);
                break;
            }
        } while (strchr(line, '\n') == NULL);

        // Default value?
        {
            char *cursor = line;
            while ((*cursor == ' ' || *cursor == '\t') && *cursor != '\0') {
                cursor++;
            }
            if (*cursor == '\n') {
                return n;
            }
        }

        // Not a number ?
        if (sscanf(line, "%d", &n) != 1) {
            printf("Error: this is not valid entry!\n\n");
            continue;
        }

        // Not in the range ?
        if (n < min || n > max) {
            printf("Error: value should be between %d and %d!\n\n", min, max);
            continue;
        }

        return n;
    } while(true);
}

int main() {
    do {
        printf("You said %d\n",
            read_integer("What's the answer", 10, 50, 42));
    } while(!feof(stdin));
}

regarding:

if (fgets(line, sizeof(line), stdin) != line || feof(stdin))

This is unnecessary messy.  Suggest:

if( fgets( line, sizeof( line ), stdin )

If any error occurs, the returned value is NULL so the body of the if() will not be entered.

regarding:

if (fgets(line, sizeof(line), stdin) != line || feof(stdin)) {
            exit(EXIT_FAILURE);
            break;

the break; will never be executed because the call to exit() will have already exited the program.

regarding:

while(!feof(stdin));

please read: while(!feof()) is always wrong

regarding:

    return n;
} while(true);

The return is always executed, so this loop will never iterate, looking for a valid input.

regarding:

char *cursor = line;
        while ((*cursor == ' ' || *cursor == '\t') && *cursor != '\0') {
            cursor++;
        }
        if (*cursor == '\n') {
            return n;
        }

This while() code block will iterate to the end of the array line[], most of the time.  the result will be no number will be extracted.  Suggest, starting at line[0] to check for isdigit( line[i] ) and if true, then extract the number, perhaps using something like: strtol()


Given points on a 2D plane, find line that passes through the most points Could someone give some feedbacks from perspective of oo design and coding style on the following codes:

Question: Given n points on a 2D plane, find the Line with maximum number of points that lie on it. Similar to https://leetcode.com/problems/max-points-on-a-line/description/. I was given Point interface, empty Line class, empty Solution class and was asked to fill in some functions.

My codes:

public interface Point
{
  public double getX();
  public double getY();
}

public class Line
{ // added by myself.
  private Point p1, p2;
  public Line(Point p1, Point p2) {
    this.p1 = p1;
    this.p2 = p2;
  }
}

public class Solution
{
  // Write the function here
  public Line maxPointsLine(Point[] points) {
    if(points == null) return null;
    if(points.length==1) return new Line(points[0], new Point(p));//
    Map<Double, Integer> map = new HashMap<>();// O(n), n = points.length. n-1
    Line result;
    int maxPoints = 0;
    for(int i = 0; i < points.length; i++) { // One Point
      map.clear();
      int overlap =0;
      int countSameX = 1;
      for(int j = i+1; j < points.length; j++) { // the second Point
        double x = points[j].getX() - points[i].getX(); // x intersect
        double y = points[j].getY() - points[i].getY(); // intersect on y coordinate

        if(x==0 && y==0) {
          overlap++;
        }
        if(points[j].getX()==points[i].getX()) {// slope is infi,
          // slope (1) finite, (2) infinite,
          countSameX++;
          continue;
        }
        double slope = y/x;
        if(map.contains(slope)) map.put(slope, map.get(slope)+1);
        else map.put(slope, 2);
        if(map.get(slope)+overlap > maxPoints) { // each line slope and points[i]
          // update result and maxPoints
          maxPoints = map.get(slope)+overlap;
          result = new Line(points[i], points[j]);
        }
      }
      if(countSameX>maxPoints) { // line parallel to Y coordinate
        // update result and maxPoints
        maxPoints = countSameX;
        result = new Line(points[i], points[j]);
      }
    }
    return result;// null
  }
}

In the above codes, I added some comments based on questions I was asked.

(I did this coding question long ago. just came into my mind. ) In fact, the interviewer told me that didn’t have good coding style and lack of OO design. suggestions are welcomed for me to improve myself. Thanks.

By the way, as for coding style, there are plugins following standards like https://google.github.io/styleguide/javaguide.html. I just did not fix coding style issues manually due to lack of time.

As for OO design, I was given empty classes. I just added some functions to solve problem I was given.

I really did not find any big issues in my codes. Confused and frustrated.

i think your task was to use the given classes Point and Lineand extend them...

public class Line{

    public Line (Point a, Point b);

    public boolean contains(Point p);

}

thus leaving a quite simple task in your Solution (and pretty easy to read)

Lines lines = getAllLines();//easily iterate over the permutated Point matrix

for(Line line: lines){
    int amountOfPoints = 0;
    for (Point p: points){
        if (line.contains(p) {
            amountOfPoint = amountOfPoint + 1;
    }
}

NOTE

this is just an explanation of OO-Programming, i didn't include the subtaks, e.g. how to get the max of the lines, or how contains() works, but it surely gives a hint on where the questions aims at...


Tiny Encryption Algorithm implementation in C I was just looking for a general feedback about this code, which encrypts a file with the Tiny Encryption Algorithm. I'm currently studying C, and I wanted to know if there was anything I could improve in my coding style or anything. Sorry for my english.

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>

#define BASE_DELTA 0x9e3779b9
#define KEY_VALUE 0x7A24432646294A404E635266556A586E //128bits value

void encrypt (u_int32_t* v, u_int32_t* k) {
  u_int32_t delta = BASE_DELTA * 1;
  for (int i = 1; i <= 32; i++) {
    v[0] += (v[1] * delta) ^ ((v[1] << 4) + k[0]) ^ ((v[1] >> 5) + k[1]);
    v[1] += (v[0] * delta) ^ ((v[0] << 4) + k[2]) ^ ((v[0] >> 5) + k[3]);
    delta += BASE_DELTA;
  }
}

void encryptFile(const char file_to_encrypt_path[], u_int32_t key[4]){

  u_int32_t block[2];

  int nb_bytes_read = 0;
  int fd_to_encrypt = open(file_to_encrypt_path, O_RDONLY, 0655); //RDONLY permissions are 0655
  int fd_result_file = open("encrypted_file.dec", O_CREAT|O_TRUNC|O_WRONLY, 0666);

  while(1){
    memset(block, 0x0, sizeof(u_int64_t));

    nb_bytes_read = read(fd_to_encrypt, block, sizeof(u_int64_t));
    if(nb_bytes_read == -1){
      perror("read");
      exit(errno);
    }
    if(nb_bytes_read == 0){
      break; //EOF
    }
    encrypt(block, key);

    if(write(fd_result_file, block, sizeof(u_int64_t)) ==-1){
      perror("write");
      exit(errno);
    }
  }
}

int main(int argc, char const *argv[]) {
  if (argc < 2){
        printf("usage : %s <file to encrypt> \n", argv[0]);
        exit(EXIT_FAILURE);
  }

  u_int32_t key[4];
  u_int32_t block[2];
  memset(key, 0x0, sizeof(u_int32_t) * 4);
  memset(block, 0x0, sizeof(u_int32_t) * 2);

  __uint128_t key_val = KEY_VALUE; //Note that __uint128_t requiers gcc version > 4.4
  *key = key_val;

  encryptFile(argv[1], key);

  return 0;
}

Good formatting and layout.

Aside from bug, good code.

Bug?

I'd expect v[1] + delta, v[0] + delta per Tiny Encryption Algorithm.

// Reference code
// for (i=0; i < 32; i++) {                       /* basic cycle start */
//     sum += delta;
//     v0 += ((v1<<4) + k0) ^ (v1 + sum) ^ ((v1>>5) + k1);
//     v1 += ((v0<<4) + k2) ^ (v0 + sum) ^ ((v0>>5) + k3);
// }

  for (int i = 1; i <= 32; i++) {
    //            v--- ???
    v[0] += (v[1] * delta) ^ ((v[1] << 4) + k[0]) ^ ((v[1] >> 5) + k[1]);
    v[1] += (v[0] * delta) ^ ((v[0] << 4) + k[2]) ^ ((v[0] >> 5) + k[3]);
    delta += BASE_DELTA;
  }

Questionable sample code.

*key = key_val; only sets the first element of key[4],   Certainly the below functionality was desired.

// *key = key_val;
key[0] = key_val >> 0;
key[1] = key_val >> 32;
key[2] = key_val >> 64;
key[3] = key_val >> 96;

Use standard types

Use standard types from #include <stdint.h> like uint32_t instead of u_int32_t.  They are better known and more portable.

Minor

const

Use const as able to convey code's intent.

// void encrypt(u_int32_t* v, u_int32_t* k) {
void encrypt(u_int32_t* v, const u_int32_t* k) {

// void encryptFile(const char file_to_encrypt_path[], u_int32_t key[4]) {
void encryptFile(const char file_to_encrypt_path[], const u_int32_t key[4]) {

Make constants unsigned

Make constants unsigned for unsigned variables by adding u.

// #define BASE_DELTA 0x9e3779b9
#define BASE_DELTA 0x9e3779b9u

Unneeded *1

Perhaps there is some reason for this, - I do not see it.

// u_int32_t delta = BASE_DELTA * 1;
u_int32_t delta = BASE_DELTA;

Debug code?

Clearing the block serves little aside from cleaner debug.  Don't use a type unrelated to the variable.

// memset(block, 0x0, sizeof(u_int64_t));
memset(block, 0x0, sizeof block);

0 base

Starting from 0 is more C-ish, yet I have no real issue with OP's code.

// for (int i = 1; i <= 32; i++) {
for (int i = 0; i < 32; i++) {

Avoid naked magic numbers

TEA uses 64 rounds, nice to self-document that in code

#define TEA_ROUNDS 64
for (int i = 0; i < TEA_ROUNDS/2; i++) {

open() test, close()?

Missing tests if open() success.
Missing close(), x2, in encryptFile().

Advanced

restrict

Use restrict (C99) to let encrypt() know that v, k do not overlap.  This allows for some optimizations.

// void encrypt(u_int32_t* v, u_int32_t* k) {
void encrypt(u_int32_t* restrict v, const u_int32_t* restrict k) {

Out of range constant.

0x7A24432646294A404E635266556A586E is out of range of most compilers, even when __uint128_t exists.

Portable code would use a different way to set the key as well and a different type.  Good that it work for you.


JSLint says "unexpected function" doCalc See the full calculator with HTML -> https://gist.github.com/1861120
You should be able to drag that html file from github into a browser and start using it. I'm only putting the js below. The calculator works fine but JS Lint is telling me I have an unexpected 'function' doCalc line 9 character 1.

Also just looking for some general tips on best practices and how this might be refactored. Any help is appreciated. Thank you.

var type, n, r, formula;
Math.factorial = function (n) {
    if (n < 2) {
        return 1;
    }
    return (n * Math.factorial(n - 1));
}

function doCalc(type) {
    switch (type) {
        case 'permutation':
            n = document.permutation.T1.value;
            r = document.permutation.T2.value;
            formula = function() {
                document.permutation.T3.value = Math.factorial(n) / Math.factorial(n - r);
            }
            break;
        case 'combination':
            n = document.combination.T1.value;
            r = document.combination.T2.value;
            formula = function() {
                document.combination.T3.value = Math.factorial(n) / (Math.factorial(r) * Math.factorial(n - r));
            }
            break;
    }
    n = Number(n);
    r = Number(r);
    if (isNaN(n) || isNaN(r)) {
        alert("Factorial requires a numberic argument.");
        return null;
    }
    formula();
}

window.onload = function() {
    document.getElementById('calculate-p').onclick = function() {
        doCalc('permutation');
    }
    document.getElementById('calculate-c').onclick = function() {
        doCalc('combination');
    }
}

Well, you asked for general tips, so here goes ;)

I would almost always refrain from extending built-in types. I will "shim" to provide standard-compliant functions which are not available (e.g. "forEach" on Array.prototype), but adding new methods to existing types is a recipe for problems. You could simply have a factorial function without the Math and it would work just as well.

In fact, you could do yourself a favor and create your own Math object (named something different, of course) and add your formula functions to that, so:

var MathFactor = new function () {
    this.factorial = function (n) {
        return n < 2 ? 1 : n * this.factorial(n - 1);
    };
    this.permutate = function (n, r) {
        return this.factorial(n) / this.factorial(n - r);
    };
    this.combine = function (n, r) {
        return this.factorial(n) / (this.factorial(r) * this.factorial(n - r));
    };
};

Then doCalc is just a matter of a simple numeric check, then setting your T3.value to the output of a call to MathFactor.permutate() or MathFactor.combine(). Hope that helps!


Checking whether a list is sorted, ascending or descending What is the best way to conditionally use an operator? All 3 methods seemed acceptable to me, and the last 2 better.

If you're curious: I've got a list which is unsorted, and many other lists which may or may not contain a smaller selection of sorted indices referencing the unsorted list. Processed data is being identified as lists which contain sorted references.

I've tried an anonymous function...

if reverse:
    list_is_sorted = lambda unsorted, indices:    \
                     all(unsorted[indices[i]] >=  \
                         unsorted[indices[i + 1]] \
                         for i in xrange(len(indices) - 1)))
else:
    list_is_sorted = lambda unsorted, indices:    \
                     all(unsorted[indices[i]] <=  \
                         unsorted[indices[i + 1]] \
                         for i in xrange(len(indices) - 1)))
list_is_sorted()

And an evaluated expression and...

if reverse:
    my_operator = '>='
else:
    my_operator = '<='
eval('all(unsorted[indices[i]] %s  \
          unsorted[indices[i + 1]] \
          for i in xrange(len(indices) - 1))' % my_operator)

Assigning an operator like this:

import operator
if reverse:
    my_operator = lambda x, y: operator.ge(x, y)
else:
    my_operator = lambda x, y: operator.le(x, y)
all(my_operator(unsorted[indices[i]], unsorted[indices[i + 1]]) \
    for i in xrange(len(indices) - 1)))

I'm a little sure if this is well suited for Code Review, but here goes a little analysis:

Using anonymous functions

This doesn't look right as it is repeating too much code, which is a clear code smell. When quickly looking at it, it is unclear what the actual difference is between the two. So this is not a good option.

Using eval

An old saying says "using eval is evil", and you should avoid using eval if possible. Not only does it have an performance hit, but it is also a major source of security flaws, and not readable code.

Defining your own operator

This is by far the nicest solution, as it doesn't repeat code, and it reads the best. However you could simplify it using direct assignment of the operator instead of using a lambda expression, thusly making it to be:

import operator

my_operator = operator.ge if reverse else operator.le
all(my_operator(unsorted[indices[i]], unsorted[indices[i + 1]]) \
    for i in xrange(len(indices) - 1)))

In a function using a pairwise recipe

Or introducing a function, and the pairwise iterator from itertools recipes v2.7:

import operator
import itertools

def is_sorted(iterable, reverse= False):
    """Check if the iterable is sorted, possibly reverse sorted."""

    def pairwise(iterable):
        """s -> (s0,s1), (s1,s2), (s2, s3), ..."""
        a, b = itertools.tee(iterable)
        next(b, None)
        return itertools.izip(a, b)

    my_operator = operator.ge if reverse else operator.le

    return all(my_operator(current_element, next_element)
                   for current_element, next_element in pairwise(iterable))

# Test the function
print is_sorted([1, 2, 3, 4])            # Should be True
print is_sorted([4, 3, 2, 1], True)      # Should be True
print is_sorted([4, 1, 2, 3])            # Will not be True, ever...


Implementation of system() function in Linux using fork() , exec() and waitpid() system calls I have written the code for implementation of  system() function using fork(), exec() and waitpid(). Could someone please review this code and provide feedback. Thanks a lot.

#include<stdio.h>
#include<string.h>
#include<unistd.h>
#include<sys/types.h>
#include<sys/wait.h>
#include<error.h>
#include<errno.h>
#define size 30

char *get_command(int argc, char *argv[]);
int my_system(const char *command);

int my_system(const char *command)
{
    pid_t pid;
    int wstatus  = 0;
    int ret = 0;

    if (command == NULL)
        return 1;
    pid = fork();
    if (pid == -1) {
        return -1;
    } else if (pid == 0) {
        ret = execle("/bin/sh", "sh", "-c",command, (char *)NULL);
        if (ret == -1)
            return wstatus;

    } else {
        ret = waitpid(-1, &wstatus, 0);
        if (ret == -1)
            return -1;
    }
    return wstatus;
}

char *get_command(int argc, char **argv)
{
    int i = 0;
    static char command[size];

    if (argc == 1)
        return NULL;

    strcpy(command, argv[1]);
    for (i = 2; i < argc; i++) {
        strcat(command, " ");
        strcat(command, argv[i]);
    }
    return command;
}

int main(int argc, char *argv[])
{
    int ret;
    char *command;

    command = get_command(argc, argv);
    ret = my_system(command);
    if (ret == 1)
        printf("Command is NULL, shell is available\n");
    else if (ret == -1)
        printf("Child process could not be created or error in the wait system call\n");
    else if (ret == 127)
        printf("The Child process could not be executed in the shell\n");
    else
        printf("The status of the child process is :%d\n", ret);
    return 0;
}
```

I think the biggest mistake is the return after the exec.  If you reach that point, and return, you have returned to the calling function in the subprocess.  Use exit() instead.  Note that the value you pass to exit should come out in wstatus.  Note also, if execle returns, it must have failed, so don't even both checking the status.  (For this to happen, the path must be invalid.  Change it for testing, and set it back afterwards.)

The use of execle() is a mistake.  You didn't pass an environment.  Use execl() instead.

You have at least one case where you return -1 without having set errno.  Set errno.  (Many cases, something else has already set errno for you.)

One other thing that can bite you:  If the caller has made another subprocess, and it terminates while you are waiting for your subprocess, you will return the result of the wrong subprocess.  To fix this, pass the pid as the first parameter to waitpid().

As a style issue, I would recommend moving wstatus completely into the last block.  And nothing should ever come out of the compound if statement.

As a personal style point: I tend to use switch to test for -1, 0, or other.  if statements work equally well.


Confirmation to delete items using a jQuery modal dialog I'm looking for suggestions on how to clean up JavaScript/jQuery code that looks like the example in the Before snippet.

I'm a backend developer so some of the constructs in JavaScript are foreign to me. I took a stab at it in the After block but I'm not sure if there is a name for that design pattern.

Most of the JS code we have are click handlers and DOM table manipulation. There are also situations where we need to keep state like rows visible, etc.

In order to easily toggle console logging, I added a log() function that checks the debug flag.

Before

$(function() {

  // Delete button handler
  $('.delete-button').click(function(e) {
    e.preventDefault();
    var id = $(this).closest('tr').data('id');
    var name = $(this).prop('name');
    var modal = $('#delete-modal');
    modal.find('.modal-body').html("You've chosen to delete " + name + "");
    modal.data('id', id);
    modal.modal('show');
    console.log('Showing modal');
  });

  // Confirm delete button handler
  $('#confirm-delete-button').click(function(e) {
    var modal = $('#delete-modal');
    var id = modal.data('id');

    modal.modal('hide');

    console.log('Deleting id ' + id);

    // Build form for submission
    var $form = $('<form action="" method="POST"></form>');
    $form.append('<input type="hidden" name="id" value="' + id + '">');
    $form.append('<input type="hidden" name="reqType" value="Delete">');
    $(document.body).append($form);
    $($form).submit();

  });
});

After

var App = {

  rowCount: 0,
  visibleCount: 0,
  debug: true;

  onReady: function () {
    App.initHandlers();
  },

  log: function (message) {
    if (App.debug) {
      console.log(message);
    }
  },

  initHandlers: function () {
    $('.button-delete').click(App.showDeleteModal);
    $('#button-confirm-delete').click(App.deleteRow);
  },

  showDeleteModal: function (event) {
    event.preventDefault();
    var id = $(this).closest('tr').data('id');
    var name = $(this).prop('name');
    var modal = $('#delete-modal');
    modal.find('.modal-body').html("You've chosen to delete " + name + "");
    modal.data('id', id);
    modal.modal('show');
    App.log('Showing modal');
  },

  deleteRow: function () {
    var modal = $('#delete-modal');
    var id = modal.data('id');

    modal.modal('hide');

    App.log('Deleting id ' + id);

    // Build form for submission
    var $form = $('<form action="" method="POST"></form>');
    $form.append('<input type="hidden" name="id" value="' + id + '">');
    $form.append('<input type="hidden" name="reqType" value="Delete">');
    $(document.body).append($form);
    $($form).submit();
  },

  updateToolbar: function () {
    App.visibleCount = $('.product-row:visible').length;
    App.rowCount = $('.product-row').length;

    if (App.visibleCount === App.rowCount) {
      $('div.toolbar').html('
Showing ' + App.visibleCount + ' of ' + App.rowCount + ' rows
');
    } else {
     $('div.toolbar').html('
Showing ' + App.visibleCount + ' of ' + App.rowCount + ' rows
');
    }
  },
};

$(document).ready(App.onReady);

The main benefit to your refactoring job is to clarify what the code is doing. The original version lacks names for things, so does clicking the .button-delete element delete the item? After further reading the code, "oh, it appears to be showing a modal dialog. Now where to we actually delete the item..." - so your intent here is very good. :)

Use $(...) instead of $(document).ready(...) as this is more idiomatic for jQuery.

So $(document).ready(App.onReady); becomes $(App.onReady);
No need to App.onReady, just use an anonymous function:

$(function() {
  App.initHandlers();
});

Remember that the context of what this points to changes based on how you call a function in JavaScript. Using the code above means the this variable in initHandlers refers to the App object, instead of the window object.
Don't maintain state if it is not necessary

The rowCount and visibleCount properties are only referenced in the updateToolbar. Use local variables instead of properties, since you overwrite the values of those properties on every invocation of updateToolbar
Cleaning up repeated HTML code

The HTML code to generate the
 tag in the toolbar has only one minor difference, which is the HTML class name. Use a local variable for that class name and then have one hard coded HTML string for the heading tag:

var headingClass = App.visibleCount === App.rowCount
    ? "label-default"
    : "label-primary";

var headingMarkup = '
Showing ' + App.visibleCount + ' of ' + App.rowCount + ' rows
';

$('div.toolbar').html(headingMarkup);

The name App is pretty generic. Methods like "initHandlers" and "deleteRow" are also too generic. This class could end up expanding way beyond what it should. A more focused approach will prevent this class from becoming an unmaintainable monster later on.

Based on the naming of things in the code, it looks like something that is deleting items from a "shopping cart" table. A more appropriate name would be ShoppingCartTableController instead of App.

Now names like "initHandlers" and "deleteRow" have a little context, and a clear focus.

Calling this a "controller" is appropriate here, because it is responding to events in the View, which is precisely what a "Controller" does in the MVC design pattern.
In the deleteRow method, a <form> tag is created, then two hidden inputs are appended. There is a hidden cost to this. HTML is being parsed 3 times, and the document tree on the page is being modified 3 times. With a little rearranging you can parse the HTML once, and only append to the document tree once:

// Build form for submission
var html = '<form action="" method="POST">'
         +      '<input type="hidden" name="id" value="' + id + '">'
         +      '<input type="hidden" name="reqType" value="Delete">'
         + '</form>';
var $form = $(html);

$(document.body).append($form);

$form.submit();

Second of all, passing $form into the $ function is completely unnecessary, since $form is already a jQuery object. Just call $form.submit() directly.

Refactoring this into an object isn't really gaining you anything. Instead, you could opt for a named function that much more clearly makes your intent known:

function ShoppingCartTableController() {
  var showDeleteModal = function (e) {
    // ...
  },
  deleteRow = function (e) {
    // ...
  },
  log = function (message) {
    if (ShoppingCartTableController.debug) {
      console.log(message);
    }
  };

  $('.delete-button').click(showDeleteModal);
  $('#confirm-delete-button').click(deleteRow);
}

ShoppingCartTableController.debug = true;

$(ShoppingCartTableController);


Populating an unordered list from values in a JSON file I have this view that populates an unordered list from values in a JSON file, which I am doing in Node.

Is there a better way to do this, or a more 'node-like' way? Can I load the JSON file without an Ajax call?

<!DOCTYPE html>

    <title>4&middot;pli -- news</title>
    <link rel='stylesheet' href='/stylesheets/style.css'/>

    <link href='http://fonts.googleapis.com/css?family=Didact+Gothic' rel='stylesheet' type='text/css'>

<% include shared/nav.ejs %>

<% include /shared/footer.ejs %>

What you're looking for is JSON.parse. It's not in the Node docs, because it's "lower" than that: Every modern javascript runtime has it (see MDN).

Here's a simple function to read a file, parse it as JSON, and send it to a callback (all in Node)

function readJSONFile(filename, callback) {
  require("fs").readFile(filename, function (err, data) {
    if(err) {
      callback(err);
      return;
    }
    try {
      callback(null, JSON.parse(data));
    } catch(exception) {
      callback(exception);
    }
  });
}

In keeping with Node conventions (and just mirroring readFile itself), you pass it a callback with this signature: function(err, json). E.g.

readJSONFile("path/to/file.json", function (err, json) {
  if(err) { throw err; }
  console.log(json);
});

You can make a none-async one if you prefer, but Node's nature is async.


PHP form validation and db insertion, one page or two I have a PHP form that uses the POST method to get information from form to db. I have my form separate from the form validation and insertion page.

Here is my form:

<?php
//get the header and the navigation bar inside the page.
$page_title = 'Demographics';
include('php_includes/header.html');
?>

    <form class="form" action="insert_demographics.php" method="post" name="demographics" id="demographics">

Demographics

Medical Record Number / Visit ID (Number 17 Characters):

        <input class="form-control" id="mrn" name="mrn" type="text" maxlength="17">

Last Name:

        <input class="form-control" id="LastName" name="LastName" type="text" onfocus="emptyElement('status')" maxlength="75">

First Name:

        <input class="form-control" id="FirstName" name="FirstName" type="text" onfocus="emptyElement('status')" maxlength="75">

Social Security Number:

        <input class="form-control" id="SSN" name="SSN" type="text" onfocus="emptyElement('status')" maxlength="9">

    <button type="submit" id="demographicsbtn">Insert Demographics Data</button>

</form>

<?php
include('php_includes/footer.html');
?>

Here is my validation and insert page:

<?php
$page_title = 'Insert Demographics';
include('php_includes/header.html');

// Connect to the database
include_once("php_includes/db_connect.php");

// Gather the posted data into local variables
$mrn_demographics = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z0-9]/', '', $_POST['mrn']));
$last_name        = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z]/', '', $_POST['LastName']));
$first_name       = mysqli_real_escape_string($db_connect, preg_replace('/[^a-zA-Z]/', '', $_POST['FirstName']));
$ssn              = mysqli_real_escape_string($db_connect, preg_replace('/[^0-9]/', '', $_POST['SSN']));

// Get user IP Address
$ip = preg_replace('#[^0-9.]#', '', getenv('REMOTE_ADDR'));

// Form data error handling
if($mrn_demographics == "" || $last_name == "" || $first_name == "" || $ssn == ""){
echo "The form submission is missing data.";
exit();
} else {
// End form data error handling
$sql = "INSERT INTO nys_demographics (
    mrn,
    pt_last_name,
    pt_first_name,
    pt_ssn,
    ip_address,
    record_insert_dtime
)
VALUES('$mrn_demographics' ,'$last_name','$first_name','$ssn','$ip',
now())";
}

if (mysqli_query($db_connect, $sql)) {
  echo "
Demographics have been inserted successfully, you will be auto-directed to the Injury Details form.

";
  echo "MRN: $mrn_demographics
";
  echo "Last Name: $last_name
";
  echo "First Name: $first_name
";
  echo "SSN: $ssn
";
  echo "From IP Address: $ip
";
  echo "On " .date("Y-m-d") ." at " .date("h:i:sa");
  header("refresh:3; url=injury_details.php");
} else {
  printf("Error: %s\n", mysqli_error($db_connect));
}

include('php_includes/footer.html');
?>

I have a couple of questions:

Should my validation code be in a different PHP page or the same?
Could my validation be better and better written?

I want to echo out what has been input into the form.

You don't need to have your validation codes written in different page. But if you want to organize your code, you can go and separate your validation block.
Consider changing your validation. Your preg_replace is quite limiting. An example would be for names. Spaces, numbers, special characters are not allowed. Don't forget that there are names that has the mentioned above. It would work. Your validation could be written better if you want to increase security. There are tons of practice for validation of data and securing it for SQL query.
You could add some client side validation using HTML5 such as pattern, and required attribute.
You said you want to output what has been submitted in the form. Do you really need to output it in the validation/insertion page and then get redirected to the other page? You can instead put the details on sessions and head directly to the page you need to be and output what you have put on the sessions.
It is best to avoid putting PHP variables inside the quotation marks. "Your name is $name". Instead you can use {} to include it. "Your name is {$name}". However, it is recommended that you concatenate it instead "Your name is" . $name.
When including database page, it is also recommended to use require instead of include. This is to make the script stop upon error on getting the file.


Measuring typing speed in terminal without a mistake I tend to dislike my code. It's always a mess. I can't critique my own code though, because I just turn it into another mess. I've been programming slightly better every year, but this year it has hardly changed.

#!/bin/bash

RED='\e[101m'
NC='\033[0m' # No Color

lesson[1]="#!/bin/bash"
lesson[2]="#!/bin/bash"

lessons=${#lesson[@]}
current_lesson=1
speed=70

function start() {
        SECONDS=0 #restart timer

        reset_user_input
        string_to_type=${lesson[$1]};
        length_of_string=${#string_to_type}
        echo $string_to_type

        count=0
        while [ "$count" -ne "$length_of_string" ]; do
                checkCharacter
        done
        time_it_took=$SECONDS
        sec=60
        wpm=$((sec / time_it_took))
        wpm=$((wpm * length_of_string / 5))
        echo " "
        echo "speed: "$wpm
}

function checkCharacter() {
        IFS= read -r -s -n 1 user_input_char
        add_user_input $user_input_char

        if [ "$user_input_char" == " " ]; then
                add_user_input " "
                if [ "$user_input_char" == "${string_to_type:$count:1}" ]; then
                        printf " "
                else
                        printf ${RED}" "${NC}
                fi
        else
                if [ "$user_input_char" == "${string_to_type:$count:1}" ]; then
                        printf $user_input_char
                else
                        printf ${RED}$user_input_char${NC}
                fi
        fi
    let count+=1
}

function check_correct() {
        if [ "$user_input" == "$string_to_type" ]; then
                return;
        else
                echo "too many mistakes, try again!"
                false;
        fi
}

function check_speed() {
        if [ "$wpm" -gt speed ]; then
                return;
        else
                echo "too slow, try again!"
                false
        fi
}

function start_lesson() {
        start $1
        if check_speed && check_correct; then
                echo "you succeeded!"
                echo " " # padding for every lesson
                return;
        fi
        echo " " # padding for every lesson
        false
}

function reset_user_input() {
        user_input=""
}

function add_user_input() {
        user_input=${user_input}$1
}

while [ "$current_lesson" -le "$lessons" ]; do
        echo "lesson:" $current_lesson
        until start_lesson $current_lesson; do : ; done;
        let current_lesson+=1
done

I think this is very nice, clean code. Let's make it better!

Consistency

Consistency helps a lot, making the code easier to understand.

Some functions are named camelCase, others snake_case. Stick to one pattern.
Some variables are named SHOUT_CASE, others snake_case. Stick to one pattern. The latter is recommended, because upper case variable names are commonly used for system variables like PATH and others.
The indentation is mostly 8 spaces, with few exceptions. Use the same everywhere.

Variable declaration

It's good to declare global constants at the top of the file where they are easy to see.
The same is not true for state variables that are implementation details,
such as current_lesson.
That variable would have been better to initialize right before you need it.

Iterating over arrays

Instead of a counting loop over the lesson array,
there is a more idiomatic approach:

for value in "${lesson[@]}"; do ...; done

I would also rename the array to plural lessons.

The return value of functions

There are some subtle points I don't like in this code:

function check_correct() {
        if [ "$user_input" == "$string_to_type" ]; then
                return;
        else
                echo "too many mistakes, try again!"
                false;
        fi
}

A return command without parameter will use the exit code of the last statement. In this case it's success. I would find it clearer if you used an explicit return 0 there.

The else branch ends with a false statement.
If this is the last statement in the function,
then the exit code of the function will be 1, as intended.
But this is not explicit enough.
The function doesn't return immediately on the false statement.
An explicit return 1 would be much better.

Like this:

function check_correct() {
        if [ "$user_input" == "$string_to_type" ]; then
                return 0
        else
                echo "too many mistakes, try again!"
                return 1
        fi
}

With my other suggestions further down, I recommend to write this function like this:

check_correct() {
        if ! [ "$user_input" = "$string_to_type" ]; then
                echo "too many mistakes, try again!" >&2
                return 1
        fi
}

Undocumented features

The == operator is not documented in man [.
For that reason I recommend using = instead of == in [ ... ].

Style

The recommended function declaration style is without the function keyword:

start() {

There are a bunch of redundant semicolons at end of lines that can be safely removed.

Instead of this:

wpm=$((sec / time_it_took))

You can write slightly simpler like this:

((wpm = sec / time_it_took))

This works too:

((wpm *= length_of_string / 5))

I don't remember the explanation, but instead of let count+=1, ((count+=1)) is recommended. An even better way to write is ((count++)).

Sharpening the saw

  I've been programming slightly better every year, but this year it has hardly changed.

Just keep doing it!


Comparing columns values and change their places by condition There are three simple tables. Actor, Film, Film_actor.

Film_Actor table:

 actor_id     | film_id
--------------+---------------------------
     1        |  3
     2        |  4

Actor table:

 actor_id   | first_name                  | last_name
------------+-----------------------------+----------
     1      | Daniel                      |  Craig
     2      | Eva                         |  Green

Film table:

 film_id     | title
-------------+---------------------------------------
    3        | Casino Royale
    4        | Colombo

I would like to find two actors who cast together the most and list the titles
of those movies. Moreover, actor_id of the first_actor should be lower than actor_id of the second_actor.

So the desired result should look like this:

first_actor     | second_actor  | title
----------------+---------------+---------------
Daniel Craig    | Eva Green     | Casino Royale

I've written the following code:

SELECT
        ac1.actor_id first_actor_id
        , ac1.first_name + ' ' + ac1.last_name   first_actor
        , ac2.actor_id second_actor_id
        , ac2.first_name + ' ' + ac2.last_name second_actor
        ,count(*) FilmCount
    FROM
        Actor as ac1
        INNER JOIN Film_Actor as fa1 ON fa1.actor_id = ac1.actor_id
        INNER JOIN Film_Actor as fa2 ON fa1.film_id = fa2.film_id
        INNER JOIN Actor as ac2 ON fa2.actor_id = ac2.actor_id
            and fa1.actor_id < ac2.actor_id
        INNER JOIN Film as fi ON fa1.film_id = fi.film_id
    GROUP BY
        ac1.actor_id
        , ac1.first_name
        , ac1.last_name
        , ac2.actor_id
        , ac2.first_name
        , ac2.last_name

In my view, the above code is a little bit clumsy as the code makes checking of this condition  actor_id of the first_actor should be lower than actor_id of the second_actor*` just for second actor.

Is it possible to improve comparing actor_id of the first_actor with actor_id of the second_actor`?

Since inner joins are used, you'll only ever be returned rows where the first_actor_id is smaller than the second_actor_id, so the code works as intended.

If I understand correctly, you want the code to check that first_actor_id is smaller than second_actor_id AND second_actor_id is bigger than first_actor_id. However, those two conditions are the same so you only need one of them.

As a sidenote, your code will not work as intended for the following reasons: (1) It will bring back any actors who have starred in ONE movie together, rather than two, and (2) it does not list the movie names they have starred in.

The first problem is easy to fix; add HAVING count(*) >= 2 to the end of the statement.

The second problem depends on the version of SQL Server you are using (use SELECT @@VERSION to find out):

If you're running SQL Server 2017 or later, exchange count(*) in the SELECT for STRING_AGG(fi.title, ', ') (documentation linked) – this will give you a comma-separated list of films.
If not, the best way to concatenate strings from a group is the "stuff XML path" method. A quick search online should explain how this trick works.


String matching algorithm I have been going through algorithms and data structures and recently came across string matching algorithms.  I have not yet wrapped my head around the significance KMP algorithm so I came up with the function below.  Is it really efficient or is there a better approach?

bool simplePatternMatch(const std::string &text, const std::string &pattern){
    //return true if pattern occurs otherwise false
    auto pat_it = pattern.begin();
    std::string::const_iterator it = text.begin();
    for(; it != text.end(); it++){
        if(*it == *pat_it){
            auto internal_it = it + 1;
            pat_it++;
            for(; internal_it != text.end();internal_it++){
                if( *internal_it == *pat_it)
                    pat_it++;
                else{
                    //a partial match occured so begin again to look for a match
                    pat_it = pattern.begin();
                    break;
                }
                if(pat_it == pattern.end())
                    return true;

            }

        }
    }
    //if we reach here there was no match
    return false;
}

Code Style

Whitespace and Braces

Be consistent in your usage of whitespace and braces. for and if statments should have a space between the keyword, parentheses, and opening brace (if using K&R style braces). Also, don't omit braces in an if, then include them in the else. For example:

if( *internal_it == *pat_it)
    pat_it++;
else{ // ...

Becomes:

if(*internal_it == *pat_it) {
    pat_it++;
} else { // ...

Reduce Variable Scope

it and internal_it are both declared outside their respective for statement, but are not referenced outside of it. Declaring them inside the for statement limits their scope to within the for loop, reducing opportunities for them to be misused. You can also use auto for type deduction, saving you the necessity of typing long iterator type names.

Reduce Nesting

Your code contains many nested if and for statements. This can make the code hard to read. If you have an if statement wrapping the entire loop body, you can reduce the level of nesting by inverting the condition and breaking early:

if(*it == *pat_it){
    // loop body
}

Becomes:

if (*it != *pat_it) {
    continue;
}
// loop body

Simplify, Simplify

Your inner for loop has almost exactly the same bounds as the outer loop. This can be simplified to a single loop over text and pattern each:

bool simplePatternMatch(const std::string &text, const std::string &pattern)
{
    // handle empty-string case
    if (pattern.empty() || text.empty())
    {
        return pattern.empty() && text.empty();
    }

    for(auto it = text.begin(); it != text.end(); it++) {
        auto pat_it = pattern.begin();
        while (pat_it != pattern.end() && *it == *pat_it) {
            ++it;
            ++pat_it;
        }
        if (pat_it == pattern.end()) {
            return true;
        }
    }

    //if we reach here there was no match
    return false;
}

You could change the while to a for and increment it and pat_it in the increment statement, but I find this more readable. Writing it this way also makes it explicit that there is a loop over each of your string arguments, which makes analyzing complexity much easier.

But wait! All that iterator comparison is complicated. Wouldn't it be nice if there was another way? Turns out, there is std::mismatch(), which is essentially what the inner loop does. Then the loop can be simplified:

for (auto it = text.begin(); it != text.end(); ++it) {
    const auto mismatches = std::mismatch(pattern.begin(), pattern.end(), it);
    if (mismatches.first == pattern.end()) {
        return true;
    }
}

Algorithm

Now that your function has been simplified a bit, it is easier to see that you have written a naive pattern match, which has complexity \$\mathcal{O}(mn)\$, where m is your pattern length, and n is the string length. You can improve upon this by implementing one of the well-known pattern-matching algorithms, like Boyer-Moore or Knuth-Morris-Pratt. I leave this as an exercise to you, both because it would make this answer too long, and it would be a good candidate for another question on Code Review.


BaseRepository for EF6 Is this okay?

public class BaseRepository<T> where T : class
{
    private readonly DbContext _dbContext;

    public BaseRepository(DbContext dbContext)
    {
        _dbContext = dbContext;
    }

    public T Get(Func<T, bool> predicate)
    {
        return GetAll(predicate).FirstOrDefault();
    }

    public IEnumerable<T> GetAll(Func<T, bool> predicate = null)
    {
        IEnumerable<T> result = _dbContext.Set<T>().AsEnumerable();
        return (predicate == null) ? result : result.Where<T>(predicate);
    }

    public void Add(T entity)
    {
        _dbContext.Entry(entity).State = EntityState.Added;
    }

    public void Delete(Func<T, bool> predicate)
    {
        IEnumerable<T> entities = GetAll(predicate);
        foreach (T entity in entities)
            _dbContext.Entry(entity).State = EntityState.Deleted;
    }

    public void Delete(T entity)
    {
        _dbContext.Entry(entity).State = EntityState.Deleted;
    }

    public void Update(T entity)
    {
        _dbContext.Entry(entity).State = EntityState.Modified;
    }

    public async void Save()
    {
        await _dbContext.SaveChangesAsync();
    }

    public void Dispose()
    {
        if (_dbContext != null)
            _dbContext.Dispose();
    }
}

It seems strange to me for Get to be a specific way of getting all items, and GetAll should always return all items. Introduce a GetSome helper for clarity.

public T Get(Func<T, bool> predicate) {
    return GetWhere(predicate).FirstOrDefault();
}

public IEnumerable<T> GetAll() {
    return GetWhere(null);
}

public IEnumerable<T> GetWhere(Func<T, bool> predicate) {
    IEnumerable<T> result = _dbContext.Set<T>().AsEnumerable();
    return (predicate == null) ? result : result.Where<T>(predicate);
}

  Caveat: I don't write C# so perhaps this doesn't match the standard pattern there.


Calculating a time in a time zone from multiple dates in SQL I'm trying to calculate what time a certain time in a time zone is today, so I can schedule something to happen at that time in that time zone.

I've got a table with what I have termed the Nominal Time, which is stored as a datetimeoffset with an arbitrary date, as the only parts I care about is the time and the time zone offset. So, the Nominal Time column has values along the lines of:

2014-07-01 10:00:00.0000000 +02:00
2014-07-01 10:00:00.0000000 -05:00
2014-07-01 10:00:00.0000000 -07:00
2014-07-01 10:00:00.0000000 +01:00

(In this case 10am is my time I want to schedule this events). From this, I want to get that time today, so these would become:

2014-07-02 10:00:00.0000000 +02:00
2014-07-02 10:00:00.0000000 -05:00
2014-07-02 10:00:00.0000000 -07:00
2014-07-02 10:00:00.0000000 +01:00

when run at the date of this writing (2014-07-02). I currently have SQL that does this, but I don't really like it:

With    NominalTimes as (select Id, NominalTime, SYSDATETIMEOFFSET() Now
                        from FaxQueue where status=0),
        CalcTimes as (select Id, NominalTime, Now, DATEPART(year,Now) NomYear,
                        DATEPART(month,Now) NomMonth,DATEPART(day,Now) NomDay,
                        DATEPART(hour,NominalTime) NomHour,DATEPART(minute,NominalTime) NomMinute,
                        DATEPART(tzoffset,NominalTime) NomOffset from NominalTimes)
select Id, NominalTime, Now,
    DATETIMEOFFSETFROMPARTS(Nomyear,NomMonth,NomDay,NomHour,NomMinute,0,0,NomOffset/60,NomOffset%60,0)
    from CalcTimes

(Excuse the excessive CTEs; I'm trying to build this up bit by bit.) The end goal of this is to have a query that returns a list of rows where the nominal time happens within the next, say, hour (actual window size isn't important).

I will also note that from the function of the program, I do not need to worry about a time straddling a daylight saving time transition (the program is meant to run before DST happens in a time zone, and deliver a notification).

Is there a better way of doing these date calculations in SQL, or is this really about as good as it's going to get?

First, let me restate your problem, to make sure I understand it correctly.  You want to take the NominalTime column, which is of type datetimeoffset, and replace the date part with today's date, where "today" is defined according to the timezone in which the SQL Server is running.  The time and timezone offset will remain unchanged, even across DST boundaries.

To roll one field of a datetime-like object forward or backward, use the DATEADD() function:

SELECT Id
     , NominalTime
     , SYSDATETIMEOFFSET() AS Now
     , DATEADD(day, DATEDIFF(day, NominalTime, SYSDATETIMEOFFSET()), NominalTime)
    FROM FaxQueue
    WHERE status = 0;

SQL Fiddle demonstration


Basic quest code in PHP I'm a beginner in PHP and I would like to know how to make a good quest system.

What is the code for : This code is to accept quest. It show the quest and where the user is in the quest (Ongoing, Available, Finished..).

What I'm looking for : As a beginner, this code is quite bad, and i'm looking for an improved version. For example, all the code is only working for 1 quest, and I would like an useful way to automatically put the quest when created instead of having to copy/paste the whole thing and change the id of the quest. And of course if i could do this in 1 query instead of querya and queryb (1 For the Fetch and 1 For the FetchColumn) it would be good.

The code is actually working if you wonder

Important informations : Most of the text is in frech, so while I'm translating it, I may forgot to change 1 or 2 things. If that's the case, don't assume the code is not working : just tell me what is wrong and it may be a mistake.

The Code

<?php
  $querya = $db->prepare('SELECT * FROM quetejoueur WHERE id_joueur=:id AND id_queteorij=1');
  $querya->bindValue(':id', $donnees["id"], PDO::PARAM_STR);
  $querya->execute();

  $queryb = $db->prepare('SELECT * FROM quetejoueur WHERE id_joueur=:id AND id_queteorij=1');
  $queryb->bindValue(':id', $donnees["id"], PDO::PARAM_STR);
  $queryb->execute();

while ( $findquetes = $querya->fetch() ) {
    if ($queryb->fetchColumn() > 0) {
      if ($findquetes["comp_quetej"] < 1) {
          ?><button disabled class="pomegranate-flat-button" style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;"> Quest - Quest 1 (Ongoing) </button><?php
      }

    if ($findquetes["comp_quetej"] > 0) {
      if ($findquetes["recomp_quetej"] < 1) {
          echo '<form action="" method="POST"><input type="submit" class="peter-river-flat-button" style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;" value="Quest - Quest 1 (Confirm the quest)" name="confirmerquete"></form>';
      }

      if ($findquetes["recomp_quetej"] > 0) {
        echo'<button class="emerald-flat-button" style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;">Quest- Quest1 (Finished) </button>';
      }
    }

    }
    else {
      ?><button class="peter-river-flat-button" style="border-radius:6px; padding-left:45px; padding-right:45px;" onclick="window.location.href='../quete/vigier.php'">Quête - Le Vigier (Disponible) </button><?php
    }

}

$querya->CloseCursor();
$queryb->CloseCursor();

  if ((isset($_POST["confirmerquete"])) AND ($findquetes["recomp_quetej"] < 1)) {
    $querya=$db->prepare('UPDATE users SET exp= (exp + :queteexp) WHERE username=:username');
    $querya->bindValue(':queteexp', $findquetes["xp_quetej"], PDO::PARAM_INT);
    $querya->bindValue(':username', $username, PDO::PARAM_STR);
    $querya->execute();
    $querya->CloseCursor();

    $queryb=$db->prepare('UPDATE users SET money=(money + :moneyquest) WHERE username=:username');
    $queryb->bindValue(':moneyquest', $findquetes["or_quetej"], PDO::PARAM_INT);
    $queryb->bindValue(':username', $username, PDO::PARAM_STR);
    $queryb->execute();
    $queryb->CloseCursor();

    $query=$db->prepare('UPDATE quetejoueur SET recomp_quetej=1 WHERE id_quetej = :idqj');
    $query->bindParam(':idqj', $findquetes['id_quetej'], PDO::PARAM_INT);
    $query->execute();
    $query->CloseCursor();
  }

?>

If you need further explainations, just tell me. I'm sorry if this code is bad written, as I said this is only a beginner's work.

I don't understand the need for two queries.  They seem to be retrieving the same exact result set, so why query twice?

Your code is difficult to read because of the inconsistent indentation and the inconsistent way in which you render your HTML output (sometimes dropping out of PHP, sometimes echoing out of PHP, for more or less that same type of use case).
 I would also encourage you to get better with overall style consistency.  Variable names are not very meaningful (querya/queryb). Spacing around operators is inconsistent. Spacing around parenthesis is inconsistent.  Use an IDE that allows you to enforce a styleguide and stick with it.

The nesting in your while loop is a bit out of control. You should think about inverting conditionals and/or using continue to clean up these nested conditionals.  Consider how much easier it is to read code like this:

<?php
while ( $findquetes = $querya->fetch() ) {
    if ($queryb->fetchColumn() <= 0) {
?>
<button
    class="peter-river-flat-button"
    style="border-radius:6px; padding-left:45px; padding-right:45px;"
    onclick="window.location.href='../quete/vigier.php'"
>Quête - Le Vigier (Disponible)</button>
<?php
        continue;
    }
    if ($findquetes["comp_quetej"] <= 0) {
        continue;
    }
    if ($findquetes["comp_quetej"] < 1) {
?>
<button disabled class="pomegranate-flat-button"
    style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;">Quest - Quest 1 (Ongoing)</button>
<?php
        continue;
    }
    if($findquetes["recomp_quetej"] <= 0) {
        continue;
    }
    if ($findquetes["recomp_quetej"] < 1) {
?>
<form action="" method="POST">
    <input type="submit" class="peter-river-flat-button"
        style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;"
        value="Quest - Quest 1 (Confirm the quest)" name="confirmerquete"></form>
<?php
        continue;
    }
?>
<button class="emerald-flat-button"
    style=" border-radius:6px; font-size:15px; padding-left:45px; padding-right:45px;">Quest- Quest1 (Finished)</button>';
<?php
}

Now, the code path is much more clear, and having done your conditionals this way, you can clearly see what might be problematic conditional logic in your current approach (what happens with certain values between 0 and 1?). What I have shown above obviously still needs to be cleaned up to the point where the design intent is made clear.  The general philosophical approach should be to make quick exits from functions/loops, etc. to enable you to de-nest your happy path code.  Also note that there are very few times where you really need an else condition.  If you have written an else, you may want to revisit your condition to see if there is a better way.

Note that you should also de-nest your very last conditional.  You have ~15 lines of code inside a conditional when it doesn't need to be.

Why so much inline styling?  If you have class names being used, why are these styles not attached to the classes in external CSS? This certainly makes code harder to read the way you have it. This along with perhaps getting to a better approach for templating your display code would server you well in getting to a higher quality code base.

onclick="window.location.href='../quete/vigier.php'"

Do you really need this in javascript as opposed to setting form's action or using a simple <a> styled similarly to button?

If you really need to apply javascript here, consider moving away from in-line usage.  Over time you will begin to realize that separation of application display and behavior code allows for a tremendous amount of code re-use.  Consider putting this in document.ready handler and applying it to element based on class or id.

You should be able to combine you to update queries against users table into a single query.

I don't know what DB class you are using, but it is unclear how you handle unexpected database interactions.  What happens is prepare fails? What happens if execution fails?  Are there any possible exceptions you should be catching or error handling that should be in place?  You code seem to really only consider "happy path".

I find it odd to have all you POST logic crammed way down at the bottom of the script after the point where you have already rendered output to the browser. Ideally, if you have a script which have different GET and POST logic, you would handle all the database interactions up front and make it much clearer earlier in the script whether the code is relevant to GET or POST.

Why would $findquetes be part of the conditional criteria for this POST logic?  This variable may or may not even be populated with a record.


Defining book models in GAE I'm learning to work in GAE. I'm so used to SQL, but transform my way of think the last 20 years to NoSQL is a little hard for me.

I have the next simple structure:

BOOKS than can have CHAPTERS
CHAPTERS that can have VOTES

In a traditional SQL I just make foreign keys from VOTES to CHAPTERS, and from it to BOOKS.

I do this for my models in Datastore:

class Book(db.Model):
    title = db.StringProperty(required=True)
    author = db.StringProperty(required=True)
    created = db.DateTimeProperty(auto_now_add=True)

    # Get all the Chapters for a book
    def getChapters(self):
        key = self.chapterMemkey()
        chapters = memcache.get(key)
        if chapters is None:
            logging.info('DB access for key %s.', key)
            chapters = Chapter.all().ancestor(self).order("number").fetch(100)
            if not memcache.set(key, chapters, 300):
                logging.error('Memcache set failed for Chapters.')
        else:
            logging.info('Memcache for key %s.', key)
        return chapters

class Chapter(db.Model):
    """ All chapters that a book have """
    title = db.StringProperty(required=True)
    number = db.IntegerProperty(default=1)
    created = db.DateTimeProperty(auto_now_add=True)

    book = db.ReferenceProperty(Book,
                                required=True,
                                collection_name='chapters')

    # Search by Book (parent)
    @classmethod
    def byBook(cls, book, limit=100):
        chapter = book.getChapters()
        return chapter

    # Search by id
    @classmethod
    def byId(cls, id, book):
        return Chapter.get_by_id(long(id), parent=book)

class Vote(db.Model):
    """ All votes that a book-chapter have """
    chapterNumber = db.IntegerProperty(required=True)
    option = db.IntegerProperty(default=1)
    value = db.IntegerProperty(default=1)

    book = db.ReferenceProperty(Book,
                                required=True,
                                collection_name='book_votes')
    chapter = db.ReferenceProperty(Chapter,
                                   required=True,
                                   collection_name='chapter_votes')

    # --------------------------
    # ClassMethods for the class
    # --------------------------

    # Search by Book (parent)
    @classmethod
    def byBook(cls, book, limit=100):
        vote = book.getVotes()
        return vote

    # Search by id
    @classmethod
    def byId(cls, id, book):
        return Vote.get_by_id(long(id), parent=book)

Well, my doubts are:

Is this approach correct?
Must I define in the Vote class a reference for a book and for a chapter, as it was a foreign keys (just like I think I've done)?
Is well defined the way to retrieve the chapters from a book? I mean, in the Chapter class the function byBook uses a function from the Book class.
In the Vote class, I also define the byBook function but, must I define another byChapter function with similar behaviour than the myBook function? I mean something like that:

@classmethod
def byChapter(cls, chapter, limit=100):
    vote = chapter.getVotes()
    return vote

Are the parents well-defined for Votes (book) or must Chapter instead be a Book? The votes are for the chapters, but a chapter belongs to book.
Which are the right ways to get the sum of all the votes for a specific chapter and for specific book?

This depends on a few things (keep reading).
Not necessarily. This is a question of optimization. You could fetch a vote's book by using Chapter as an intermediary (vote.getChapter().getBook()), but this would obviously require sequential trips to the database - one to get the vote, followed by one to get the chapter, and finally one to get the book. Do you plan on fetching a book for a vote frequently? If so, save some resources and stick with what you've got.
From a dependency standpoint, this seems counterintuitive, but since a chapter cannot exist without a book (can it?), I would say this is fine and simply a matter of preference. This always seems awkward to me as well, so maybe someone else can provide more info.
You can, again, this is a question of optimization and convenience. When you have questions like this, ask yourself, "will I really need this?". If the answer is no, or you're unsure, leave it out; you can always add it later if your needs change (the relations are setup correctly to do the joins, right?).
See above.
I would provide a Chapter.getVotesSum() and/or Book.getVotesSum(). These methods would include GQL queries to aggregate the appropriate rows from the votes table into a sum.

(Sorry if I'm missing something, I don't use GAE)


Echoing back input in uppercase over a socket with Hy, a Python Lisp dialect I think I'd like to learn Clojure eventually, but at the moment am having fun with Hy, a "dialect of Lisp that's embedded in Python."

(import socket)

(defn echo-upper (sock)
  (.send c (.upper (.recv c 10000)))
  (.close c)
)

(def s socket.socket)
(.bind s (tuple ["" 8000]))
(.listen s 5)
(while True
  (echo-upper (.__getitem__ (s.accept) 0))
)

Python generated from ABS with astor:

import socket

def echo_upper(sock):
    c.send(c.recv(10000).upper())
    return c.close()
s = socket.socket
s.bind(tuple([u'', 8000]))
s.listen(5)
while True:
    echo_upper(s.accept().__getitem__(0))

I'm not worried about not dealing with connections properly etc, I'd just like feedback on using lispy syntax. I'm tempted to do things like ((getattr s "bind") (tuple ["" 8000])) because it looks more like simple Scheme, the only lisp I have a little experience with.

Any thoughts on syntax and style? Any guidelines for dealing with Python APIs in a lispy way, something that apparently happens a lot in Clojure with Java?

I wouldn't use the threading macro here. That's most useful when you want to see the data flow, but you're using c two times in this expression, there's no clear flow that -> could make easier to untangle. It makes it more complicated in this case...

If you'd decouple the receive and the send part, then it'd be different, but then the expression becomes so simple you don't even need the macro:

(let [[received (.recv c 10000)]]
  (.send c (.upper received)))

With the threading macro, the inner part would become (-> received (.upper) (.send c)) - doesn't make it much clearer, either.

What you could do, however, is abstract away the accept -> do something -> close thing into a macro:

(defmacro with-accepted-connection [listening-socket & rest]
  (quasiquote (let [[*sock* (get (.accept (unquote listening-socket)) 0)]]
                (unquote-splice rest)
                (.close *sock*))))

(with-accepted-connection s (.send *sock* (upper (.recv *sock* 10000))))

The above may or may not work out of the box, I just conjured it up. But something similar may make the code lispier and easier to understand the intent later.


Getting products by category, price, etc I have the following concerns:

Is Set a good choice for getting products? What are drawbacks of Set for this purpose?
Is it better to do something like common.retainAll() or to get products through a request with WHERE to the database. For now I do three requests in: getProductsByCategory, getProductsByManufacturer, getProductsByPrice. So maybe it would be better to make just one?

@RequestMapping("/{category}/{price}")
public String filterProducts(@PathVariable("category") String productCategory,
        @MatrixVariable(pathVar = "price") Map<String, List<String>> priceParams,
        @RequestParam("manufacturer") String manufacturer, Model model) {

    Set<Product> productsByCategory = service.getProductsByCategory(productCategory);
    Set<Product> productsByManufacturer = service.getProductsByManufacturer(manufacturer);

    BigDecimal low;
    BigDecimal high;

    low = new BigDecimal(priceParams.get("low").get(0));
    System.out.println(low);
    high = new BigDecimal(priceParams.get("high").get(0));
    System.out.println(high);

    Set<Product> productsByPrice = service.getProductsByPrice(low, high);

    Set<Product> common = new HashSet<Product>(productsByCategory);

    common.retainAll(productsByManufacturer);
    common.retainAll(productsByPrice);

    model.addAttribute("products", common);

    return "products";

}

OK, so I assume this is the code from your @Controller, there's a @Service that it calls, and the Service further calls a @Repository (the DAO layer). Furthermore, I shall assume you're using the latest version of Java, i.e. Java 8.
Ultimately I find it all depends on the number of products you have. Given not too many, you could do all the filtering in Java. Given thousands, you'd better do it in the database.
One method in the database layer
In any case, one database call is definitely much better than three, so you could just create a generic enough database search method to which you can pass all the search parameters. When you create the method, you could also create a data transfer object named, say, SearchParameters that contains all the search parameters. That way your method signatures don't grow to ridiculous lengths even when you search with several parameters. The interface will also be easy to change, because in order to add a new search parameter the method signature won't have to change, just the data transfer object.
Personal experience
The way I've solved a similar issue, with just a couple of dozen products, was that my DAO layer had a trivial public List<Product> findAll() method with a @Cacheable annotation so that after the first call to the DB the further ones would only access a fast cache in the memory.
The Service would then have the methods you described: getProductsByCategory, getProductsByManufacturer and getProductsByPrice, but they'd all just call the findAll() method of the DAO (quickly getting the cached response) and they'd filter the results based on their parameters.
For example (assuming the Product class has a getManufacturer() method that returns a String:
public Set<Product> getProductsByManufacturer(final String manufacturer) {
    return dao.findAll().stream()
        .filter(product -> Objects.equals(product.getManufacturer(), manufacturer)
        .collect(Collectors.toSet());
}

And that's it using Java 8's streams. The filter only includes those products whose manufacturer matches the given parameter.
One method in the service layer
However, since you need an intersection of three sets, you might as well create one method to do all of your filtering. Either to the DAO layer like I proposed in the beginning, or do it in your service layer if the amount of data is not too big, or databases aren't your strong suit. ;)
So, your business code would boil down to something like this:
New SearchParameters class:
import java.util.Optional;

public class SearchParameters {
    // Here with just one search parameter for brevity, but you'll get the idea
    String manufacturer;

    public boolean manufacturerMatches(final Product product) {
        return Optional.ofNullable(manufacturer).map(m -> m.equals(product.getManufacturer())).orElse(false);
    }
}

A method in your Service class:
public Set<Product> getProducts(final SearchParameters params) {
    return dao.findAll().stream()
        .filter(params::manufacturerMatches)
        .collect(Collectors.toSet());
}

Then just add more of those matcher methods to the SearchParameters class and deploy them as filter lines in your Service -- you can chain filters, so it's easy and really concise.
In case you're not familiar with Java 8's streams, .filter(params::manufacturerMatches) is a short-hand notation for .filter(product -> params.manufacturerMatches(product)), which in turn compares all the products against the manufacturerMatches method and only allows a product to go through if the method returns true.
Answers
So, to answer your questions...

Sets are basically good for holding those Products of yours in case you are not interested in the order of the products. If that matters, you need to use some List instead. But that's not really what you asked or wanted to know, so:
The issue with Sets in your code is that you duplicate or triplicate a lot of data and operate on many more objects than you need to in order to achieve your objective. Comparing big product objects is also more expensive (in terms of CPU cycles) than simple Strings or other such parameters, which would be enough for you. You could have all of the objects (Products) in one collection and just filter that. Also when you compare your objects in Sets, you need to be sure your equals and hashCode methods for the Products are always up to date -- if you forget to update them when you add a new field, you will get wrong results.

Yes, it's best to do just one DB call -- and even that one call can sometimes be omitted by using a cache. You can achieve that "one call only" by creating a bit more complicated DB query to account for all the search parameters or by handling the searching (filtering) in Java. Which way should you do it, depends on the factors I've listed above, i.e. mainly about the number of products in your database and your personal preference. But I wouldn't really use the Sets, as (like I said) they multiply the amount of objects you have, and also because I just don't find that .retainAll(...) kind of code very readable.


Processing a string from the console How can I make this program run faster?  On enter, I have a string from the console.  For example:

1 1 7 3 2 0 0 4 5 5 6 2 1

On exit, it will be

6 20

Could you recommend a way to make this run faster?

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.ArrayList;
import java.util.StringTokenizer;
public class Main {
    public static void main(String[] args) throws IOException {
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        String line;
        ArrayList<Integer> _arr = new ArrayList<Integer>();
        while((line=br.readLine())!= null){
                StringTokenizer tok = new  StringTokenizer(line);
                while (tok.hasMoreTokens()) {
                    _arr.add((Integer.parseInt(tok.nextToken())));
                }
            }
        Algorithm(_arr);
    }
    private static void Algorithm(ArrayList<Integer> arr) {
        int ascLength = 1,
            descLength = 1,
            ascSequenceStart = 0,
            descSequenceStart = 0,
            ascLongest = 1,
            descLongest = 1,
            summa = 0,
            tmp1, tmp2;
        for (int i = 1; i<arr.size(); i++) {
            tmp1 = arr.get(i-1);
            tmp2 = arr.get(i);
            if(tmp2>tmp1){
                ascLength++;
                descLength = 1;
            } else if (tmp2<tmp1) {
                descLength++;
                ascLength = 1;
            } else if (tmp1==tmp2){
                descLength++;
                ascLength++;
            }
            if (ascLength > ascLongest) {
                ascLongest = ascLength;
                ascSequenceStart = i - ascLength + 1;
            }
            if (descLength > descLongest) {
                descLongest = descLength;
                descSequenceStart = i - descLength + 1;
            }
        }
        if (ascLongest > descLongest) {
            for (int i = ascSequenceStart; i < (ascSequenceStart+ascLongest); i++) {
                summa += arr.get(i);
            }
            System.out.print(String.valueOf(ascLongest)+" "+String.valueOf(summa));
        } else {
             for (int i = descSequenceStart; i < (descSequenceStart+descLongest); i++) {
                 summa += arr.get(i);
             }
             System.out.print(String.valueOf(descLongest)+" "+String.valueOf(summa));
        }
    }
}

Yeah, without giving too much away (this is obviously homework)....

First though, the assumption I make is that the assignment is:

find the length of the longest series of either increasing, or decreasing numbers
sum the values in this longest sequence.

There are two parts to this problem as well. The first part is getting the user input, and the second is identifying and processing the sequences. I will assume that you can process the user input adequately, and put it in an array of integer int[] values = .... You use an ArrayList<Integer> which is not, in my opinion, a great container for doing arithmetic on...

This problem can be solved by managing three variables, and keeping a maxlen and maxsum total.

First you need to ensure you have at least 2 items in your values array, and, assuming you do, you initialize your three variables:

boolean increasing = values[0] < values[1];
int sum = values[0];
int len = 1;

int maxlen = len;
int maxsum = sum;

OK, we set up the system where we expect the third value to be a continuation of the sequence.... and then we start the loop, starting from position 1 (the second item) in the values array:

for (int i = 1; i <= arrays.length; i++) {
    // check here to see if we are going in the same direction
    if ( (increasing && value[i] >= value[i -1]) || ( ...decreasing and next is smaller) ) {
        // we are going on in the same direction as before:
        // increase the length, and add value[i] to the sum.
        // if the length is larger than maxlen, then change maxlen and maxsum to len and sum
    } else {
        // the value at position i breaks the sequence.
        // reset len and sum to be 2 and the sum of this and the previous value

        // EDIT: 'Obviously' you record the change in direction!
        increasing = !increasing;

    }
}

System.out.printf("The longest sequence is %d values long, and has a sum of %d\n", maxlen, maxsum);

Without going in to too much detail, you can easily convert this algorithm to work at the same time as you process the user input.... there is no real reason why you have to first convert all the values to int first, and only then process them.... you can process them as they are entered.

EDIT: I realize you may need to be smarter than I have suggested when setting up the variables initially... if the first values in the input are all the same then the algorithm above will assume the sequence is not increasing and will produce the wrong result. You need to find out the initial 'direction' of the sequence by looking for the first 'different' value


Bats Challenge from CodeEval.com I am going through coding challenges and I would like to get feedback on one of them.
I am not sure if i can post the link to the challenge but here it is.  CodeEval Challenge

  Outside of your window, there's a wire between two
  buildings. Bats love to hang there but you notice they never hang
  closer than "d" centimeters from each other. They also don't hang
  closer than 6 centimeters from any of the buildings.

  Your goal is to determine the maximum number of additional bats that
  can fit on that wire assuming they have zero width.

  INPUT SAMPLE:

  Each line of input contains three space separated integers: the length of
  the wire "l", distance "d" and number of bats "n" already hanging on
  the wire.

  "n" numbers contain the positions of the bats in any order. All number
  are integers. You can assume that the bats already hanging on the wire
  are at least 6 cm from the poles and at least "d" centimeters apart
  from each other.:

  22 2 2 9 11
  33 5 0
  16 3 2 6 10
  835 125 1 113
  47 5 0

  Output
  3
  5
  0
  5
  8

Here is my solution.  I commented it a bit.  I had to convert the method to C to get it passed through the automated system.  Any suggestions to improve it would be greatly appreciated.

#define CENTIMETERS_FROM_WALL 6

int howManyBatsBetween(int pointOne, int pointTwo, int distanceBetween)
{
    //calculates how many bats you can put between two bats already on the wire
    //Have to make sure that it would be proper distance between the first bat and second bat
    int nextBatPosition = pointOne+distanceBetween;
    int batCount = 0;
    while ((nextBatPosition+distanceBetween) <= pointTwo) {
        batCount++;
        nextBatPosition+=distanceBetween;
    }
    return batCount;
}

-(void)batCount
{
    NSString *line = @"47 5 0"; //input
    //extract input
    NSArray *inputArray = [line componentsSeparatedByString:@" "];
    int lengthWire = [[inputArray objectAtIndex:0] intValue];
    int distanceFromEachOther = [[inputArray objectAtIndex:1] intValue];
    int batsOnWire = [[inputArray objectAtIndex:2] intValue];

    //create an array with bats already on the wire
    NSMutableArray *batsOnWireArray = [[NSMutableArray alloc]init];
    for (int indx = 1; indx <= batsOnWire; indx ++){
        [batsOnWireArray addObject:[inputArray objectAtIndex:indx+2]];
    }
    [batsOnWireArray sortedArrayUsingSelector:@selector(compare:)];
    int firstLocation = 0;
    int secondLocation = 0;
    int additionalBats = 0;

    if ([batsOnWireArray count] == 0) {
        //if there are no bats on the wire
        additionalBats = floor((lengthWire-(CENTIMETERS_FROM_WALL*2))/distanceFromEachOther)+1;
    }else{
        //calculate how many bats can be added between the beginig of the wire + 6cm and the first bat
        additionalBats = floor(([[batsOnWireArray objectAtIndex:0] integerValue]-CENTIMETERS_FROM_WALL)/distanceFromEachOther);

        for(int indx = 0; indx < [batsOnWireArray count]; indx++){
            firstLocation =[[batsOnWireArray objectAtIndex:indx] intValue];
            //calculate how many bats you can add between bats already on the wire
            if (indx < [batsOnWireArray count]-1) {
                secondLocation = [[batsOnWireArray objectAtIndex:indx+1] intValue];
                additionalBats += howManyBatsBetween(firstLocation, secondLocation, distanceFromEachOther);
            }else{
                //calculate how bats you can fit between the last bat and the end of the wire - 6cm
                additionalBats += floor(((lengthWire - CENTIMETERS_FROM_WALL) - firstLocation) / distanceFromEachOther);
            }

    }

    }

    NSLog(@"Result %d", additionalBats);
}

You posted this question with the objective-c tag, and made this comment:

  I had to convert the method to C to get it passed through the automated system.

As such, I'm going to assume you're perhaps more interested in a review of the Objective-C stuff, so that's what this'll be.  (Essentially, I'm ignoring the howManyBatsBetween function for this.)

As a start, the first problem I notice is that our method takes no arguments and returns nothing.  The problem statement gives sample inputs in the form of strings and expects an integer return.  Our method should be changed to reflect this:

- (NSInteger)batsOnWire:(NSString *)wire;

Now we can remove the hardcoded // input.

componentsSeparatedByString: is a good approach at breaking up the string, but we also need to make sure our input is in a good format.

NSArray *batPositions = [wire componentsSeparatedByString:@" "];

if (batPositions.count < 3) {
    return NSNotFound;
}

We'll use NSNotFound as a return result representing some sort of problem with the input.  Truly we should actually take an NSError ** as an argument, but I won't get into that mess.  I'll keep it simple for now.  The point here is, we're dealing with inputs, either typed by a user or read from a file... and inputs always have a chance of having errors, so we have to deal with that.

Grabbing lengthWire, distanceFromEachOther, and batsOnWire as you do is fine.

But again, we are dealing with user input.  The problem with calling intValue is that if the string in question can't be parsed as an int, the method just returns 0.  It's impossible to tell the difference between @"0" and @"Hello World" when looking at what they return intValue.

[@"0" intValue] == [@"Hello World" intValue]

Instead, we can convert these strings to NSNumber objects... which we'll want to do later for another reason which I'll address, but here's how we do it:

NSNumberFormatter *formatter = [[NSNumberFormatter alloc] init];
formatter.numberStyle = NSNumberFormatterDecimalStyle;

NSNumber lengthWire = [formatter numberFromString: batPositions[0]];
if (!lengthWire) {
    return NSNotFound;
}

NSNumberFormatter's numberFromString: method returns nil if the string cannot be parsed to a number.  Otherwise, it returns an NSNumber object representing the value.  Once we do the if(!lengthWire) check, we know we have a valid number.  We can always extract the raw int value by calling the same intValue on this NSNumber object.

But there's another reason we're going to want the entire array of strings converted into an array of NSNumber objects.

Given the following mutable array of strings:

@[@"1", @"2", @"40", @"10", @"300", @"100", @"9"];

If we sort if using sortedArrayUsingSelector:@selector(compare:)];, the result would be an array of this order:

@[@"1", @"10", @"100", @"2", @"300", @"40", @"9"];

And this isn't the right order.  The compare: method of NSString compares the strings character by character.

If we have to strings we want to compare individually, we can call another method:

[@"2" compare:@"100" options:NSNumericSearch];

And we'll get the correct numeric result (2 would be sorted before 100 correctly).

But when using sortedArrayUsingSelector:, we can't use a method that takes other arguments.  We can't specify that our strings our compared numerically.

However, if they are NSNumber objects, they'll of course be compared as numbers and sorted appropriately.  So if we turn our array of strings into an array of NSNumber objects, and then call the same:

[arrayOfNumberObjects sortedArrayUsingSelector:@selector(compare:)];

We'll get our array sorted in the proper order.

And once again, NSNumber responds to the intValue method in exactly the same way NSString does, so we can still use this to get a workable int out of the object.

#define CENTIMETERS_FROM_WALL 6

As one final note about Objective-C, we don't really care for #define variables.  I can't discourage it enough.  In fact, Apple agrees with me.  As evidence, you can't even use the #define functionality in their new language, Swift, at all.

Instead, we should opt for a typed constant:

static const int kCentimetersFromWall = 6;


Palindrome Checker with Stack In my answer to this question, I wrote a new solution.  Personally, I would not use a stack, but given that a stack was required, how good is this solution?

In this solution, I add half of the word, not including the middle character if applicable, to the stack and remove it from the string.  If there is a middle character in the total string, the sizes of the string and the stack are not equal, and the middle character is at the beginning of the string, making it easy to delete (we don't need to compare it to itself, do we?).  Then, I iterate through the stack, comparing the top of the stack to the beginning of the string; if they are equal, I remove the character from the stack and the string, and continue; otherwise, I return false;.  If there are no characters left in the stack, I return true;.

bool isPalindrome(string word) {

    stack<char> s;
    for (int i = 0; i <= word.size() / 2; i++) {
        s.push(word[0]);
        word.erase(0, 1);
    }

    if (word.size() > s.size()) {
        word.erase(0, 1);
    }

    if (s.size() == 0) {
        return true;
    }

    while (s.size() > 0) {

        if (s.top() == word[0]) {
            s.pop();
            word.erase(0, 1);

            // return true only when our stack is empty
            if (s.size() == 0 || word.size() == 0) {
                return true;
            }
        }
        else {
            return false;
        }
    }
}

Your logic is incorrect in a few places.  The first problem is that you are modifying word inside the loop so you probably want to cache the size before you use it in the loop indices.  This is one of those insidious bugs.

int size = word.size();

for (int i = 0; i <= size / 2; i++) {
    s.push(word[0]);
    word.erase(0, 1);
}

if (word.size() > s.size()) {
    word.erase(0, 1);
}

I'm going to assume from here on that the cached size code above is what you intended the behavior of this program to be.  Moving on to the next problem.  Consider when word.size() is even, say 8.  i will iterate from 0 to 4 inclusive.  So s will have 5 characters and word will have 3.  If word.size() was 9 then i would iterate from 0 to 4 as well, leaving word with 4 characters.  Hence, after the loop, word.size() will never be greater than s.size().  In trying to handle the odd case you produced a bug in the even case.  The loop bound you are looking for is:

for (int i = 0; i < size / 2; i++)

Here is the updated code for the first half:

int size = word.size();

for (int i = 0; i < size / 2; i++) {
    s.push(word[0]);
    word.erase(0, 1);
}

if (word.size() > s.size()) {
    word.erase(0, 1);
}

Now you can verify that word.size() and s.size() will always be the same at this point.  This means that inside the while loop you can remove the following check since s.size() and word.size() will both reach 0 at the same time:

if (s.size() == 0 || word.size() == 0) {
    return true;
}

Now the only way you can exit the loop is if all the letters on the stack and in the second half of the word were indeed the same.  Which of course means that word is a palindrome.  So just put a return true at the end.

You can now remove the following check before the while loop too since in this case the loop won't run and you will jump down to return true:

if (s.size() == 0) {
    return true;
}

With all of that said I think the code is a bit too complex.  There is no need to modify word.  With some integer arithmetic you can use two loops to iterate through word without deleting anything.

I will print out the loop indices for three looping methods you can use.

Method 1:

int i;
for(i = 0; i < n/2; i++)s.push(word[i]);
if(n % 2) i++; // skip the middle index if n is odd
for(; i < n; i++)  compare_stuff();

Method 2:

for(int i = 0; i < n/2; i++)s.push(word[i]);
for(int i = (n+1)/2; i < n; i++) compare_stuff(); // skip the middle index if n is odd

Method 3:

for(int i = 0; i < (n+1)/2; i++)s.push(word[i]);
for(int i = n/2; i < n; i++) compare_stuff();  // check the middle index against itself if n is odd

Method 2 is probably the best for this problem since it allows for local loop indices and doesn't check the middle value unnecessarily.

My code would be:

bool isPalindrome(const std::string& word)
{
    uint n = word.size();
    std::stack<char> s;

    for(uint i = 0; i < n/2; i++)
    {
        s.push(word[i]);
    }

    for(uint i = (n+1)/2; i < n; i++)
    {
        if(s.top() != word[i])
        {
            return false;
        }

        s.pop();
    }

    return true;
}


Generate N pairs of checkboxes and N textboxes I've written a piece of code, where a pair of checkboxes and a textbox are generated N amount of times. Depending on what's in the DB, the checkbox/textbox may have a default value or are left empty.

I've pasted the code below, could anyone take a look to see if I haven't created an abomination?

    @if (singleItem != null && singleItem.FolderRead)
    {
        <input type="checkbox" id="canRead_@item.SecureFolderID" class="CHKreader" onchange="changePermissions(@item.SecureFolderID)" checked />
    }
    else
    {
        <input type="checkbox" id="canRead_@item.SecureFolderID" class="CHKreader" onchange="changePermissions(@item.SecureFolderID)" />
    }

    @if (singleItem != null && singleItem.FolderWrite)
    {
        <input type="checkbox" id="canWrite_@item.SecureFolderID" class="CHKwriter" onchange="changePermissions(@item.SecureFolderID)" checked />
    }
    else
    {
        <input type="checkbox" id="canWrite_@item.SecureFolderID" class="CHKwriter" onchange="changePermissions(@item.SecureFolderID)" />
    }

    @if (singleItem != null && singleItem.ExpireDate != null)
    {
        <input class="datepick" id="expDate-@item.SecureFolderID" onchange="changePermissions(@item.SecureFolderID)" value="@singleItem.ExpireDate.Value.ToString("yyyy-MM-dd")" />
    }
    else
    {
        <input class="datepick" id="expDate-@item.SecureFolderID" onchange="changePermissions(@item.SecureFolderID)" />
    }

I know the copy/paste code is not that clean. The textbox could be filled in with a variabele, which could remain empty if the DB record is as well. But I don't know if that makes the code more 'readable' or if such a method works for the Checkbox as well.

I would use the razor boolean attributes:

  <input type="checkbox" id="canRead_@item.SecureFolderID" class="CHKreader" onchange="changePermissions(@item.SecureFolderID)" checked="@(singleItem != null && singleItem.FolderRead)" />

    <input type="checkbox" id="canWrite_@item.SecureFolderID" class="CHKwriter" onchange="changePermissions(@item.SecureFolderID)" checked="@(singleItem != null && singleItem.FolderWrite)" />

    <input class="datepick" id="expDate_@item.SecureFolderID" onchange="changePermissions(@item.SecureFolderID)" value="@((singleItem != null && singleItem.ExpireDate != null)?singleItem.ExpireDate.Value.ToString("yyyy-MM-dd"):null)" />

Going further, if item/singleitem are coming from an Enumerable, there is probably a better way to generate the Ids as well.  Especially if you want them to post back into a nice object.  Remove the classes, remove the onchange handler.

@for(var i=0;i<Model.Count;i++)
{

      @Html.CheckBoxFor(m=>m[i].FolderRead)

      @Html.CheckBoxFor(m=>m[i].FolderWrite)

      @Html.TextBoxFor(m=>m[i].ExpireDate,new{type="date"}) // HTML5 date w/datepicker for browsers that support it

}

You could also use an EditorFor on the date and create your own template if you prefer that route.