Split a character string based on change of character I am contributing a solution to the following task on Rosetta Code:

  Task
  Split a string into comma (plus a blank) delimited strings based on a change of character (left to right).

  Show the output here (use the 1st example below).

  Blanks should be treated as any other character (except they are
  problematic to display clearly). The same applies to commas.

  For instance, the string:

    gHHH5YY++///\

  should be split and show:

    g, HHH, 5, YY, ++, ///, \

Here is the code:

// Solution for http://rosettacode.org/wiki/Split_a_character_string_based_on_change_of_character
#include<string>
#include<iostream>
auto spliter(const std::string &input) {
   auto firstCommaPast = false;
   std::string res ="";
   auto prev = '\0';
   for(auto it = input.cbegin(); it != input.cend();++it) {
      if(*it!=prev) {
         if(!firstCommaPast) {
            firstCommaPast = true;
         } else {
            res+=", ";
         }
      }
      res+=*it;
      prev=*it;
   }
   return res;
}

int main() {
   auto input = R"(gHHH5  ))YY++,,,///\)";
   std::cout<<spliter(input);
}

I am fairly new to C++11/14 so any suggestions here will really help.

Algorithm

At least at first glance, your algorithm appears to be unnecessarily complex. I'm not entirely sure what firstCommaPast is intended to accomplish. It seems to be trying to give some sort of special treatment to commas, even though the directions explicitly state that commas should be treated exactly like any other character.

Whitespace

I prefer to see a blank line between the #includes and other code. I also prefer to see a space between the include and the < or " that follows it:

#include <string>
#include <iostream>

auto spliter(const std::string &input) {

These don't make any difference to functionality, but fit pretty well with widely accepted practice. I won't try to comment on whether the opening curly brace ({) belongs on the same line as the function header, or on the next line by itself. That's widely debated, and I don't think there's any real consensus on which is preferred.

Initialization

Given that it's going to be an empty string either way, I'd rather use std::string res; rather than std::string res="";. The latter will typically add a little extra work without accomplishing anything useful.

Iterators vs. indexing

The primary reason for using iterators is to support generic algorithms. Using them for a loop outside of a generic algorithm is somewhat questionable. In this case, I see no real improvement over just using an integer type and indexing into the string.

Logic Error

Right now, your code assumes that the first character of a string can't be a '\0'. While that's probably at least somewhat unusual, unlike with C strings, it is actually allowed/supported in a std::string (and if it's present, your code won't really work as intended).

Personally, I think it's easier to initialize prev to the first character of the input string, then have the loop proceed from input[1] through the end of the string. This way we don't have to initialize prev to a value that can't be in the string (which is good, because there is no such character).

Function Naming

A function does something. As such, its name should normally be a verb, not a noun. Although it may be open to argument whether the best name for what it does is splitting, let's just go with that general idea--in which case its name should be split, not splitter.

Resulting Code

Incorporating the elements above, the code could end up something like this:

#include <string>
#include <iostream>

auto split(const std::string &input) {
    auto prev = input[0];
    std::string res{ prev };

    for (size_t i = 1; i < input.length(); i++) {
        if (input[i] != prev)
            res += ", ";
        prev = input[i];
        res += prev;
    }
    return res;
}

int main() {
    auto input = R"(gHHH5  ))YY++,,,///\)";
    std::cout << split(input);
}

Looks perfect.
You have the input string hardcoded here, is there a possibility of having a new line character in your input (if input is given in run-time).??


Simple Game Engine in JavaScript I’m planning on creating a Mortal Kombat-inspired game in JavaScript. I’ve written a basic game engine that manages my game states and creates a game loop.

I just wondered if it could be improved at all? As I’m a web developer by trade and this is my first foray into programming games.

/**
 * Game engine.
 */
function GameEngine() {
    this.states = new Array();
    this.currentState;
    this.running = false;
};

GameEngine.prototype.init = function() {
    this.running = true;
};

GameEngine.prototype.changeState = function(state) {
    this.currentState = state;
};

GameEngine.prototype.isRunning = function() {
    return this.running;
};

GameEngine.prototype.handleEvents = function() {
    this.currentState.handleEvents(this);
};

GameEngine.prototype.update = function() {
    this.currentState.update(this);
};

GameEngine.prototype.draw = function() {
    this.currentState.draw(this);
};

/**
 * Base game state state.
 */
function GameState() {};

/**
 * Intro state.
 */
function IntroState() {};

IntroState.prototype = Object.create(GameState.prototype);

IntroState.prototype.handleEvents = function(game) {
    console.log('IntroState::handleEvents');
};

IntroState.prototype.update = function(game) {
    console.log('IntroState::update');
};

IntroState.prototype.draw = function(game) {
    console.log('IntroState::draw');
};

/**
 * Initialises the game.
 */
(function() {
    var game = new GameEngine();
    var gameLoop;
    var frameLength = 500;

    game.init();
    game.changeState(new IntroState());

    gameLoop = setInterval(function() {
        game.handleEvents();
        game.update();
        game.draw();
    }, frameLength);
})();

There's really not much to say about this code, so I'll quote a comment posted on this question:

  [...] a basic game engine is a loop and a redraw/update system. So yes you are good. You should explore other game engines in javascript and see what they do. – Steven

Just one thing that's itching: you're declaring var frameLength = 500; - I think this is going to bite you later, when you want to adjust frame rate (FPS).

Just a general tip in regard to the comments...

It's not recommended to use /* */ comment blocks in your code. The reason for that is because in Javascript, that character pair can occur in regular expression literals as well. Comment blocks are therefore not safe for commenting blocks of code.

A better alternative is to generally stick to line comments (//) whenever possible and save block comments for formal documentation and for commenting out.


Graph Implementation in Java using Set and Map There is a Graph implementation. I'm using Set to store unique Edges and Map to store Vertices and their associated edges. Assume Vertex and Edge class has been defined somewhere else implementing hashcode() and equals() working fine.

public class Graph
{
    static class Vertex {}

    static class Edge {}

    private Map<Vertex, Set<Edge>> vertexSetMap;
    private Set<Edge> edgeSet;

    public Graph()
    {
        edgeSet = new LinkedHashSet<>();
        vertexSetMap = new LinkedHashMap<>();
    }

    public boolean addVertex(Vertex vertex)
    {
        if(!vertexSetMap.containsKey(vertex))
        {
            vertexSetMap.put(vertex, new LinkedHashSet <>());
            return true;
        }
        return false;
    }

    public boolean addEdge(Vertex v1, Vertex v2, Edge edge)
    {
        if(edgeSet.add(edge))
        {
            vertexSetMap.get(v1).add(edge);
            vertexSetMap.get(v2).add(edge);
            return true;
        }
        return false;
    }

    public Set<Edge> getEdges(Vertex vertex)
    {
        return vertexSetMap.get(vertex);
    }

    public Set<Vertex> getNeighborsOf(Vertex vertex)
    {
        Set<Vertex> neighbors = new LinkedHashSet <>();
        for(Edge edge : vertexSetMap.get(vertex))
        {
            Vertex v1 = edge.getVertex1();
            Vertex v2 = edge.getVertex2();
            if(v1.equals(vertex)) neighbors.add(v2);
            else neighbors.add(v1);
        }
        return neighbors;
    }

    public void removeVertex(Vertex vertex)
    {
        Set<Edge> deleteEdges = getEdges(vertex);
        vertexSetMap.remove(vertex);
        edgeSet.removeAll(deleteEdges);
        vertexSetMap.keySet().forEach(vertex1 ->
        vertexSetMap.get(vertex1).removeAll(deleteEdges));
    }

    public void removeEdge(Edge edge)
    {
        edgeSet.remove(edge);
        Vertex v1 = edge.getVertex1();
        Vertex v2 = edge.getVertex2();
        vertexSetMap.get(v1).remove(edge);
        vertexSetMap.get(v2).remove(edge);
    }

    public boolean containsVertex(Vertex vertex)
    {
        return vertexSetMap.containsKey(vertex);
    }

    public boolean containsEdge(Vertex v1, Vertex v2, Edge edge)
    {
        return (edgeSet.contains(edge) && vertexSetMap.get(v1).contains(edge)
            && vertexSetMap.get(v2).contains(edge));
    }

    public boolean containsEdge(Vertex v1, Vertex v2)
    {
        for(Edge e : vertexSetMap.get(v1))
            if(vertexSetMap.get(v2).contains(e)) return true;
        return false;
    }

    public int totalVertices()
    {
        return vertexSetMap.keySet().size();
    }

    public int totalEdges()
    {
        return edgeSet.size();
    }

    public Set<Vertex> getAllVertices()
    {
        return new LinkedHashSet <>(vertexSetMap.keySet());
    }

    public Set<Edge> getAllEdges()
    {
        return edgeSet;
    }

    public void clear()
    {
        edgeSet = null;
        edgeSet = new LinkedHashSet <>();
        vertexSetMap = null;
        vertexSetMap = new LinkedHashMap <>();
    }
}

As of now, you only use LinkedHashSet and LinkedHashMap, thus declaring edgeSet and vertexMap as HashSet and HashMap is pointless. You could deploy Inversion of Control in order to demand some Set and Map at construction.

Method getEdges(...) could possibly return null, so you may want to wrap the return value in an Optional. Using Optional, you can also unify the behaviour of Map#get(Object) (which may or may not throw a NPE if a null is passed as argument):

public Optional<Set<Edge>> getEdges(Vertex vertex)
{
    Optional<Set<Edge>> result = Optional.empty();
    try
    {
        result = Optional.ofNullable(vertex);
    }
    catch (NullPointerException e)
    {
        // No action needed, we return an empty Optional
    }
    return result;
}

Your remove...(...)-methods throw a NPE if the Vertex/Edge provided is not contained within the graph or is null. As with Map#get(Object), Set#remove(Object) may or may not throw a NPE. You can rewrite them to not throw:

public void removeVertex(Vertex vertex)
{
    Optional<Set<Edge>> deleteEdges = getEdges(vertex);
    if (deleteEdges.isPresent())
    {
        vertexSetMap.remove(vertex);
        edgeSet.removeAll(deleteEdges.get());
        vertexSetMap.keySet().forEach(vertex1 ->
                vertexSetMap.get(vertex1).removeAll(deleteEdges.get()));
    }
}

public void removeEdge(Edge edge)
{
    try
    {
        if (edgeSet.remove(edge))
        {
            Vertex v1 = edge.getVertex1();
            Vertex v2 = edge.getVertex2();
            vertexSetMap.get(v1).remove(edge);
            vertexSetMap.get(v2).remove(edge);
        }
    }
    catch (NullPoionterException e)
    {
        // No action needed, there is nothing to remove.
    }
}

When you add methods

public boolean containsVertex(Vertex vertex)
public boolean getOther(Vertex vertex)

To your Edge-class, you can rewrite getNeighborsOf(...) to use Streams

public Set<Vertex> getNeighborsOf(Vertex vertex) {
    return vertexSetMap.get(vertex).stream()
          .filter(edge -> edge.contains(vertex))
          .map(edge -> edge.getOther(vertex))
          .collect(Collectors.toSet());
}

What is missing is a Set<Vertex> getVertices() method (you have one for Edges, so I would expect one for Vertexs as well). This would probably require creating an additional field Set<Vertex> vertices in your Graph class.


Fractional binary number to decimal number Write and test your own double abc (char * b) function in the program, taking in the form of a string the fractional binary number of the form "1101.101" and returning an equal decimal number (here 1 * 2 ^ 3 + 1 * 2 ^ 2 + 0 * 2 ^ 1 + 1 * 2 ^ 0 + 1 * 2 ^ -1 + 0 * 2 ^ -2 + 1 * 2 ^ -3 = 8 + 4 + 0 + 1 + 0.5 + 0 + 0.125 = 13.625). The function should return the special value -1 if an incorrect fractional binary number is entered (e.g. containing a different digit than 0, 1 or more than one period '.').

My solution:

#include <iostream>
#include <cmath>
using namespace std;
double abc(char* b);
int main() {
cout <<  abc("1101.101");
return 0;
}

double abc(char* b){
int i = 0;
int a = 0;
int dot = 0;
double sum=0;
int c = 0;
do{
    if ((b[a]=='1')||(b[a]=='0')||(b[a]=='.')){
        a++;
        if(b[a]=='.'){
            dot++;
        }
        if(dot==2){
            return -1;
        }
    } else {
        return -1;
    }
} while(b[a]!='\0');

while(b[i]!='.'){
    i++;
}
while(i!=0) {
    if(b[c]=='1'){
        sum = sum + pow (2, i-1);
    }
    c++;
    i--;
}
while(b[i]!='.'){
    i++;
}
if(b[i]=='.'){
    int m = 0;
    while(b[i]!='\0'){
        if(b[i]=='1'){
            sum = sum + pow (2, m);
        }
        m = m - 1;
        i++;
    }
}
return sum;
}

Problem:
Everything is fine, it gives me 13.625, but there is a lot of code. Is there any faster way to solve this exercise?

Here are some thoughts about your code:

Don't use using namespace std. This is considered bad practice and you can find many reasons on the internet why this is the case (for example here). You can, for example write std::cout instead.
You should use variable-names that tell the person who reads the code the purpose of the variable.
To improve the legibility of your code, you really should use indentation.
To further improve legibility, you can leave spaces between operators.
I made your code a bit shorter by avoiding redundant code. You could even make it a lot shorter still, but I wanted to maintain your code structure.
Finally, you should have a look at edge cases: Empty strings, strings that only contain "." and strings that don't have positions after/before the decimal point (for example 101 = 5 or .101 = 0.625).

The result looks like this:

#include <iostream>
#include <cmath>
#include <cstring>

double abc(char* b);

int main() {
    std::cout <<  abc(".101") << "\n";
    return 0;
}

double abc(char* b){

    if(strcmp("", b) == 0 || strcmp(".", b) == 0) {       //Empty String or only "."
        return -1;
    }

    if(b[0] == '.') {       //No positions before the decimal point
        int length = std::strlen(b);
        char* newB = new char[length + 1];
        newB[0] = '0';
        for(int k = 0; k < length; k++) {
            newB[k + 1] = b[k];
        }

        b = newB;
    }

    int countDots = 0;
    int index = 0;
    int dotIndex = -1;
    do{
        if ((b[index] == '1') || (b[index] == '0') || (b[index] == '.')){
            index++;
            if(b[index] == '.'){
                countDots++;
                dotIndex = index;
            }
            if(countDots == 2){
                return -1;
            }
        }
        else {
            return -1;
        }
    } while(b[index] != '\0');

    if(dotIndex == -1) {    //No dot found, but not empty string, so you have a natural number
        int length = std::strlen(b);
        char* newB = new char[length + 2];
        for(int k = 0; k < length; k++) {
            newB[k] = b[k];
        }
        newB[length] = '.';
        newB[length + 1] = '0';
        dotIndex = index;
        b = newB;
    }

    double sum = 0;
    index = 0;
    int exponent = dotIndex;

    while(exponent != 0) {
        if(b[index] == '1'){
            sum = sum + pow (2, exponent-1);
        }
        index++;
        exponent--;
    }

    while(b[index] != '\0'){
        if(b[index] == '1'){
            sum = sum + pow (2, exponent);
        }
        exponent = exponent - 1;
        index++;
    }

    return sum;
}

I will leave it to you to comment the code properly.

Please note: The code now is a bit longer than yours, but it is able to handle more things.

EDIT:

After one of the comments below, I decided to also show a version without C-style arrays, and without new. This also should solve the problem with possible memory leaks. So all in all, this is the better version (I will leave the old version, because the questioner asked for a function double abc(char* b)):

#include <iostream>
#include <cmath>
#include <string>

double abc(std::string b);

int main() {
    std::cout <<  abc("1101.101") << "\n";
    return 0;
}

double abc(std::string b){

    if(b.compare("") == 0 || b.compare(".") == 0) {       //Empty String or only "."
        return -1;
    }

    if(b[0] == '.') {       //No positions before the decimal point
        b.insert(0, "0");
    }

    int countDots = 0;
    size_t index = 0;
    int dotIndex = -1;
    do{
        if ((b[index] == '1') || (b[index] == '0') || (b[index] == '.')){
            index++;
            if(b[index] == '.'){
                countDots++;
                dotIndex = index;
            }
            if(countDots == 2){
                return -1;
            }
        }
        else {
            return -1;
        }
    } while(index < b.length());

    if(dotIndex == -1) {    //No dot found, but not empty string, so you have a natural number
        b.push_back('.');
        b.push_back('0');
        dotIndex = index;
    }

    double sum = 0;
    index = 0;
    int exponent = dotIndex;

    while(exponent != 0) {
        if(b[index] == '1'){
            sum = sum + pow (2, exponent-1);
        }
        index++;
        exponent--;
    }

    while(index < b.length()){
        if(b[index] == '1'){
            sum = sum + pow (2, exponent);
        }
        exponent = exponent - 1;
        index++;
    }

    return sum;
}

Code Review

I suggest using std:string so you can use existing std functions and for each / for instead of do while.

Separate the validation to different function.

Edge cases:

non frictions
empty string
"."

For those inputs this loop

while(b[i]!='.'){ i++; }

will cause segmentation fault or infinite loop.

Alternative solution

You wrote

  Is there any faster way to solve this exercise?

So I understand you ask for alternative solution.

Here is my solution, excluding the validation:

split the binary friction to 2 strings
Convert each part to decimal number.
Convert the numbers to strings and concat them with '.'
Parse the string as double

I believe this will be shorter


Function to alternate players' turns, unless a player has won I am doing a coding challenge and it basically says this. There are two players, "black" and "white" and they take turns at a game. If a player won the last turn then it is his turn again, otherwise it is the other player's turn. The whole challenge is here.

Here is the original code I used for that:

function whoseMove(lastPlayer, win) {
  if (lastPlayer == "white") {
      if (win) {
          return "white";
      }
      return "black";
  } else {
      if (win) {
          return "black";
      }
      return "white";
  }
}

however I realised that this could be simplified by changing it to this:

function whoseMove(lastPlayer, win) {
  if (lastPlayer == "white") {
      return win ? "white" : "black";
  } else {
      return win ? "black" : "white";
  }
}

I wonder if this could be simplified even more? Is it possible to have more than one condition in the conditional return statement (I think that is what it is called)?

You could return the same player if win is true, otherwise return the other player, this is done with only 2 tests:

function whoseMove(lastPlayer, win) {
    if (win) return lastPlayer;
    return lastPlayer == "black" ? "white" : "black";
}

"White" is returned when both conditions are true or when both are false so you can use XOR, denoted as ^ in JavaScript.

function whoseMove(lastPlayer, win) {
  return lastPlayer == "white" ^ !!win ? "black" : "white";
}

I would advise against it since that's less obvious. Here's a self-explanatory condition:

function whoseMove(lastPlayer, win) {
  return lastPlayer == "white" && win || lastPlayer != "white" && !win ? "white" : "black";
}


Sort multidimensional array based on another one containing the desired order I have an array like this:
$array = [
    ['id' => 1, 'content' => 'value 1'],
    ['id' => 2, 'content' => 'value 2'],
    ['id' => 3, 'content' => 'value 3'],
];

I have another array containing the actual order of ids in which $array should be sorted :
$order = [3, 1, 2];

Which means, the result should be this:
$sorted = [
    0 => ['id' => 3, 'content' => 'value 3'],
    1 => ['id' => 1, 'content' => 'value 1'],
    2 => ['id' => 2, 'content' => 'value 2'],
];

The final requirement is, that the array should be printed using foreach:
foreach($sorted as $value) {
    print $value['content'];
}

I came up with this solution:
function orderArray(array $array, array $order) {
    $sorted = [];

    foreach($array as $value) {
        $id = $value['id'];
        $index = array_search($id, $order);
        $sorted[$index] = $value;
    }

    ksort($sorted);

    return $sorted;
}

This works fine. However, I wonder whether this is efficent, especially as I have to call ksort to make the array be printed correctly by foreach.
Can this be optimized? Maybe there's also a more elegant solution to the problem.

It's a given, that all ids and only those are in $order - so no need to test for that.

I think this would be the cleanest solution. Similar to @YourCommonSense answer, but slightly inverted as ordered index is already there - only values missing.

$sorted = array_flip($order);
foreach ($array as $value) {
    $id = $value['id'];
    $sorted[$id] = $value;
}

Assuming you aren't going to use the sorted array you are creating for something else just use the $order array to echo the values directly.

$array = [
    ['id' => 1, 'content' => 'value 1'],
    ['id' => 2, 'content' => 'value 2'],
    ['id' => 3, 'content' => 'value 3'],
];

$order = [3, 1, 2];

foreach ($order as $point) {
   echo $array[$point - 1]['content'] . PHP_EOL;
}


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).

The problem with the slow running time is due to the presence of two loops in your code. In this particular case, only one suffices. The following code should be fast enough to pass your benchmarking test.

#include <stdio.h>
#include <string.h>
#include <math.h>
#include <stdlib.h>
#include <assert.h>
/* Head ends here */

int commonPrefix(char* a, char* b) {
    char *x, *y;

    for (x = a, y = b; *x && *y; x++, y++) {
        if (*x != *y)
            break;
    }
    return x-a;
}

int stringSimilarity(char *a) {
    int len = strlen(a);
    char *suffix;
    int i, sum = 0;
    for (i = 1; i <= len; i++) {
        suffix = a + len-i;
        sum += commonPrefix(a, suffix);
    }
    return sum;
}

int main() {
    int res, t, i;
    scanf("%d",&t);
    char a[100001];
    for (i=0;i<t;i++) {
        scanf("%s",a);
        res=stringSimilarity(a);
        printf("%d\n",res);
    }

    return 0;
}


MYSQL select count two table left join I have two table. I want count how many times a value is in other table. So the codice from table "sorgente" is different in table contatore, because I have the suffix 'BRANO-' before the code.
sorgente
| codice | nome  |
|   15   | mario |
|   16   | mary  |

contatore
| nome_evento |   data     |
| BRANO-15    | 2020-08-15 |
| BRANO-15    | 2020-08-16 |
| BRANO-16    | 2020-08-14 |

the query for select is
select
    s.codice,
    (
        select count(*)
        from contatore c
        where c.nome_evento = concat('BRANO-', s.codice)
    ) no_matches_in_contatore
from sorgente s

but the original "sorgente" table has 25000 records, the "contatore" table has at now 60000 records.
when I try to execute the query it takes 600 seconds....
can you help me?

You should try below query, also let me know what indexes are on your tables.
  WITH cte1 as (select SUBSTR(nome_evento, instr(nome_evento, '-') + 1) as nome_evento  from contatore)
SELECT count(b.codice),
        b.codice
FROM cte1 a
INNER JOIN
sorgente b
ON (a.nome_evento = b.codice)
group by b.codice;


PHP checking for array key or using default value I currently use this code pattern fairly frequently

if(array_key_exists('possible_key', $array)) {
    $my_variable = $array['possible_key'];
} else {
    $my_variable = 'my default value';
}

Is there a better way to be writing this?

One quick change you can do is making use of ternary operator instead of writing if else block.

$my_variable = array_key_exists('possible_key', $array)
               ? $array['possible_key']
               : 'my default value';

I prefer to use use isset and make little refactor and wrap that code to function/method

function defaultize( $array_var ,$defaul_val ){
 if(isset($array_var)) {
    $ret = $array_var;
 } else {
    $ret = 'my default value';
 }
 return $ret;
}

and then use in code

$my_variable = defaultize($array['possible_key'],'my default_value' );
$my_variable2 = defaultize($array['possible_key2'],'my default_value2' );


Searching the same values in an array If I am not mistaken, this method has a complexity \$O(N^2)\$:

int[] exampleArray = { 4, 6, 2, 2, 6, 6, 1, 55, 55, 2, 5, 6, 90, 8, 8, 8, 10, 70 };
int k = GetMaximumCycle(exampleArray);

public static int GetMaximumCycle(int[] anArray)
{
   int length = anArray.Length;
   int result = 0;
   for (int i = 0; i < length; i++)
   {
      for (int j = 0; j < length; j++)
      {
          if (anArray[i] == anArray[j])
          {
             if (!(result > Math.Abs(i - j)))
                 result = Math.Abs(i - j);
          }
      }
   }
   return result;
}

I compare values and gets the maximum difference between items of array.
The result variable is 10.

And if an input array is very large, this code works slowly. Is it possible to simplify the for loops or remove the second for loop to have a complexity of \$O(N)\$?

You can use a Dictionary<int, int> of positions of the first occurrence of each element value.
Next, iterating over all elements of the input array, you do:

If the positions[value] is empty, store the current index i in it: positions[value] = i.
Else, calculate the distance (or cycle length).
If this distance is greater than maximum, save it as a new maximum.

This algorithm has time complexity \$O(n)\$.

The code:

public static class CycleSearch
{
    public static int GetMaximumCycle(int[] array)
    {
        Dictionary<int, int> positions = new Dictionary<int, int>();

        int result = 0;
        for (int i = 0; i < array.Length; i++)
        {
            int value = array[i];
            int position;
            if (!positions.TryGetValue(value, out position))
            {
                positions[value] = i;
            }
            else
            {
                int cycleLength = i - position;
                if (cycleLength > result)
                {
                    result = cycleLength;
                }
            }
        }
        return result;
    }
}

Usage:

int[] exampleArray = { 4, 6, 2, 2, 6, 6, 1, 55, 55, 2, 5, 6, 90, 8, 8, 8, 10, 70 };
Console.WriteLine(CycleSearch.GetMaximumCycle(exampleArray));

Output:

  10

You are correct, it is \$O(n^2)\$.

I seriously doubt it is possible to reduce time complexity to \$O(n)\$, however there is a solution with \$O(n\log n)\$ time and \$O(n)\$ space complexity: sort the set of (value, index) pairs, and obtain the result in a single linear pass.


Beginner's Number Guessing Game I've started learning to program, and as the first program I wrote on my own I wanted to come up with a number guessing game - the below is what I got.
I'm mainly looking for a review of my Guess function, but I do have a few specific questions:

Does it make sense to have my loop variable as a function parameter?
I'm currently defining some variables (e.g. tries) outside of my function - is this an idiomatic way to do it?

import random
tries = [1]
def Guess(playing):
    number = random.randint(1,100)
    print(number)
    print("We are going to play high and low, the avabile numbers are from 1 to 100 included.")
    while playing == True:
        user_input = input("What is your number? ")
        if int(user_input) == number:
            print("You have won!")
            playing = False
        elif int(user_input) < number:
            print("You need to give a higher number")
            tries.append(1)
        elif int(user_input) > number:
            print("You need to give an lower number")
            tries.append(1)
        else:
            print("You have put an wrong number")
            playing = False
    print("It took you " + str(sum(tries)) + " turns to guess")
    still = input("Do you want to play again?")
    if still == "yes" or still == "YES" or still == "y" or still == "si":
        Guess(playing=True)
    else:
        playing=False

Guess(playing=True)

Overall you've done a good job with your code, there are a couple things I'd look out for :

You aren't paying for whitespace! What I mean is that it's important to let your
code breathe. Use some empty lines, it'll do wonder for your eyes when dealing with big chunks of code.
Why is tries an array? In my opinion, tries should represent the number of tries, so it should be a number!
Notice that you have for conditions, ==, <, > and... ? Say I ask you to give me a number that is neither equal, smaller or greater than 3, is there a possible option? What you should do instead is make sure the player input a number! Right now, if I was to answer "I don't know my numbers I'm just a baby" to "We are going to play high and low, the avabile numbers are from 1 to 100 included.", your program will crash! You should look into try/catch blocks to solve this problem, that could be a next thing to learn for you!
Say I need three tries on my first playthrough, then I start a new game (using "yes" when prompted) and I'm able to succeed on the first try, your code will tell me I've needed four tries to succeed. Can you find out why and how you can fix that?
You could simplify your loop using the break keyword, that's also something you should search for (while we're at it, try to understand the usage of break, return and continue. Those three keywords are pretty useful, although you probably already know about return).
What if I'm an english major and I can't stand making grammar mistakes (It's obviously not my case) and I write "Yes", instead of "yes" or "YES", when asked if I want to play again? What you could do is compare the lowered (or uppered) version of the input with what you want to check (ex. still.lower() == "yes").

I tried this. Once look it's helpful or not.
n=20
count=0
while count<5:
    a=int(input("Enter guess number:"))
    count=count+1
    if a==n:
        print("Guess number matched.")
        print("Number of guess took is "+str(count))
        break
    else:
        if a>n:
            print(str(a)+" is greater than guess number.")
        else:
            print(str(a)+" is less than number.")
        print("Remaining chance is "+str(5-count))
        continue
if count==5:
    print('Game Over')


"Utopian Tree" challenge optimization Problem statement

The Utopian tree goes through 2 cycles of growth every year. The first growth cycle occurs during the monsoon, when it doubles in height. The second growth cycle occurs during the summer, when its height increases by 1 meter.
Now, a new Utopian tree sapling is planted at the onset of the monsoon. Its height is 1 meter. Can you find the height of the tree after \$N\$ growth cycles?

Input Format

The first line contains an integer, \$T\$, the number of test cases.
\$T\$ lines follow. Each line contains an integer, \$N\$, that denotes the number of cycles for that test case.

Constraints

1 <= T <= 10
0 <= N <= 60

Sample Input: #01:

2
3
4

Sample Output: #01:

6
7

Explanation: #01:

There are 2 testcases.

N = 3:

the height of the tree at the end of the 1st cycle = 2
the height of the tree at the end of the 2nd cycle = 3
the height of the tree at the end of the 3rd cycle = 6

N = 4:

the height of the tree at the end of the 4th cycle = 7

My solution:

   'use strict';

function processData(input) {
    var parse_fun = function (s) {
        return parseInt(s, 10);
    };

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

    var data = lines.splice(0, T).map(parse_fun);
    for (var i = 0; i < data.length; i++) {
        var value = 1;
        for (var j = 0; j < data[i]; j++) {
            if (j % 2 == 0)
                value *= 2;
            else value += 1;
        }
        process.stdout.write(value + '\n');
    }
}

process.stdin.resume();
process.stdin.setEncoding("ascii");
var _input = "";
process.stdin.on("data", function (input) {
    _input += input;
});
process.stdin.on("end", function () {
    processData(_input);
});

The running time is of \$O(n^2)\$.  Can anyone tell how to optimize this code to decrease runtime?

A lot of your code seems to be Hackerrank's boilerplate code, so I'll ignore that (though it could use a review), and focus on the "meat" which is:

var value = 1;
for (var j = 0; j < data[i]; j++) {
    if (j % 2 == 0)
        value *= 2;
    else value += 1;
}
process.stdout.write(value + '\n');

(It's run T times of course, but that's irrelevant for now.)

There's not a lot of code there, so there's little to review. Still, the if..else is not great, if you ask me. For one, I'd advice that you always use braces - even for one-liners. But if you don't, at least use linebreaks:

if (j % 2 == 0)
    value *= 2;
else
    value += 1;

However, you can do it all in a ternary, which I would find more appropriate here:

value += (j % 2 === 0 ? value : 1);

The parentheses aren't required, but I find they make it more readable.

Of course, you could also rely on zero being false'y in JavaScript, and just do

value += (j % 2 ? 1 : value);

A slightly faster solution to the even/odd branching would be

value += (j & 1 ? 1 : value);

In other words: If the least-significant bit is 1, the number is odd.

In all, you get:

var value = 1;
for (var j = 0; j < data[i]; j++) {
    value += (j & 1 ? 1 : value);
}
process.stdout.write(value + '\n');

Of course, there may be a purely mathematical, loop-less solution to this. But that's unfortunately not my strong suit.

Update: As mjolka points out in the comments, there is a very simple pattern to this. (And I feel pretty dumb for not realizing it.) I can't really explain it more succinctly than mjolka already has, so I'll just quote the comment here:

  Look at the first few terms: 1, 2, 3, 6, 7, 14, 15, 30, 31, ... and compare that to powers of two: 2, 4, 8, 16, 32, ...

As is obvious, the output values are all equivalent to a power of 2, minus 1 or minus 2. In code, that can be expressed as a function like so:

// calculate tree height after n cycles
function utopiaTreeHeight(n) {
  var exp = Math.ceil(n / 2) + 1,   // calculate the exponent
      value = Math.pow(2, exp) - 1; // power of 2, minus 1

  return value - (n & 1);           // subtract another 1 if n is odd, and return
}

Which means that the rest of the code is just:

for(var i = 0 ; i < data.length ; i++) {
  var value = utopiaTreeHeight(data[i]);
  process.stdout.write(value + '\n');
}

Now, that's pretty clean, I'd say. Big tip of the hat to mjolka!

Below are my previous (iterative and naïve) solutions. I'll leave them in my answer, only because there's probably something of value in there, even if this particular problem has a much more elegant solution.

(End of update.)

Now, about repeating it T times: If you're processing many test cases, it might be worth it to do some pre-processing. All you really need is to iterate to the highest cycle-count. So if you're asked to solve for data = [3, 6, 5] what you really want to just solve for N = 6 but store the intermediate values for N = 3 and N = 5 along the way. You only need to loop once; from zero to Nmax.

Of course, doing so will require extra setup, which may be less efficient than simply doing what you're doing now, if there's only one or two test cases.

Just for fun, though, one solution might be:

var cycles = data.length,          // cache this for later
    sorted = data.slice(0).sort(), // copy and sort the input values (slightly faster if you use an explicit comparison function)
    limit  = sorted[cycles - 1],   // get the max
    target = sorted.shift(),       // grab the lowest value (our first target cycle)
    value  = 1,                    // initial tree height
    values = {};                   // a place to store values

// loop to the highest cycle-count (note the range is 0..limit)
for(var n = 0 ; n <= limit ; n++) {
  while( n === target ) {
    // we reached our target cycle, so store the current value
    values[n] = value;

    // and grab the next target
    target = sorted.shift();
  }
  value += (n & 1 ? 1 : value);
}

// print results in the correct order
for(var i = 0 ; i < cycles ; i++) {
  process.stdout.write(values[data[i]] + '\n');
}

The while loop is there to handle duplicates in the input.

A simpler solution would be to simply store every value for N in 0..Nmax in an array. That tradeoff would be memory consumption. If data = [1, 923123] you'd end up storing 923121 values (~7MB at worst) you're not going to use. Crazy example, but if you don't know your input, well...

Still, such an approach could look like:

var limit = Math.max.apply(null, data),
    values = [],
    value = 1;

for(var n = 0 ; n <= limit ; n++) {
  values.push(value);
  value += (n & 1 ? 1 : value);
}

for(var i = 0 ; i < data.length ; i++) {
  process.stdout.write(values[data[i]] + '\n');
}

Use preprocessed values.

function processData(input) {
    var parse_fun = function (s) {
            return parseInt(s, 10);
        },
        heights = function() {
            var result = [1]
            for (var i = 1; i <= 60; i++) {
                if (i & 1) {
                    result[i] = result[i-1] * 2;
                } else {
                    result[i] = result[i-1] + 1;
                }
            }
            return result;
        }();

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

    var data = lines.splice(0, T).map(parse_fun);
    for (var i = 0; i < data.length; i++) {
        process.stdout.write(heights[data[i]] + '\n');
    }
}

In this case complexity is \$O(n+t)\$


Calculate the Hamming difference between two DNA strands Write a program that can calculate the Hamming difference between two DNA strands.

  GAGCCTACTAACGGGAT
     CATCGTAATGACGGCCT
     ^ ^ ^  ^ ^    ^^

  The Hamming distance between these two DNA strands is 7.

import java.util.Optional;

public class Hamming {
  public static int compute(String s1, String s2) {
    validateInputs(s1, s2);

    int hammingDistance = 0;
    int stringLength = s1.length();

    for (int i = 0; i < stringLength; i++) {
      if (s1.charAt(i) != s2.charAt(i)) {
        hammingDistance++;
      }
    }
    return hammingDistance;
  }

  private static void validateInputs(String s1, String s2) {
    if (s1.length() != s2.length()) {
      throw new IllegalArgumentException();
    }
  }
}

Test suite:

import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;

import org.junit.Test;

public class HammingTest {

    @Test
    public void testNoDifferenceBetweenIdenticalStrands() {
        assertThat(Hamming.compute("A", "A"), is(0));
    }

    @Test
    public void testCompleteHammingDistanceOfForSingleNucleotideStrand() {
        assertThat(Hamming.compute("A", "G"), is(1));
    }

    @Test
    public void testCompleteHammingDistanceForSmallStrand() {
        assertThat(Hamming.compute("AG", "CT"), is(2));
    }

    @Test
    public void testSmallHammingDistance() {
        assertThat(Hamming.compute("AT", "CT"), is(1));
    }

    @Test
    public void testSmallHammingDistanceInLongerStrand() {
        assertThat(Hamming.compute("GGACG", "GGTCG"), is(1));
    }

    @Test(expected = IllegalArgumentException.class)
    public void testValidatesFirstStrandNotLonger() {
        Hamming.compute("AAAG", "AAA");
    }

    @Test(expected = IllegalArgumentException.class)
    public void testValidatesOtherStrandNotLonger() {
        Hamming.compute("AAA", "AAAG");
    }

    @Test
    public void testLargeHammingDistance() {
        assertThat(Hamming.compute("GATACA", "GCATAA"), is(4));
    }

    @Test
    public void testHammingDistanceInVeryLongStrand() {
        assertThat(Hamming.compute("GGACGGATTCTG", "AGGACGGATTCT"), is(9));
    }

}

Questions:

Am I using the correct data structures?
Can I improve performance? Right now it seems to be \$Θ(n)\$ since it has to check all the characters in the string.

Your code is really all that there is to it. It is clear and concise: one method to validate the input (validateInputs) and the rest of the method to calculate the Hamming distance with a simple loop.

A couple of comments:

In case of inputs of different length, you are (rightfully) throwing a IllegalArgumentException like this:

if (s1.length() != s2.length()) {
  throw new IllegalArgumentException();
}

This is a generic IllegalArgumentException with no message. You might want to add a message so that it is clearer to the caller what went wrong; it would also help the debugging (imagine a bigger application).
To resemble what is done by the existing Objects class, you could rename your validating method to requireSameLength and make it return the length:

private static int requireSameLength(String s1, String s2) {
    if (s1.length() != s2.length()) {
        throw new IllegalArgumentException();
    }
    return s1.length();
}

This has the advantage that the method name is now self-documenting and it is returning the correct value, which means we can chain the result (just like requireNonNull which returns the non-null value or throws an exception).
import java.util.Optional; is not needed, you aren't using it.
Consider making the class final since it looks like a utility class (only public static methods).

If you're using Java 8, you could write this a bit shorter using the Stream API:

public static int compute(String s1, String s2) {
    int length = requireSameLength(s1, s2);
    return IntStream.range(0, length).map(i -> s1.charAt(i) == s2.charAt(i) ? 0 : 1).sum();
}

private static int requireSameLength(String s1, String s2) {
    if (s1.length() != s2.length()) {
        throw new IllegalArgumentException();
    }
    return s1.length();
}

It maps each index to 0 or 1 based on whether the two input Strings have equal characters at that index, and sums the result.

As a good practice, I would suggest to work with interface and object instead of static class (then, any refactoring must not break this interface). Here something like :

public interface HammingDifference {
    int compute();
    boolean validate();
}

Then your implementation will be restricted to this contract. And you can have a constructor based on the implementation specifics, eg:

StringHammerDifference(String a, String b)
ArrayHammerDifference(character[] a, character[] b)
// or even custom objects
DnaSequenceHammerDifference(DnaSequence a, DnaSequence b)

If you are really concerned about speed - and when you talk about speed, I hope you mean long DNA sequence comparision (??) - I would strongly suggest to work on the DnaSequence representation.
For example:

An array of small chunks with a corresponding checksum (then you compare only when checksums are different)
A tree (I don't know if there are Hamming focused trees, but my guess is there are)

I think that tweaking from big to short integers may not be sufficient enough.

My final thought will be: there are, I guess, a lot of documentation and even libraries out there, so looking at them may be a good thing ;-)


How can I optimize this code that finds the GCD? int count;
int gcdCount;
int testCase = 5;

while (testCase > 0)
{
    int n = 5;
    count = 0;
    gcdCount = 0;
    // to get two  random numbers a and b a<=n and b<=n
    //get probablity of gcd(a,b) ==b
    for (int i = 1; i <= n; i++)
    {
        for (int j = 1; j <= n; j++)
        {
            count++;
              // check if gcd(i,j) is equal to second number j
            if (findgcd(i, j) == j)
            {
                gcdCount++;
            }
        }
    }
     //return probability of gcdcount
    System.out.println(gcdCount + "/" + count);
    testCase--;
}

Assuming that findgcd is correctly implemented and that i and j are guaranteed to both be non-negative, findgcd(i, j) == j is equivalent to j != 0 && i % j == 0. That allows a further optimisation to a single loop, because the number of values in 1..n which are divisible by j is floor(n/j):

for (int j = 1; j <= n; j++)
{
    count += n;
    gcdCount += n / j;
}

You do not need to increment "count" variable in every loop. The value of it depends on "n".

Here you can find some ways to optimize a loop:
http://en.wikipedia.org/wiki/Loop_optimization

What is the difference between the test cases? For me it seems nothing change; you always run the same double for-loop.


Filter out date records which don't fall within or span a date range I have records which have specific time spans. The need is to select records which adhere to the following three rules (read them as or conditions) concerning a target time span:

Record's Begin date can begin within the target time span and end date does not matter.
Record's End date falls with the target time span and the start date is inconsequential.
Record's dates actually span the target time.

So basically a query should return all records which start, end or span a target time frame.

Is there a more efficient way in Oracle 11g of doing the above rules than what I currently have stated in the SQL below?

I have created a test SQL Fiddle Test for the below schema and SQL

Schema

CREATE TABLE dates_test (
  id         NUMBER,
  hint       VarChar(20),
  BEGIN_DATE DATE,
  END_DATE DATE
);
-- If the span is Mar 6 to April 6 then 2, 3 and 4 are valid -
INSERT INTO dates_test VALUES (1, 'exclude before', TO_DATE('01-JAN-2014','DD-MON-YYYY'), TO_DATE('05-JAN-2014','DD-MON-YYYY'));
INSERT INTO dates_test VALUES (2, 'spans',          TO_DATE('05-MAR-2014','DD-MON-YYYY'), TO_DATE('08-SEP-2014','DD-MON-YYYY'));
INSERT INTO dates_test VALUES (3, 'Starts within',  TO_DATE('07-MAR-2014','DD-MON-YYYY'), TO_DATE('07-SEP-2014','DD-MON-YYYY'));
INSERT INTO dates_test VALUES (4, 'Ends within',    TO_DATE('01-JAN-2014','DD-MON-YYYY'), TO_DATE('07-MAR-2014','DD-MON-YYYY'));
INSERT INTO dates_test VALUES (5, 'exclude after',  TO_DATE('06-JUN-2014','DD-MON-YYYY'), TO_DATE('07-SEP-2014','DD-MON-YYYY'));
INSERT INTO dates_test VALUES (6, 'within OK',  TO_DATE('08-MAR-2014','DD-MON-YYYY'), TO_DATE('09-MAR-2014','DD-MON-YYYY'));

Here is the SQL which returns what I need but can it be improved?

select * from
dates_test t0
where
(  -- Starts Within range
  ( t0.BEGIN_DATE
    BETWEEN TO_DATE('03-06-2014', 'mm-dd-yyyy')
    AND     TO_DATE('04-06-2014', 'mm-dd-yyyy')
  )
OR  -- Ends within range
  (
    t0.END_DATE
    BETWEEN TO_DATE('03-06-2014', 'mm-dd-yyyy')
    AND     TO_DATE('04-06-2014', 'mm-dd-yyyy')
   )
OR    -- SPANS Range
   (
     t0.BEGIN_DATE < TO_DATE('03-06-2014', 'mm-dd-yyyy')
     AND
     t0.END_DATE > TO_DATE('04-06-2014', 'mm-dd-yyyy')
    )
);

Result:

| ID |          HINT |                     BEGIN_DATE |                         END_DATE |
|----|---------------|--------------------------------|----------------------------------|
|  2 |         spans |   March, 05 2014 00:00:00+0000 | September, 08 2014 00:00:00+0000 |
|  3 | Starts within |   March, 07 2014 00:00:00+0000 | September, 07 2014 00:00:00+0000 |
|  4 |   Ends within | January, 01 2014 00:00:00+0000 |     March, 07 2014 00:00:00+0000 |
|  5 |     within OK |   March, 08 2014 00:00:00+0000 |     March, 09 2014 00:00:00+0000 |

So, My first answer messed up (Can I blame it on incomplete test data?)

That's a good thing to note, that your should design your tests to cover the situations that are significant to you. Because your tests were incomplete, when I ran and modified your code, and got the 'same' results, I assumed the code was good. I never validated your test cases (my fault).

This re-work made me realize that your data would be better served with a negative test case. What you want to test for is exclusionary ranges. If you can assume your data is valid (it is, right), then you want to just test those cases where there is no intersection.

There is no intersection if the test date ends before the data's date, or the data's date ends before the test date. This simplifies the query to:

select *
from dates_test t0
where not (
      END_DATE < TO_DATE('03-06-2014', 'mm-dd-yyyy')
   or BEGIN_DATE > TO_DATE('04-06-2014', 'mm-dd-yyyy')

As was pointed out to me (very nicely, I may add), the above logic can be simplified further using 'basic' boolean logic to:

select *
from dates_test t0
where END_DATE >= TO_DATE('03-06-2014', 'mm-dd-yyyy')
  and BEGIN_DATE <= TO_DATE('04-06-2014', 'mm-dd-yyyy'))

which is my final answer (thanks @mjolka)

Using your (revised) SQL fiddle, the above query produces (note that your revised data has two records with id 5):

| ID |          HINT |                     BEGIN_DATE |                         END_DATE |
|----|---------------|--------------------------------|----------------------------------|
|  2 |         spans |   March, 05 2014 00:00:00+0000 | September, 08 2014 00:00:00+0000 |
|  3 | Starts within |   March, 07 2014 00:00:00+0000 | September, 07 2014 00:00:00+0000 |
|  4 |   Ends within | January, 01 2014 00:00:00+0000 |     March, 07 2014 00:00:00+0000 |
|  5 |     within OK |   March, 08 2014 00:00:00+0000 |     March, 09 2014 00:00:00+0000 |

EDIT: I've squeezed the filter a lot. I couldn't wrap my head around it before how to make sure something really fit within the time period. It's this: Start date BEFORE the END of the time period, and End date AFTER the BEGINNING of the time period

With the help of someone in my office I think we've figured out how to include everyone in the filter.
There are 5 scenarios where a student would be deemed active during the time period in question:

1) Student started and ended during the time period.

2) Student started before and ended during the time period.

3) Student started before and ended after the time period.

4) Student started during the time period and ended after the time period.

5) Student started during the time period and is still active (Doesn't have an end date yet)

Given these criteria, we can actually condense the statements into a few groups because a student can only end between the period dates, after the period date, or they don't have an end date:

1) Student ends during the time period AND [Student starts before OR during]

2) Student ends after the time period AND [Student starts before OR during]

3) Student hasn't finished yet AND [Student starts before OR during]

   (
        (
         student_programs.END_DATE  >=  '07/01/2017 00:0:0'
         OR
         student_programs.END_DATE  Is Null
        )
        AND
        student_programs.START_DATE  <=  '06/30/2018 23:59:59'
   )

I think this finally covers all the bases and includes all scenarios where a student, or event, or anything is active during a time period when you only have start date and end date. I've attached a screenshot of the SAP Business Objects query as well. Please, do not hesitate to tell me that I am missing something. I want this to be perfect so others can use this, as I don't believe the other answers have gotten everything right yet.


SQL code reuse with C# I have some code that allows for SQL reuse for C#. I'm pretty happy with it, but the fact that I can't find anything similar on the web makes me think I'm going about it the wrong way. I have a GitHub repo with a Visual Studio solution, and the code is repeated down below.

Here's the use case:

I'm working on a fairly standard C# and SQL Server project where we're generating reports. Each report is essentially the results of SQL query, displayed in HTML in the browser. I'd rather not use an ORM like Entity Framework, since we need to exactly control the SQL for performance reasons (e.g. use SQL server windowing functions, table hints, etc).

However, many of the reports are similar, just slicing and dicing by different attributes, so there's a desire for code reuse. SQL Server has stored procedures, views, and functions, but the consensus seems to be that you'll get into trouble trying to use them to create abstractions. To get around this I'm trying to use C# to reuse SQL code.

I'm essentially using Visual Studio's T4 text templates to generate SQL files at development time. I'm then passing the SQL to Stack Exchange's Dapper.NET project to handle variable substitution, issuing the query, and POCO [de]serialization.

Given a query like this, in the template file CuteAnimalsByLocation.tt:

<#@ output extension=".sql" #>
select * from animals a
where a.IsCute = 1 and a.IsFuzzy = 1 and a.Location = @location

And this file, DeadlyMachinesByLocation.tt:

<#@ output extension=".sql" #>
select * from DeadlyMachines m
where m.IsLethal = 1 and m.HasExplosives = 1 and m.Location = @location

I can reuse the above queries in a more complex query by writing the following, AnimalsInPeril.tt:

<#@ output extension=".sql" #>

WITH
CuteAnimalsInLocation as
(
    <#@ include file="CuteAnimalsByLocation.tt" #>
),
DeadlyMachinesInLocation as
(
    <#@ include file="DeadlyMachinesByLocation.tt" #>
)
select a.* from CuteAnimalsInLocation a
inner join DeadlyMachinesInLocation m on a.Location = m.Location

I can run all 3 of these queries like this: (assuming the POCOs Animal and DeadlyMachine exist; they're just POCOs that map to the table schemas):

using (var connection = new SqlConnection(ConnectionString))
{
    connection.Open();
    var queryParams = new { Location = "NorthAmerica" };

    IEnumerable<Animal> animalsNeedingHelp =
        connection.QueryFromFile<Animal>("AnimalsInPeril", queryParams);
    IEnumerable<Animal> cuteAnimals =
        connection.QueryFromFile<Animal>("CuteAnimalsByLocation", queryParams);
    IEnumerable<DeadlyMachine> deadlyMachines =
        connection.QueryFromFile<DeadlyMachine>("DeadlyMachinesByLocation", queryParams);
}

connection.QueryFromFile is defined as this, and depends on Dapper:

public static class DapperFileExtensions
{
    public static IEnumerable<TReturn> QueryFromFile<TReturn>(this IDbConnection cnn, string file, object params = null)
    {
        var sql = File.ReadAllText(file + ".sql");
        return cnn.Query<TReturn>(sql, params);
    }
}

Can anyone see problems with this approach? I guess the only thing I can think of is that I could add caching around the File.ReadAllText call.

You say performance is a concern, but you're doing this.

select a.* from

That's poor form whether or not you already have a table scan in the underlying query plan for the CTE.

I think you misunderstood the article you linked to. It is warning against doing exactly what you've done here. SQL just doesn't lend itself to code reuse in the way a "regular" programmer is accustomed to. It's a different beast that way, it takes a different mindset.

SQL is a set based query language. Code reuse comes in the form of (well tuned) views and stored procedures. If you need this kind of fine grained control over the SQL, then keep it where it belongs, in the database.

Depending on just how dynamic your queries are, you may not always be getting the benefit of query plan caching. Each time a query that the analyzer hasn't seen before is processed, it has to generate a new plan, so you'll take a performance hit.

Lastly, I left a comment, but I should mention it here too. Be careful using query hints. You may be able to outwit the analyzer now, based on the current data, but the data will change over time. When the data has changed and your hint is no longer the most efficient query plan, the analyzer won't be able to choose the most efficient plan because you've told it not to. Performance gained this way may degrade over time.

The main problem I see with this approach is you have magical strings, which are not compile time safe.

I highly recommend you look into using an ORM like entity framework

public class AnimalsRepository : IDisposable
{
    private MyDataModel Model { get; set; }
    public AnimalsRepository()
    {
        Model = new MyDataModel();
    }
    public void Dispose()
    {
        if (this.Model != null)
            this.Model.Dispose();
    }
    public IQueryable<Animal> GetCuteAnimals(string location)
    {
        return this.Model.Animals.Where(a => a.IsCute && a.IsFuzzy && a.Location == location);
    }
    public IQueryable<DeadlyMachine> GetDeadlyMachines(string location)
    {
        return this.Model.DeadlyMachines.Where(m => m.IsLethal && m.HasExplosives && m.Location == location);
    }
    public IQueryable<Animal> GetAnimalsInPeril(string location)
    {
        return from a in this.GetCuteAnimals(location)
               join m in this.GetDeadlyMachines(location) on a.Location equals m.Location
               select a;

    }
}

See Nuget Package: https://www.nuget.org/packages/EntityFramework


Find the minimum number of coins I'm trying to learn dynamic programming and the most popular one is to find the minimum number of coins for the change. I come up with this but I just want to improve this code which I think it could be shorter in Python but I'm new to Python too.

import sys
coins = [1, 3, 5]
min_coin = [sys.maxint] * 20
min_coin[0] = 0

for min_of_i in range(20):
    for c in coins:
        if c <= min_of_i and (min_coin[min_of_i - c] + 1 < min_coin[min_of_i]):
                min_coin[min_of_i] = min_coin[min_of_i - c] + 1

print min_coin

This is already pretty short. Making it shorter doesn't seem a good objective, and risks hurting readability.
That being said, there are a few things you could do.

You could do this in one step:

min_coin = [sys.maxint] * 20
min_coin[0] = 0

Like this:

min_coin = [0] + [sys.maxint] * 19

While at it, why are there 20 elements? Your program will find the minimum number of coins up to 19, and I have a feeling that you actually want it for 20. In which case you would need:

min_coin = [0] + [sys.maxint] * 20

And then use range(21) in the loop. However, you shouldn't hard code this number, give it a name, like target_amount, and use that.

Speaking of names, many of the names in your code can be better:

min_coin is a list, so plural min_coins would be better
min_of_i is actually an amount

One more optimization is possible by flipping the order of the nested loops.
In the current code you need an if statement to skip the amounts that are too small for the coin.
If you make the loop over coins to be the outer loop,
then you can set a starting value for the inner value to make the if statement unnecessary.

def get_min_coins(coins, target_amount):
    n = len(coins)
    min_coins = [0] + [sys.maxint] * target_amount
    for i in range(1, n + 1):
        for j in range(coins[i - 1], target_amount + 1):
            min_coins[j] = min(min_coins[j - coins[i - 1]] + 1, min_coins[j])
    return min_coins

First of all, that's not dynamic programming as I understand the term.  You haven't broken the problem down into simpler sub-problems, and in fact you have used an algorithm so convoluted in its logic that it's inordinately difficult to understand.

Divide the problem into two parts: (1) a function to compute the number of coins needed to achieve a given total.  Here it is:

coins = (5,3,1)  # order this list max to min
def coin_count(x):
    n = 0
    for a_coin in coins:
        count = x // a_coin
        n += count
        x -= a_coin * count
    return n

(2) You simply call this function to generate any desired output.  You want the answers for the first 20 integers, so...

min_coin = [coin_count(n) for n in range(20)]

print min_coin

Which gives the same answer as yours, except it only declares one variable (coins), creates a re-usable function (coin_count) and can easily be modified for different coin denominations and different output requirements.  I can only guess whether that qualifies as dynamic programming, but I think it's a far better approach.


Get Ajax request implementation without jQuery I'm writing a pixel library to do an AJAX request so I'm not allowed to use any helper.

function ajax(options) {
  var xhr = new XMLHttpRequest();
  var opts = Object.assign({
    withCredentials: false,
    method: 'GET'
  }, options);

  xhr.withCredentials = options.withCredentials;
  xhr.open(opts.method, opts.url);
  xhr.setRequestHeader('Accept', 'text/plain');
  xhr.send(null);
  return {
    done: function(cb) {
      xhr.onreadystatechange = function onStateChange() {
        if (this.readyState === 4) {
          if (this.status >= 200 && this.status < 300) {
            cb(this.responseText);
          } else {
            cb('error');
          }
        }
      };
      xhr.ontimeout = function(e) {
        console.error(e);
        cb(false);
      };

      xhr.onerror = function(e) {
        console.error(e);
        cb(false);
      };
    }
  }
};

ajax({
  url: '/echo/json'
}).done(function(response) {
  console.log(response);
})

I also have my code in a jsfiddle in case anyone wants to test the code.

Note: This library doesn't support POST as I want to keep the code as less as possible. It will only support GET.

Your ajax() function gives me a good first impression. The structure seems sound and the event handling well done - the done() event handler is called exactly once (someone else might want to confirm, though).

A few suggestions:

var opts = Object.assign({...}, options);

opts vs. options is inconsistent and carries no semantics. Reassigning to options would mutate the argument which could cause issues for the user. jQuery names the merged options and defaults settings, which might be a little bit more clear.
if (this.status >= 200 && this.status < 300) {

This line seems good for now. You might want to check 304 in a future version when allowing users to supply caching headers. However, such requests don't respond with data and are probably not within the intended use of your library right now.
xhr.send(null);

The send method accepts an optional parameter which is ignored and set to null for GET requests. Therefore, I would recommend the simpler xhr.send().
if (this.readyState === 4) {

Add more semantic by replacing 4 with XMLHttpRequest.DONE - not available in old browsers such as IE 8.
cb('error');

This seems like a design flaw to me. A user cannot distinguish between an 'error' string received from the server vs. supplied as a result of an invalid HTTP status code. You already supply false on multiple occasions - when a network level error or timeout occurs - which is easily distinguished from a valid response by type checking. Why not supply false in the above case, too? If that's not an option, supply an error status flag or object or introduce error callback(s).

I updated your code a little bit. Have a nice day ;)

[UPDATE]:

I have used arrow functions to write cleaner and shorter code.
You can see it for example at the end of the ajax function (ontimeout and onerror)

before:

  xhr.ontimeout = function(e) {
    console.error(e);
    cb(false);
  };

after:

  xhr.ontimeout = e => reject(e);

Also i've used the spread operator to extend the opts variable.
Info: https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Operators/Spread_operator

ajax, xhr and opts changed from var to const, because theire values never change.

The ajax function now returns a Promise instead of calling a function.
Better error handling, chaining and state of the art.

I've removed all console.log calls and always return an Error Object for better handling instead of false.

const ajax = (options) => {
  const xhr = new XMLHttpRequest();

  // request options
  const opts = {
    withCredentials: false,
    method: options.method || 'GET',
    ...options
  };

  xhr.withCredentials = options.withCredentials;
  xhr.open(opts.method, opts.url);
  xhr.setRequestHeader('Accept', opts.type || 'text/plain');
  xhr.send(opts.body || null);

  // create promise to handle asynchron call
  return new Promise((resolve, reject) => {
    xhr.onreadystatechange = () => {
      if (xhr.readyState === XMLHttpRequest.DONE) {
        if (xhr.status >= 200 && xhr.status < 300) {
          resolve(xhr.responseText);
        } else {
          // return errors if status is wrong (catch)
          reject(new Error(xhr.status));
        }
      }
    }

    // return errors on timeout (catch)
    xhr.ontimeout = e => reject(e);
    xhr.onerror = e => reject(e);
  })
}

ajax({
  url: '/echo/json',
  method: 'POST', // Standard: GET
  type: 'application/json', // Standard: text/plain
  body: {  // Standard: null
    form: 'data'
  }
}).then(data => data)
  // chaining. Handle returned `data` in next `then`
  .then((data) => {
  console.log(data);
}).catch((error) => {
  console.log(error);
});


MultiThreaded TCP Server with high CPU usage I wrote simple C# TCP Server application and my program use a lot of cup, i was wondering anyone can review my code and would be grateful for any hints and suggestion.

This is my code.

public class Program
    {
        static void Main(string[] args)
        {
            int _Port = Convert.ToInt32(System.Configuration.ConfigurationManager.AppSettings["TCPPort"]);
            string _IP = System.Configuration.ConfigurationManager.AppSettings["TCPIP"];
            TcpServer server = new TcpServer(_Port);
        }
    }

  public class TcpServer
    {
        private TcpListener _server;
        private Boolean _isRunning;
        private static int defaultMax = 8;
        public TcpServer(int port)
        {
            _server = new TcpListener(IPAddress.Any, port);
            _server.Start();
            _isRunning = true;
            LoopClients();
        }

public void LoopClients()
    {
        while (_isRunning)
        {

            TcpClient newClient = _server.AcceptTcpClient();
            SentFirstTimeData_New(newClient);
            Thread t = new Thread(new ParameterizedThreadStart(HandleClient));
            t.Start(newClient);
        }
    }

    public void HandleClient(object obj)
    {

        int requestCount = 0;
        byte[] bytesFrom = new byte[209666];
        string dataFromClient = null;
        Byte[] sendBytes = null;
        requestCount = 0;
        int byteread = 0;
        string respdata = string.Empty;
        byte[] buffer = new byte[4096];
        int numberOfBytesRead;

        TcpClient client = (TcpClient)obj;
        NetworkStream networkStream = client.GetStream();
        IPAddress address = ((IPEndPoint)client.Client.RemoteEndPoint).Address;

        Boolean bClientConnected = true;
        int id = Thread.CurrentThread.ManagedThreadId;
        while (bClientConnected)
        {

            try {
                if (networkStream.DataAvailable)
                {
                    byteread = networkStream.Read(bytesFrom, 0, (int)client.ReceiveBufferSize);
                    dataFromClient = Encoding.ASCII.GetString(bytesFrom).TrimEnd('\0');
                    System.Text.Encoding.ASCII.GetString(bytesFrom);

                    XmlDocument doc = new XmlDocument();
                    string xml = Encoding.UTF8.GetString(bytesFrom).TrimEnd('\0');
                    doc.LoadXml(xml);
                    XElement incc = XElement.Load(new XmlNodeReader(doc));
                    respdata = AysncTCPIncomeString(incc);

                    sendBytes = Encoding.ASCII.GetBytes(respdata);
                    networkStream.Write(sendBytes, 0, sendBytes.Length);
                    networkStream.Flush();

                }

            }
            catch (Exception ex)
            {
                Helper.WriteLog("Exception=" + ex.Message);
            }

        }
    }

    public void SentFirstTimeData_New(TcpClient tc)
    {
        try
        {
            int threadid = Thread.CurrentThread.ManagedThreadId;
            string Msg = HLStatusResponse.FirstTimeStatusSuccessfulResponse("STATUS", "1");
            NetworkStream _NetworkStream = tc.GetStream();
            if (_NetworkStream != null)
            {
                byte[] data = Encoding.ASCII.GetBytes(Msg);
                _NetworkStream.Write(data, 0, data.Length);

                Helper.WriteLog("ThreadId=" + threadid.ToString() + ";SentFirstTimeData :\t " + Msg);
            }

        }
        catch (Exception ex)
        {
            Helper.WriteLog("Sent Though Tcp Eror :" + ex.ToString());

        }
    }

}

Big picture, you probably don't want to allocate and start a new thread to handle each incoming connection.  .Net has a very good ThreadPool implementation that scales up to as many threads as your machine can support without creating so many that performance suffers.

That said, it looks like your particular problem is that threads are never allowed to complete.  bClientConnected is the control variable for your main while loop, and it is never set to false, even if an exception is thrown.  You should always be suspicious of a catch that doesn't rethrow or at least exit the current function.

Background threads tend to run at 100% CPU utilization unless you do something to prevent it.  If your requirements allow for it, try adding a Thread.Sleep(10) (or some other number) to the code.

if (networkStream.DataAvailable)
{
    //...
}
else
{
    Thread.Sleep(10);
}

If you can't do that, when you start the new thread, try setting the thread priority to low or tweaking other related settings.


Reading input keys from a shell I was cleaning up some old code and came across this (it's to read input from a terminal):
static public int read_input_key() {

    /*
    F1 ESC O P [27 79 80]
    F2 ESC O Q [27 79 81]
    F3 ESC O R [27 79 82]
    F4 ESC O S [27 79 83]
    Up ESC [ A [27 91 65]
    Down ESC [ B [27 91 66]
    Left ESC [ D [27 91 68]
    Right ESC [ C [27 91 67]
    Home ESC [ 1 ~ [27 91 49 126]
    F5 ESC [ 1 5 ~ [27 91 49 53 126]
    F6 ESC [ 1 7 ~ [27 91 49 55 126]
    F7 ESC [ 1 8 ~ [27 91 49 56 126]
    F8 ESC [ 1 9 ~ [27 91 49 57 126]
    End ESC [ 4 ~ [27 91 52 126]
    Page-up ESC [ 5 ~ [27 91 53 126]
    Page-down ESC [6 ~ [27 91 54 126]
    Insert ESC [ 2 ~ [27 91 50 126]
    F9 ESC [ 2 0 ~ [27 91 50 48 126]
    F10 ESC [ 2 1 ~ [27 91 50 49 126]
    F11 ESC [2 2 ~ [27 91 50 50 126]
    F12 ESC [ 2 3 ~ [27 91 50 51 126]
    */

    //

    try {

        if (System.in.available() != 0) {

            int c = System.in.read();
            if (c == 27) {

                switch (System.in.read()) {
                case 79:
                    switch (System.in.read()) {
                    case 80:
                        return VK_F1;
                    case 81:
                        return VK_F2;
                    case 82:
                        return VK_F3;
                    case 83:
                        return VK_F4;
                    }
                case 91:
                    switch (System.in.read()) {
                    case 65:
                        return VK_UP;
                    case 66:
                        return VK_DOWN;
                    case 68:
                        return VK_LEFT;
                    case 67:
                        return VK_RIGHT;
                    case 49: {
                        switch (System.in.read()) {
                        case 126:
                            return VK_HOME;
                        case 53:
                            return System.in.read() == 126 ? VK_F5 : 0;
                        case 55:
                            return System.in.read() == 126 ? VK_F6 : 0;
                        case 56:
                            return System.in.read() == 126 ? VK_F7 : 0;
                        case 57:
                            return System.in.read() == 126 ? VK_F8 : 0;
                        }
                    }
                    case 52:
                        return System.in.read() == 126 ? VK_END : 0;
                    case 53:
                        return System.in.read() == 126 ? VK_PAGE_UP : 0;
                    case 54:
                        return System.in.read() == 126 ? VK_PAGE_DOWN : 0;
                    case 50: {
                        switch (System.in.read()) {
                        case 126:
                            return VK_INSERT;
                        case 48:
                            return System.in.read() == 126 ? VK_F9 : 0;
                        case 49:
                            return System.in.read() == 126 ? VK_F10 : 0;
                        case 50:
                            return System.in.read() == 126 ? VK_F11 : 0;
                        case 51:
                            return System.in.read() == 126 ? VK_F12 : 0;
                        }
                    }
                    }
                }
            } else {
                return c;
            }
        }
    } catch (IOException e) {
        // restore terminal?
        e.printStackTrace();
    }

    return 0;
}

It work's fine, but I feel like it can be done really different. To explain the long comment (I should atleast refactor that), this for example:
    F1 ESC O P [27 79 80]

Means when you press the F1 key you will get the sequence ESC O P, which is represented by the following ascii keys, 27 79 80.

I think you will be able to implement this in a cleaner way by representing the key codes as data instead of control flow. Not only will the resulting code be shorter, but it will be more abstract. If you need to add new sequences or support multiple sets of sequences, you will be able to use the same basic logic.
More specifically, a trie data structure is a good match for your problem. The mapping process is simple: start at the root, input a character, and follow the corresponding branch. If there is no branch, fail. This must be modified slightly to output the original character if it is the sequence does not start with 27.
There is no trie in the Java collections framework, so you will have to build your own. The signature should be something like
class AsciiTrie {
  AsciiTrie[] children;
  int data;
}

Given this data structure, your new algorithm is the following
private static int readInputKey(AsciiTrie t) {
  if (System.in.available() == 0) {
    return 0;
  }

  int c;
  int depth = 0;
  while (t != null) {
    try {
      c = System.in.read();
    } catch {
      // handle error properly
    }

    t = t.children[c];
    depth += 1;

    if (t.data != 0) {
      return t.data;
    }
  }

  if (depth == 1) {
    return c;
  } else {
    return 0;
  }
}

I have not included code to build or populate the trie. But it is quite similar to traversal.

What would already improve this would be to use constants instead of numbers, like this:
            int c = System.in.read();
            if (c == VK_ESC) {

                switch (System.in.read()) {
                case VK_O:
                    switch (System.in.read()) {
                    case VK_P:
                        return VK_F1;

That is, if the VK_* constants contain those, otherwise roll your own.

Another solution would be to use a class to represent each key sequence and test whether the pressed keys match any of that. Something like this:
public class KeySequence {
    protected int key = 0;
    protected int[] sequence = null;

    public KeySequence(int key, int... sequence) {
        super();

        // TODO: Validate key and sequence

        this.key = key;
        this.sequence = sequence;
    }

    public int getKey() {
        return key;
    }

    public int[] getSequence() {
        return sequence;
    }

    public boolean matches(int[] keys, int keyCount) {
        if (keyCount < sequence.length) {
            // Can't match, too few keys.
            return false;
        }

        for (int index = 0; index < sequence.length; index++) {
            if (sequence[index] != keys[index]) {
                return false;
            }
        }

        // keys started with sequence, so it matches.
        return true;
    }
}

With this your comment at the start of the function becomes logic:
List<KeySequence> keySequences = new ArrayList();
keySequences.add(new KeySequence(VK_F1, VK_ESC, VK_O, VK_P));
keySequences.add(new KeySequence(VK_F2, VK_ESC, VK_O, VK_Q));
// ...

And then, you just gather keystrokes from the terminal and test whether or not a KeySequence matches:
int[] keyBuffer = new int[16];
int keyCount = 0;

while ((keyBuffer[keyCount++] = System.in.read()) >= 0) {
    for (KeySequence keySequence : keySequences) {
        if (keySequence.matches(keyBuffer, keyCount)) {
            return keySequence.getKey();
        }
    }

    if (keyCount >= keyBuffer.length) {
        // TODO Handle that we've read too many keys.
    }

    if (keyCount >= MAX_KEYS_IN_SEQUENCE) {
        // TODO Handle that we can't find anything.
    }
}

Error handling needs to be extended.


Number of distinct elements in an array This is my solution to find the Number of distinct elements in an array. Is there away fast way to do this? It run-time is O(N).

    public int DistinctNumberOfItems( int[] A ) {

        if (A.length == 0) return 0;
        if(A.length == 1) return 1;

        Set<Integer> set = new TreeSet<Integer>();

        for (int i : A) {

            set.add(i);
    }
        return set.size();
}

Your code was better in Rev 1.  I don't know why you changed it.  Whenever you can get away without special cases, it's probably better to do so.

This code is a pure function of the input array and nothing else.  Therefore, it would probably be better as a static function.

Renaming the function from NumberOf… to …Count would make it less verbose.  You should also respect Java naming conventions, in which method names start with a lowercase letter.  The parameter name A also has unconventional capitalization.

As others have said, your algorithm, using TreeSet, is O(n log n).  You would need a HashSet for O(n).  When constructing a HashSet, it's a good idea to provide an estimate of the collection size, since a wrong guess would hurt performance.

public static int distinctItemCount(int[] array) {
    Set<Integer> set = new HashSet<Integer>(array.length);
    for (int i : array) {
        set.add(i);
    }
    return set.size();
}

Another approach is to just do it. I haven't programmed in Java in a long time, so here's how I'd do it in C++:

int number_of_distinct_elements(std::vector<int> data) {
    std::sort(data.begin(), data.end());
    auto last = std::unique(data.begin(), data.end());
    return last - data.begin();
}

One advantage of this approach is that it doesn't pound memory with a bunch of discrete allocations for a node-based container. Another is that it says what it does, instead of leaving it to the reader to figure out how the original code eliminates duplicates.

You may have to write your own counterpart to std::unique, but that's straightforward.

Note that this is a different algorithm than the one under review.


Is this the right way to space four news blocks right from each other? I am working on a website and I've created four text/image news blocks.  I am not sure if it's the right / most efficient way to do this.

See my code here at jsfiddle

HTML:

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

        fasdfasf

CSS:

body{
background: url("../img/bg.jpg") repeat-x;
}

#wrapper{
margin: 0 auto;
width: 980px;
}

#header{
float: left;
width: 100%;
height: 70px;
}

#logo img{
float: left;
margin-top: 20px;
}

#headNews{
float: left;
margin-top: 20px

}

.headNewsItem{
float: left;
margin-right: 4px;
width: 242px;
height: 184px;
}

#headNewsItemLast{
float: left;
width: 242px;
height: 184px;
}

.headNewsItem  img {
display: block;
}

#headNewsItemLast img{
display: block;
}

.textContainer{
font-family: Arial;
line-height: 21px;
font-size: 14px;
color: #c8cbcb;
height: 40px;
padding: 10px 15px;
background-image: linear-gradient(#262828,#1c1e1e);
}

#midSection{
float:left;
width: 100%;
height: 800px;
margin-top: 20px;
background-color: #FFF;

}

I wouldn't say that there's a right or a wrong way to do it.  The only problem with what you're doing is that it only works well when you have a device that is wide enough to display all of those items on one line (ie. it doesn't work so well for handheld devices).

My recommendation would be to use the CSS multi-column module, which is responsive by default without having to use media queries.

http://jsfiddle.net/cTM3L/1/

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

Lorem ipsum dolor sit amet, consectetur adipisicing elit

fasdfasf

The CSS:

@import url(reset.css);
 body {
    background: url("../img/bg.jpg") repeat-x;
}
#wrapper {
    margin: 0 auto;
}
#header {
    height: 70px;
}
#logo img {
    float: left;
    margin-top: 20px;
}
#headNews {
    margin-top: 20px;
    -webkit-columns: 242px;
    -moz-columns: 242px;
    columns: 242px;
    -webkit-column-gap: 10px;
    -moz-column-gap: 10px;
    column-gap: 10px;
    list-style: none;
    padding-left: 0;
}
.headNewsItem {
    margin-right: 4px;
    width: 242px;
    -webkit-column-break-inside: avoid;
    page-break-inside: avoid; /* Moz is weird */
    break-inside: avoid;
    margin-top: 10px;
}
.headNewsItem:first-child {
    margin-top: 0;
}
.headNewsItem a, .headNewsItem img {
    display: block;
}
.textContainer {
    font-family: Arial;
    line-height: 21px;
    font-size: 14px;
    color: #c8cbcb;
    height: 40px;
    padding: 10px 15px;
    background-image: linear-gradient(#262828, #1c1e1e);
}
#midSection {
    width: 100%;
    margin-top: 20px;
    background-color: #FFF;
}

There's quite a few other things I would change here, but most of them are not relevant to the requested reviewed.  Just make sure you're using the most specific tag that appropriately describes your content: avoid using div for everything (check out some of those HTML5 tags you seem to have missed such as header, figure/figcaption, aside, etc.).  Also, there are better ways to clear floats than to use more floats.

As I mentioned in a comment, those blocks look like a 1 row table.

I redid your code with a table and it is actually shorter ( I could take out a few divs ) and it does require less css. I also extracted the style on the image into a css rule.

http://jsfiddle.net/konijn_gmail_com/w9f6F/4/

HTML:

                   Lorem ipsum dolor sit amet, consectetur adipisicing elit

                   Lorem ipsum dolor sit amet, consectetur adipisicing elit

                   Lorem ipsum dolor sit amet, consectetur adipisicing elit

                   Lorem ipsum dolor sit amet, consectetur adipisicing elit

fasdfasf

CSS :

@import url(reset.css);
 body {
    background: url("../img/bg.jpg") repeat-x;
}
#wrapper {
    margin: 0 auto;
    width: 980px;
}
#header {
    float: left;
    width: 100%;
    height: 70px;
}
#logo img {
    float: left;
    margin-top: 20px;
}
#headNews {
    float: left;
    margin-top: 20px
}
.headNewsItem {
    float: left;
    width: 242px;
    height: 184px;
}
.headNewsItem img {
    width: 242px;
    height:124px;
}
.textContainer {
    font-family: Arial;
    line-height: 21px;
    font-size: 14px;
    color: #c8cbcb;
    height: 40px;
    padding: 10px 15px;
    background-image: linear-gradient(#262828, #1c1e1e);
}
#midSection {
    float:left;
    width: 100%;
    height: 800px;
    margin-top: 20px;
    background-color: #FFF;
}


Redacting everything in a string except a particular word I am working through the Coding Bat exercises for Java. Here is one I have just completed:

  Given a string and a non-empty word string, return a version of the original String where all chars have been replaced by pluses ("+"), except for appearances of the word string which are preserved unchanged.

And here is my solution:

public String plusOut(String str, String word){

    String s = "";
    String n = str + "  ";

    int i = 0;
    while(i < str.length()) {

        if (!(n.substring(i, i + word.length()).equals(word))) {
            s += "+";
            i++;
        } else {
            s += word;
            i += word.length();
        }
    }

    return s;
}

As you can see, I concatenated two spaces on the original string and used that to test the substrings. This was to avoid out of bounds exceptions.

What I want to do is optimise my solution, because I don't like how I added two spaces just to avoid errors. I want to change the if statement so that it tests the substrings of str rather than n, and get rid of n entirely. I have tried but I can't seem to find a way of doing it. How can I do this? What else needs optimising?

Manipulating Strings directly is a bad idea, especially when you are doing it repeatedly.  Java strings are immutable, so every time you do s += …, you are making a copy of the entire provisional result.  Anytime you need to do a substantial amount of editing on a string, use a StringBuilder.

To look for the next occurrence of word, use String.indexOf() or StringBuilder.indexOf().  It's a lot easier than extracting substrings and checking for equality.  When extracting the substring, your code crashes if word is longer than three characters.  Using .indexOf() is one way to eliminate that problem.

Here is an example of a more efficient solution:

public String plusOut(String str, String word) {
    StringBuilder s = new StringBuilder(str);

    // Find substrings to redact, from index i (inclusive)
    // to index j (exclusive)
    for (int i = 0; i < s.length(); i += word.length()) {
        int j = s.indexOf(word, i);
        if (j < 0) {
            // word not found; redact the rest of the string
            j = s.length();
        }

        while (i < j) {
            s.setCharAt(i++, '+');
        }
    }
    return s.toString();
}

200_success and DJanssens both make good points.

I want to add that, again, Regular Expressions are often useful for text manipulation. This work requires a complicated pattern if you want to keep the groups, but if you can ignore the groups, it is easier.

Note also, that StringBuilders are also more efficient than String concatenation.

Two traditional Java versions. One using a simple split, the other using a more complex one:

Simple split

public static String plusOut(String str, String word) {
    String[] parts = str.split(Pattern.quote(word));
    StringBuilder sb = new StringBuilder(str.length());
    for (int i = 0; i < parts.length; i++) {
        if (i > 0) {
            sb.append(word);
        }
        sb.append(parts[i].replaceAll(".", "+"));
    }
    return sb.toString();
}

Note the use of Pattern.quote(...) in case the input word is something like Boo(hoo)? ... we don't want the word to be interpreted as a regex pattern itself.

Complex split:

public static String plusOut(String str, String word) {
    String[] parts = str.split("(?=\\Q" + word + "\\E)|(?<=\\Q" + word + "\\E)");
    StringBuilder sb = new StringBuilder(str.length());
    for (int i = 0; i < parts.length; i++) {
        sb.append(i % 2 == 0 ? parts[i].replaceAll(".", "+") : parts[i]);
    }
    return sb.toString();
}

Java 8

In Java 8 I would consider the following:

public static String plusOut8(String str, String word) {
    return Stream.of(str.split("\\Q" + word + "\\E"))
        .map(fill -> fill.replaceAll(".", "+"))
        .collect(Collectors.joining(word));
}

See these all working in Ideone


Summarizing calories for a food diary I'm new to C and I've been working on some code for a project for school. This was the finished result and now that I'm done it, I came back and tried making it better. So I was wondering, how would I make this printAvgCalPerGroup simplified?

#include "diary.h"

void printTotalCal(Food * list)
{
     int total = 0;
     Food * temp = list;

     while (!isEmpty(temp))
     {
          total += (int)temp->calories;
          temp = temp->next; // change to the next list item
     }

     printf("%d\n", total);
     return;
}

void printAvgCalPerGroup(Food * list)
{
     double vegCount = 0.0;
     double meatCount = 0.0;
     double dairyCount = 0.0;
     double grainsCount = 0.0;
     double fatCount = 0.0;

     double vAmnt = 0.0;
     double mAmnt = 0.0;
     double dAmnt = 0.0;
     double gAmnt = 0.0;
     double fAmnt = 0.0;

     Food * temp = list;

     while (temp!= NULL)
     {
          if (temp->foodGroup[0] == 'v')
          {
               vAmnt+=1.0;
               vegCount+=temp->calories;
          }
          else if (temp->foodGroup[0] == 'm')
          {
               mAmnt+=1;
               meatCount+=temp->calories;
          }
          else if (temp->foodGroup[0] == 'd')
          {
               dAmnt+=1;
               dairyCount +=temp->calories;
          }
          else if (temp->foodGroup[0] == 'g')
          {
               gAmnt+=1;
               grainsCount+=temp->calories;
          }
          else if (temp->foodGroup[0] == 'f')
          {
               fAmnt+=1;
               fatCount+=temp->calories;
          }
          temp = temp->next;
     }
     printf("%.2f\n", calcAvg(vegCount,vAmnt));
     printf("%.2f\n", calcAvg(meatCount,mAmnt));
     printf("%.2f\n", calcAvg(dairyCount,dAmnt));
     printf("%.2f\n", calcAvg(grainsCount,gAmnt));
     printf("%.2f\n", calcAvg(fatCount,fAmnt));

}

double calcAvg(double x, double y)
{
     double ans = x/y;
     if (ans != ans) return 0.0;
     return ans;
}

void run(char * argv)
{
     Food * f = getDiary(argv);
     printTotalCal(f);
     printAvgCalPerGroup(f);
     printList(f);
     destroyList(f);
}

Reducing repetition
The big if statement in printAvgCalPerGroup is a bit repetitive (as you pointed out).  Imagine if you increased the number of food groups.  You would have to copy/paste 4 places for each food group you added.
First, I would suggest using arrays for the counts and amounts of each foodgroup.  Then you only have to convert the letter such as 'v' into an array index.
Second, if the food groups could possibly change, I would suggest using an enum to keep track of them.  This is definitely optional, and you could use hardcoded numbers instead (e.g. 'v' -> 0, 'm' -> 1, etc).
Third, a small change to make would be to track the amounts using ints instead of doubles, since the amounts always increase by 1 only.
Here is what your code would look like using arrays and enums:
typedef enum FoodGroup {
    VEG,
    MEAT,
    DAIRY,
    GRAINS,
    FAT,
    FOODGROUP_MAX
} FoodGroup;

void printAvgCalPerGroup(Food * list)
{
    double    count[FOODGROUP_MAX] = {0};
    int       amnt [FOODGROUP_MAX] = {0};
    FoodGroup group;

    Food * temp = list;

    while (temp!= NULL) {
        switch (temp->foodGroup[0]) {
            case 'v': group = VEG;           break;
            case 'm': group = MEAT;          break;
            case 'd': group = DAIRY;         break;
            case 'g': group = GRAINS;        break;
            case 'f': group = FAT;           break;
            default : group = FOODGROUP_MAX; break;
        }
        if (group < FOODGROUP_MAX) {
            amnt [group]++;
            count[group] += temp->calories;
        }
        temp = temp->next;
    }

    for(group=0; group<FOODGROUP_MAX; group++)
        printf("%.2f\n", calcAvg(count[group], amnt[group]));
}

If you don't need the enum, just replace VEG with 0, MEAT with 1, etc.

How about using alternative switch when you have a lot of if-else conditions?

Use data type int for the amount variables instead of double.

It is very simplified already.


Basic modular calculator I am new to modular JavaScript code, and after reading an article on the Internet, I wrote a very basic calculator. This works fine, but due to some unknown reason, I feel that this code is not well written. I will appreciate it if someone could improve my code below so that it will be helpful with learning modular JavaScript.

$(function () {
    $('.button').on('click', function() {
        var operator = $(this).attr('name'),
        calc = new Calculator('output', 'valOne', 'valTwo', operator);

        calc.init();
    });
});

calculator.js

var Calculator = function(eq, valone, valtwo, operator) {
    var eqCtl = document.getElementById(eq),
    valone    = document.getElementById(valone),
    valtwo    = document.getElementById(valtwo),
    op        = operator,

    init = function() {
        op   = operator;
        val1 = parseInt($(valone).val());
        val2 = parseInt($(valtwo).val());

        calculation();
    },

    setVal = function(val) {
        eqCtl.innerHTML = val;
    },

    calculation = function() {
        if(op == 'add') {
            addition(val1, val2);
        }
        else if(op == 'sub') {
            subtract(val1, val2);
        }
        else if(op == 'mult') {
            multiply(val1, val2);
        }
        else {
            division(val1, val2);
        }
    },

    addition = function(x,y) {
        return setVal(x + y);
    },

    subtract = function(x,y) {
        return setVal(x - y);
    },

    multiply = function(x,y) {
        return setVal(x * y);
    },

    division = function(x,y) {
        if( y == 0 ) {
                return setVal('cannot divide by 0');
        } else {
                return setVal(x / y);
        }
    };

    return {
        init: init
    };
};

The concept of creating a new Calculator() with a specified operator and then calling calc.init() to actually perform the operation seems a bit strange. I'd probably have any initialisation built into the constructor, and then have a calc.calculate() method that takes the operation as a parameter.

I don't have time now for a detailed analysis of your code, but one thing I would probably do is make the different operation functions methods of an object:

var operations = {
    "addition" : function(x,y) { return setVal(x + y); },
    "subtract" : function(x,y) { return setVal(x - y); },
    "multiply" : function(x,y) { return setVal(x * y); },
    // etc
}

Because then you can eliminate the if/else structure that decides what function to call and just do this:

calculation = function() {
    if (op in operations)
        operations[op](val1, val2);
    else
        // invalid op requested, so show message, throw exception, whatever
}

If you add more operations in the future, say a toThePowerOf() operation, you'd add it to the operations object but wouldn't need to change the calculation() function.

I've done this only for addition to make it more readable -

var Calculator = function() {

    this.calculate = function(eq, valone, valtwo, op) {

        var val1 = parseInt($(valone).val());
        var val2 = parseInt($(valtwo).val());

        var eqCtl = $('#' + eq),
        var operation;

        switch(op){
            case 'add' :
                operation = addition;
            break;
            case 'sub':
                //same here
            break;
            default :
                operation = division;
        }

       this.addition = function(){
            //for those who could not read the question
       }

        return operation.apply(null, [val1, val2]); // to setVal()
    },

    addition = function() {
        return setVal(arguments[0] + arguments[1]);
    }

    return this;
};

And then use as..

var calc = new Calculator();

calc.calculate('output', 'valOne', 'valTwo', operator);
//call as many times as required