Best C# implementation of IndexOfAny(string, params string[]) Follow-Up question

.Net provides String.IndexOfAny(string, char[]) but not String.IndexOfAny(string, string[]).

The existing built-in String.IndexOfAny takes an array of char and returns the lowest position of any one char from the array in a passed in string or -1 if none are found. Essentially it is the char equivalent of my naive definition.

My extension takes a string to search s and an array of strings to find targets and returns the lowest position found of any member of targets in s or -1 if none are found.

A naive implementation (using LINQ) isn't particularly efficient:

public static int IndexOfAny1(this string s, params string[] targets) =>
    targets.Select(t => s.IndexOf(t)).Where(p => p >= 0).DefaultIfEmpty(-1).Min();

My improved implementation tracks the current candidate position and restricts future searches to be before that candidate position:

public static int IndexOfAny2(this string s, params string[] targets) {
    int? curAns = null;
    foreach (var target in targets) {
        var posAns = s.IndexOf(target, 0, curAns.HasValue ? curAns.Value + target.Length : s.Length);

        if (posAns >= 0 && (!curAns.HasValue || posAns < curAns)) {
            curAns = posAns;
            if (curAns == 0) // once you're at the beginning, can't be any less
                break;
        }
    }

    return curAns ?? -1;
}

This runs up to two times faster.

Sample code to test the two methods:

Console.WriteLine($"IndexOfAny1 should be 8: {"foo bar baz".IndexOfAny1("barz", "baz")}");
Console.WriteLine($"IndexOfAny1 should be 0: {"aabbccddeeffgghh".IndexOfAny1("bbb", "hh", "aa")}");
Console.WriteLine($"IndexOfAny2 should be 8: {"foo bar baz".IndexOfAny2("barz", "baz")}");
Console.WriteLine($"IndexOfAny2 should be 0: {"aabbccddeeffgghh".IndexOfAny2("bbb", "hh", "aa")}");

Is there a better algorithm or another way to make this faster?

PS Test harness for testing random possibilities:

var r = new Random();

var sb = new StringBuilder();
for (int j1 = 0; j1 < r.Next(80,160); ++j1)
    sb.Append((char)('0'+r.Next(0, 26+52)));
var s = sb.ToString();

var listTargets = new List<string>();
for (int j1 = 0; j1 < r.Next(5, 10); ++j1)
    if (r.NextDouble() < 0.8) {
        var tLen = r.Next(4, Math.Min(s.Length - 4, 10));
        var beginPos = r.Next(0, s.Length - tLen);
        listTargets.Add(s.Substring(beginPos, tLen));
    }
    else {
        sb.Clear();
        for (int j2 = 0; j2 < r.Next(5, 10); ++j2)
            sb.Append((char)('0'+r.Next(0, 26+52)));
        listTargets.Add(sb.ToString());
    }

var targets = listTargets.ToArray();
if (s.IndexOfAny1(targets) != s.IndexOfAny2(targets))
    Console.WriteLine($"Fail on {s} containing {String.Join(",", targets)}");

Review

Don't use abbreviated variable names s, curAns and posAns; use self describing names: value, index and targetIndex instead.
The nullable int could be replaced with -1. This reads cleaner and allows you to bypass the final ?? operator in curAns ?? -1.
You can optimize the count curAns.Value + target.Length with adding -1 because we don't care about finding another match on the currently best found index.
You should exit early if no targets are specified

Refactored

public static int IndexOfAny2(this string value, params string[] targets)
{
    var index = -1;
    if (targets == null || targets.Length == 0) return index;

    foreach (var target in targets)
    {
        var targetIndex = value.IndexOf(target, 0,
            index > -1 ? index + target.Length - 1 : value.Length);

        if (targetIndex >= 0 && (index == -1 || targetIndex < index))
        {
            index = targetIndex;
            if (index == 0)
            {
                break;
            }
        }
    }

    return index;
}


Channels vs. "for loop" prime number generator I've found a question on SO (here), which is a prime number generator that uses channels. First of all, I had some trouble to find out how this code works, but later on, I saw the fun of it. Then, all the sudden, I saw the light! The code can be used perfectly to benchmark the difference between channels versus a for loop.  The question on SO uses a channel to store the total range of integers, then pulls the numbers one by one to figure out if it's prime or not. So I used my fasted code to calculate a prime number, versus the channel one. The question is, can it be improved?

This is the channel version:

package main

import "fmt"
import "time"

func Generate(ch chan<- int) {
    for i := 2; ; i++ {
        ch <- i
    }
}

func Filter(in <-chan int, out chan<- int, prime int) {
    for {
        i := <-in
        if i%prime != 0 {
            out <- i
        }
    }
}

func main() {
    ch := make(chan int)
    go Generate(ch)
    t0 := time.Now()
    for i := 0; i < 170; i++ {
        prime := <-ch
        fmt.Println(prime)
        ch1 := make(chan int)
        go Filter(ch, ch1, prime)
        ch = ch1
    }
    t1 := time.Now()
    fmt.Printf("The call took %v to run.\n", t1.Sub(t0))
}

This is my own version:

package main

import "fmt"
import "time"

func isPrime(number int) bool {
    switch {
    case number < 2:
        return false
    case number == 2:
        return true
    case number%2 == 0:
        return false
    default:
        for i := 3; (i * i) <= number; i += 2 {
            if number%i == 0 {
                return false
            }
        }
        return true
    }
}

func main() {
    t0 := time.Now()
    for i := 1; i < 1014; i++ {
        if isPrime(i) {
            fmt.Println(i)
        }
    }
    t1 := time.Now()
    fmt.Printf("The call took %v to run.\n", t1.Sub(t0))
}

The channel version runs in 21 ms. My own version runs in 11 ms.

yes it can be improved.

first, Golang is supposed to be big on concurrency. Your version is not concurrent. It will probably run on just one core.

Channels are supposed to give us concurrency for free. Here, we have a cascade of filtering channels, and while a number makes its way through a later channel, another, bigger number can well enter the earlier channel, concurrently. On multi-core, I'd expect to see high CPU utilization for this code.

The channel version can be drastically improved. Filtering by 7, say, should only be done for numbers from 49 up. There's no need to check 28 by 7—it'll be caught earlier, by a lower prime factor, 2. So the filtering channels should only be started up at primes' squares. This will radically reduce the number of channels working largely for nothing right now. The time complexity should improve from about quadratic to about n1.5, give or take.

I call it the postponement technique. I described it in this SO answer as well as many others. I first thought it up when I worked through the SICP book many years ago and tried re-writing its primes code in Haskell for fun. I thought then it must be widely known... but it turns out, it wasn't.

And of course, there's no need to enumerate any even number above 2. Here's the code:

func Sieve(out chan<- int) {   // 2,000 primes:
  out <- 2                     //  was: 1.32s now: 0.06s
  out <- 3
  q := 9
  ps := make(chan int)
  go Sieve(ps)                 // separate primes supply
  p := <-ps   // p=2, ps@3
  p = <-ps    // p=3, ps@5
  nums := make(chan int)
  go NumsFromBy(5,2,nums)
  for i := 0; ; i++ {
    n := <-nums
    if n < q {
        out <- n   // n is prime
    } else {
        ch1 := make(chan int)
        go Filter(nums, ch1, p)  // p=3
        nums = ch1
        p = <-ps  // 5
        q = p*p   // 25 !
    }
  }
}


Reading fields idiomatically This technique is more general than just for config files which is why I'm not using Config::Any in this example.

I have a config file with lines like this:

userid foo-admin # foo-comment
directory /some/dir # bar-comment

And I'm using some very awkward code to get the values.

my @userline = grep /userid/, <$configfh>;
my @userparts = split(' ', $userline[0]);
my @user = $userparts[1];

seek($configfh, 0, 0);
my @pathline = grep /directory/, <$configfh>;
my @pathparts = split(' ', $pathline[0]);
my $path = $pathparts[1];

It works! But man, is it ugly. There's got to be a clearer way to extract the values I want from the file.

If you're determined to avoid using a module for this, how about something like the following:

my %config;
while (my $line = <$configfh>) {
    chomp $line;  # Strip trailing linefeeds, you probably don't want them there
    my @fields = split(/\s+/, $line);
    $config{$fields[0]} = $fields[1] if $fields[0];
}

But perhaps your config file may contain values with whitespace? If you can require those to be wrapped in quotes, or some other convenient character, that's not a problem.

my %config;
while (my $line = <$configfh>) {
    chomp $line;
    my ($field, $value) = split(/\s+/, $line, 2);  # The 2 is how many parts to split into
    if ($value =~ /^\"/) {
        $value =~ s/^\"(.*?)\"/$1/;  # Keep what's in the first pair of quotes
    } else {
        ($value, undef) = split(/\s+/, $value);  # Drop trailing comments
    }
    $config{$field} = $value if $field;
}

(Note that the " characters don't actually need escaping, but the syntax highlighting breaks otherwise...)

Obviously there are other possible cases which this doesn't cover, but if your use cases are that extensive, you should probably reconsider and use a module.


Craps dice game simulator This craps game simulator is based on someone else's posted Python code that I've redone for practice. I'm interested in improving readability and making it more pythonic. I used unittest instead of nose because I'm using an online IDE.

from random import randrange
import unittest

class CrapsGame:
    def __init__(self):
        self.outcomes = {'2':False,'3':False,'12':False,'7':True,'11':True,
                '4&4':True,'5&5':True,'6&6':True,
                '8&8':True,'9&9':True,'10&10':True,
                '4&7':False,'5&7':False,'6&7':False,
                '8&7':False,'9&7':False,'10&7':False}

    def play(self, roll_dice):
        comeOut = str(roll_dice())
        print 'began game with ' + comeOut
        if comeOut in self.outcomes:
            return self.outcomes[comeOut]
        while True:
            point = str(roll_dice())
            print 'next roll is ' + point
            state = comeOut+'&'+point
            if state in self.outcomes:
                return self.outcomes[state]

class CrapsTest(unittest.TestCase):
    def testWinComeOut(self):
        game = CrapsGame()
        self.assertEquals(game.play(lambda: 7), True)

    def testLoseComeOut(self):
        game = CrapsGame()
        self.assertEquals(game.play(lambda: 2), False)

    def testWinPoint(self):
        game = CrapsGame()
        rollData = [5,6,5]
        self.assertEquals(game.play(lambda: rollData.pop()), True)

    def testLosePoint(self):
        game = CrapsGame()
        rollData = [7,5]
        self.assertEquals(game.play(lambda: rollData.pop()), False)

if __name__ == '__main__':
    unittest.main()

Some general comments first.

There's no need to make your throws in strings for the outcomes - you can use tuples instead. e.g.

self.outcomes = { (2,):False, (5,5):True }

If you pass a "wrong" set of dice throws (say \$[4,5]\$), you'll have an exception raised which isn't dealt with (and should probably be a test?).
pop removes the last element, which means you process them in reverse order - which differs from what the casual reader might expect (\$[7,5]\$ = 7 first, then 5).

You may want to look at generators which would provide a nice interface to a throw. In this case, what about:

class DiceThrow:
    def __init__( self, rolls = None ):
        self.rolls = rolls
    def First( self ):
        return self.__next__()
    def __iter__( self ):
        return self
    def __next__( self ):
        if self.rolls is not None:
            if len( self.rolls ) == 0:
                raise StopIteration
            r = self.rolls[0]
            self.rolls = self.rolls[1:]
            return r
        return randrange( 1, 13 ) # may be better as randint( 1, 12 )

This can then be called as follows:

game.play( DiceThrow( [5,7] ) )
game.play( DiceThrow() ) # for a random set of throws

and used:

def play(self, dice_throw):
    comeOut = dice_throw.First()
    print( "began game with %d"%comeOut )
    if (comeOut,) in self.outcomes:
        return self.outcomes[(comeOut,)]
    for point in dice_throw:
        print( "next roll is %d"%point )
        state = (comeOut,point)
        if state in self.outcomes:
            return self.outcomes[state]
    print( "Bad rolls!" )


Using Linq instead of multiple foreach loops A project I'm working on often has lists within lists within lists etc.  The original author often uses complicated linq expressions, but when adapting them I mostly get hopelessly bogged down and resort to foreach's which makes me feel like a lesser being (joke).  With an expression such as the following, what would the equivalent Linq expression be, and would you bother taking the time to make it, instead of the 'easy' foreach option.

        var participantList = new List<string>();

        foreach (var community in communities)
        {
            foreach (var tournament in community.Tournaments)
            {
                foreach (var round in tournament.Rounds)
                {
                    foreach (var match in round.Matches)
                    {
                        if (match.Home.ParticipantId.IsNotNull())
                        {
                           participantList.Add(match.Home.ParticipantId);
                        }
                        if (match.Away.ParticipantId.IsNotNull())
                        {
                           participantList.Add(match.Away.ParticipantId);
                        }
                    }

                }
            }
        }

The LINQ would be:

var participantList = communities.SelectMany(c => c.Tournaments
                                                   .SelectMany(t => t.Rounds
                                                                     .SelectMany(r => r.Matches)))
                                  .Where(m => m.Home.ParticipantId.IsNotNull() ||
                                              m.Away.ParticipantId.IsNotNull())
                                  .ToList();

LINQ does not add much imo, if the logic was more complicated the for loops are nicer to debug.

One downside with LINQ for this is that it requires formatting to be readable. If you rename things the formatting needs to be maintained.

With the foreach loops you get formatting for free.

Edit:
As per @RobH's suggestion:

var participantList = communities.SelectMany(c => c.Tournaments)
                                 .SelectMany(t => t.Rounds)
                                 .SelectMany(r => r.Matches)
                                 .Where(m => m.Home.ParticipantId.IsNotNull() ||
                                             m.Away.ParticipantId.IsNotNull())
                                 .ToList();

Chaining is indeed nicer.


C++ implementation of depth-first search I got the C++ implementation of depth-first search (DFS) from here and made slight modifications to it. The modified code is as follows:
#include "stdafx.h"
#include <iostream>
#include<map>
#include<list>

class Graph {
    void DFSUtil(int v);
public:
    std::map<int, bool> visited;
    std::map<int, std::list<int>> adj;
    void addEdge(int v, int w);
    void DFS();
};

void Graph::addEdge(int v, int w)
{
    adj[v].push_back(w);
}

void Graph::DFSUtil(int v)
{
    visited[v] = true;
    std::cout << v << " ";

    std::list<int>::iterator i;
    for (i = adj[v].begin(); i != adj[v].end(); ++i)
        if (!visited[*i])
            DFSUtil(*i);
}

void Graph::DFS()
{
    for (auto i : adj)
        if (visited[i.first] == false)
            DFSUtil(i.first);
}

int main()
{
    Graph g;
    g.addEdge(0, 1);
    g.addEdge(0, 9);
    g.addEdge(1, 2);
    g.addEdge(2, 0);
    g.addEdge(2, 3);
    g.addEdge(9, 3);

    std::cout << "Depth First Traversal result\n";
    g.DFS();

    std::cin.get();

    return 0;
}

What further improvements can I make to this code?

What further improvements can I make to this code?

The code is rather bad - the site you read it from tends to have lots of bad/amateur articles.

The graph class:
std::map<int, bool> visited;
std::map<int, std::list<int>> adj;

This is extremely poor and inefficient graph implementation. std::map creates a new memory allocation for each element. So you do it twice for both the boolean and the list. Also std::list allocates a piece of memory for each element as well.
Overuse of memory allocation means both memory fragmentation issues as well as performance issues due lots of unmanageable RAM access and indirections. Memory loading is currently one of the slowest operations on modern processors. But compilers tend to optimize it quite well if the memory is contiguous - which is furthest from the truth in this case.
To simplify the structure you'd better use std::vector<Node> where Node contains vertex's information + std::vector<size_t> of neighbors. In general, I don't believe there is a perfect solution for graph representation - instead there are numerous ways to represent and each suitable or more attuned to specific types of graphs. For instance, solutions that are good for sparse graphs are surely very poorly suited for dense graphs. Also certain additional optimizations can be made assuming that graph is fixes. The question is complex and I cannot provide an answer without knowledge of the use-case.

DFS() implementation is also pretty bad. It relies on recursion and I assure you, that for large enough connected graphs with it will blow the stack resulting in UB.

Proper DFS/BFS implementation store states-to-be-visited in a std::stack or std::queue or something similar while the visit routine occurs in a single loop instead of recursion.
Also, I presume you want some kind of output or action made on vertices from the DFS, no?


content_tag concatenation clean up I need to concatenate two content_tag and it works but it seems a little messy to me. Any thoughts on how may I refactor this?

def format_price(money)
  mny = get_price_string(money).to_money
  str = content_tag(:span, @visitor.currency.symbol.html_safe, class: 'pounds')
  str << mny.format(no_cents: true, symbol: '')
  str << content_tag(:span, mny.format[-3, 3], class: 'pence') unless condition?
end

Notes:

Before refactoring, let's identify what makes this code feel "messy". IMO there are two problems: the in-place method << and the inline statement unless. As usual, I'll try to make the case for functional programming (as opposed to imperative programming, not opposed to OOP). I guess you are not familiar with FP in Ruby but somehow you felt something was wrong. A quick intro: when you update variables in-place, the = symbol loses a lot of its glamour :-) In maths x = "hello" means that, in this scope, x is always equal to 'hello'. Period. In imperative programming it means "for now x is equal to 'hello', but I reserve the right to change it whenever I feel like it, in this scope or any other. Good luck debugging!". I don't know about you, but I prefer = to mean the first one :-) Yes, even for a such simple and short code.
I don't understand: m.format, where is m? is it mny?
ebeach?. What is ebeach??
m.format[-3, 3]. Do you want the last 3 chars of a string? In Rails: m.last(3).
@visitor: Using an instance variable in a helper is a no-no. Pass it as an argument.

So let's rewrite the snippet in functional style. There are at least two ways, 1) use different variable names for the different values and concat the strings, or 2) use an array+compact+join. The first:

def format_price(money_value, visitor)
  money = get_price_string(money_value).to_money
  symbol = content_tag(:span, visitor.currency.symbol.html_safe, class: 'pounds')
  value = money.format(no_cents: true, symbol: '')
  currency = ebeach? ? "" : content_tag(:span, money.format.last(3), class: 'pence')
  symbol + value + currency
end

Note how pure functions in FP follow a simple pattern: create intermediate variables (if needed, to make the method more readable and declarative) and use them in the last expression of the method. Don't "do things", declare "what things are".

Second approach:

def format_price(money_value, visitor)
  money = get_price_string(money_value).to_money
  [
    content_tag(:span, visitor.currency.symbol.html_safe, class: 'pounds'),
    money.format(no_cents: true, symbol: ''),
    *(content_tag(:span, money.format.last(3), class: 'pence') if condition?),
  ].join
end

Of course an alternative approach would have been to create a partial view instead of rendering from a helper.


Yet another Pascal triangle in Python written in functional programming pattern Here is my implementation of a Pascal program, which is used to print an n-order Pascal triangle. This is actually an exercise I came up with to familiarize myself with Python's Functional Programming Modules.

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

import operator
from itertools import starmap, tee
from functools import lru_cache

def pairwise(iterable):
    """
    s -> (s0,s1), (s1,s2), (s2,s3), ...
    https://docs.python.org/3/library/itertools.html#itertools-recipes
    """
    a, b = tee(iterable)
    next(b, None)
    return zip(a, b)

@lru_cache()
def pascal_row(n):
    """
    Print the nth row of a Pascal triangle
    """
    if n < 2:
        return (x for x in [1])
    else:
        def dispatch():
            yield 1
            yield from starmap(operator.add, pairwise(pascal_row(n-1)))
            yield 1
        return dispatch()

def pascal(n):
    """
    Print an n-order Pascal triangle
    """
    for i in range(1, n):
        print(pascal_row(i))

print([x for x in pascal_row(500)])

Is this the right way of functional programming? Also, I want you to tell me how to make this piece of code clearer :)

If you look at the documentation for lru_cache you'll see that it says that it can save time when an expensive or I/O bound function is periodically called with the same arguments. You keep calling pascal_row with different arguments and the cache will miss all the time. This can be seen if you put this at the end of your program the line:

print(pascal_row.cache_info())

Output:

CacheInfo(hits=0, misses=500, maxsize=128, currsize=128)

The only other thing I'd say is that you don't really need the -*- coding: utf-8 -*- line, UTF8 is the default and that's what you're using anyway.


Displaying information about an order when selection changes I feel like just because the below works doesn't mean it is correct, I want to improve it but I can't really figure out how. Besides the fact it is down right ugly I feel the performance could be increased a lot also.
Am I even using the Entity Framework and/or LINQ the way it is supposed to be used?
So I have these classes:
[Table("OrderInfo")]
public class OrderInfo
{
    public long ID {get; set;}
    public long OrderID {get; set;}
    public virtual Order Order { get; set; }
    public long ItemID {get; set;}
    public double Qty { get; set; }
    public virtual Item Item { get; set; }
}

[Table("Items")]
public class Item
{

    public Item()
    {
        this.Orders = new List<OrderInfo>();
    }
    #region Strings
    public string Color { get; set; }
    public string FullName { get; set; }
    [Column(@"Sheet/Roll")]
    public string Type { get; set; }
    public string PrimaryMachine { get; set; }
    public string Alias { get; set; }
    public string Brand { get; set; }

    #endregion

    #region Long
    public long ID { get; set; }
    public long? Weight { get; set; }
    #endregion

    #region Doubles
    public double? Size1 { get; set; }
    public double? Size2 { get; set; }
    public double? Size3 { get; set; }
    #endregion.

    public virtual ICollection<OrderInfo> Orders { get; set; }
}

below is the code calling the data:
    private void dgvOrders_SelectionChanged(object sender, EventArgs e)
    {
        DataGridView dgv = (DataGridView)sender;

        if (dgv.SelectedRows.Count > 0)
        {
            if (dgvOrderItems.DataSource != null)
                dgvOrderItems.DataSource = null;

            int ID = Convert.ToInt32(dgv["ID", dgv.SelectedRows[0].Index].Value);
            List<OrderInfo> OrderInfo = new List<OrderInfo>();
            OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

             if(OrderInfo.Count <= 0)
             {
                MessageBox.Show("No Info Found For This Order!");
                ClearForm();
                return;
             }

            lblPO.Text = "P.O. # " + OrderInfo[0].ID.ToString();
            lblRequestedBy.Text = "Requested By: " + OrderInfo[0].Order.RequestedBy;
            lblOrderDate.Text = "Ordered On: " + OrderInfo[0].Order.Date.ToShortDateString();

            dgvOrderItems.DataSource = OrderInfo.Select(x => new { x.ItemID, x.Qty, x.Item.FullName, x.Item.Brand, x.Item.Color }).ToList();
        }
    }

Architecture
One thing that jumps forward is that you're going straight from the UI to your datasource. In a three-layer design (UI, Model, Persistence) or MVC (Model, View, Controller) there is always a layer between the UI and your datasource.
A samplesetup would be this:
interface IOrderRepository {
    IEnumerable<OrderInfo> GetOrderById(int id);
}

class OrderRepository : IOrderRepository {
    public IEnumerable<OrderInfo> GetOrderById(int id){
        using(var context = new MyDataContext()){
            return context.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();
        }
    }
}

class UIController {
    private IOrderRepository _orderRepository;

    public void OnOrdersSelectionChanged(DataGridView view){
        _orderRepository.GetOrderById(Convert.ToInt32(dgv["ID", dgv.SelectedRows[0].Index].Value));
    }
}

Now your view is coupled loose from your datasource through the intermediate repository. You might be interested in reading up on the repository pattern, for example here.
Resources
I don't see a using statement in your code so I assume that your DataContext is held as a private field. This context class is explicitly made to be disposed after using it once so you should use it as such.
In the MSDN blog I linked just before the repository is disposed instead which is also an option although I have found that wrapping all calls to the DataContext is more often used.
Braces
Always add your braces to your if statements, even when it's just one line. It's simply too easy to make mistakes.
Inline declaration
This snippet
List<OrderInfo> OrderInfo = new List<OrderInfo>();
OrderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

is an elaborate way of writing
var orderInfo = c.OrderInfo.Include("Item").Where(x => x.OrderID == ID).ToList();

Notice also how I made orderInfo lowercase. Keep naming conventions in mind!


Adding labels and fields to a nested JSON I have a dataframe:

Name_ID | URL                    | Count | Rating
------------------------------------------------
ABC     | www.example.com/ABC    | 10    | 5
123     | www.example.com/123    | 9     | 4
XYZ     | www.example.com/XYZ    | 5     | 2
ABC111  | www.example.com/ABC111 | 5     | 2
ABC121  | www.example.com/ABC121 | 5     | 2
222     | www.example.com/222    | 5     | 3
abc222  | www.example.com/abc222 | 4     | 2
ABCaaa  | www.example.com/ABCaaa | 4     | 2

I am trying to create a JSON:

{"name": "sampledata",
 "children": [
{
    "name":9,
    "children":[
        {"name":4,
        "children":[
            {"name":"123","size":100}
                    ]
        }
                ]
},
{
    "name":10,
    "children":[
        {"name":5,
        "children":[
            {"name":"ABC","size":100}
                    ]
        }
                ]
},
{
    "name":4,
    "children":[
        {"name":2,
        "children":[
            {"name":"abc222","size":50},
            {"name":"ABCaaa","size":50}
                    ]
        }
                ]
},
{
    "name":5,
    "children":[
        {"name":2,
        "children":[
            {"name":"ABC","size":16},
            {"name":"ABC111","size":16},
            {"name":"ABC121","size":16}
                    ]
        },
        {"name":3,
        "children":[
            {"name":"222","size":50}
                    ]
        }
                ]
}
]
}

In order to do that:

I am trying to add labels such as "name" and "children" to the JSON while creating it.

I tried something like results = [{"name": i, "children": j} for i,j in results.items()]. But it won't label it properly, I believe.
Add another field with the label "size" which I am planning to calculate based on the formula
(Rating*Count*10000)/number_of_children_to_the_immediate_parent.

import pandas as pd
from collections import defaultdict
import json

data =[('ABC', 'www.example.com/ABC', 10   , 5), ('123', 'www.example.com/123', 9, 4), ('XYZ', 'www.example.com/XYZ', 5, 2), ('ABC111', 'www.example.com/ABC111', 5, 2), ('ABC121', 'www.example.com/ABC121', 5, 2), ('222', 'www.example.com/222', 5, 3), ('abc222', 'www.example.com/abc222', 4, 2), ('ABCaaa', 'www.example.com/ABCaaa', 4, 2)]

df = pd.DataFrame(data, columns=['Name', 'URL', 'Count', 'Rating'])

gp = df.groupby(['Count'])

dict_json = {"name": "flare"}
children = []

for name, group in gp:
    temp = {}
    temp["name"] = name
    temp["children"] = []

    rgp = group.groupby(['Rating'])
    for n, g in rgp:
        temp2 = {}
        temp2["name"] = n
        temp2["children"] = g.reset_index().T.to_dict().values()
        for t in temp2["children"]:
            t["size"] = (t["Rating"] * t["Count"] * 10000) / len(temp2["children"])
            t["name"] = t["Name"]
            del t["Count"]
            del t["Rating"]
            del t["URL"]
            del t["Name"]
            del t["index"]
        temp["children"].append(temp2)
    children.append(temp)

dict_json["children"] = children

print json.dumps(dict_json, indent=4)

Though the code does print what I need, I am looking for a more efficient and cleaner way to do the same, mainly because the actual dataset might be even more nested and complicated.

The rgp loop can be made more compact, and a bit faster, with:

def foo2(rgp):
    alist = []
    for n, g in rgp:
        temp2 = {"name": n}
        values = g.T.to_dict().values()
        n = len(values)
        def size(t):
            return (t['Rating'] * t['Count'] * 10000) / n
        temp3 = [{'name': t['Name'], 'size': size(t)} for t in values]
        temp2['children'] = temp3
        alist.append(temp2)
    return alist

I don't have enough experience with Pandas to know whether it is possible to improve on the groupby.  For example, it would be possible to perform a 2 level grouping with one call - ie. group on 'Count' and within that 'Rating'?

Considering that json is a string version of a dict, and you have a specific dictionary layout in mind, I don't see how you can organize the code in any other way.  With the exception of update, all dictionary additions are key by key.  So you have to have these 2 loops over groups.


OOP MySQLi singleton I'm starting with object oriented programming, but I know PHP procedural style "pretty well" I'd say. I'm working on a ticket support site for this website.

This mysqliSingleton class was in a question I previously visited. I only added the set_charset() method to feet my database. By the way, should that line be in the __constructor or in the init method?

class mysqliSingleton {
private static $instance;
private $connection;

private function __construct() {
    $this->connection = new mysqli(DB_SERVER, DB_USERNAME, DB_PASSWORD, DB_DATABASE);
    $this->connection->set_charset('utf8');
}

public static function init() {
    if(is_null(self::$instance)) {
        self::$instance = new mysqliSingleton();
    }
    return self::$instance;
}

public function __call($name, $args) {
    if(method_exists($this->connection, $name)) {
         return call_user_func_array(array($this->connection, $name), $args);
    } else {
         trigger_error('Unknown Method ' . $name . '()', E_USER_WARNING);
         return false;
    }
}
}

This is a piece of my ticket class which also has other methods delete, getFromUser, getAll, and some others:

class Ticket extends mysqliSingleton {
private $mysqli;
public function __construct() {
    $this->mysqli = mysqliSingleton::init();//Singleton db
}
//Submit a ticket
public function submitTicket($idusuario, $problem,$moreinfo){
    $query = "INSERT INTO tbl_name (tbl_field,tbl_problem,tbl_moreinfo) VALUES (?,?,?)";
    $stmt = $this->mysqli->prepare($query) or die($this->mysqli->error);
    if ($stmt) {
        $stmt->bind_param('iss',$idusuario,$problem,htmlentities($moreinfo));
        if ($stmt->execute()) {
            return true;
        } else return false;//Problem sql
    }
}
}

Am I using the singleton pattern right? But (unfortunately) after I code that, I starting reading more about abstract classes and Interfaces, and I've started to doubt if this is right. By the way, the ticket class is one of many classes that will extend to mysqliSingleton.

Well, although this code belongs to Codereview@stackexchenge, but as nobody cares to follow any rules here - so I would.

There are too much problems with your code, ut to outline most:

It seems you don't quite understand what singleton is and why use it.
There is no point in extending application class from database handler class.
trigger_error('Unknown Method ') is apparently redundant. PHP can handle absent methods as well.
Never use die() in production code.
This whole class makes very little sense as it don't help you to handle mysqli even a bit.

Frankly, exactly the same result you can have without any "singleton" class but just by creating mysqli instance and making it

public function __construct() {
    global $mysqli;
    $this->mysqli = $mysqli;
}


Functional vs Object Oriented Approach for UI codes I have the following snippets. which formats error messages based on a map. I can do it as a simple function call the function for transforming the error. or create a class and handle it in that way.

Functional doesn't require a an object instantiation and the message are exposed so that can imported in test to verify

Object Oriented will more structure but should be instantiated every time its accessed.

Functional

error_handler.js

export const messages = {
 500: 'Unexpected Error Occurred'
 400: 'Bad Request'
}

function transformError(error) {
 const message = messages[error.statusCode]
 return new Error(message)
}

error_handler.spec.js

it('should work for 400',() => {
  const error  = transformError({ statusCode :400 })
  expect(error.message).toEqual(messages[400]);
})

call().catch(transformError)

Object Oriented

class FormattedError {
 constructor(error){
   const messages = {
     500: 'Unexpected Error Occurred'
     400: 'Bad Request'
   }

   return new Error(messages[error.statusCode]);
 }
}

```

```
call().catch(e => new FormattedError(e))
```

```
it('should work for 400',() => {
  const error  = transformError({ statusCode :400 })
  expect(error.message).not.toBe(null)
})
```

What should be the ideal approach consider this is for UI

Object oriented doesn't really makes sense to me here. That FormattedError class doesn't even represent anything, just transforming error constructor parameter to another class.

Also you are calling constructor, that then returns instance of something else? I don't know javascript that much so maybe it's some unknown construct to me, but that looks like something terribly wrong.

Edit:

I see no relevance to code being UI code or not.


Logical functions and truth tables This program goes through 7 common logical functions (AND, OR, NOT, NOR, XOR, XNOR, and NAND) and generates and prints truth tables for them all.

#!/usr/bin/env python2
# coding:utf-8
import operator
import itertools

table = list(itertools.product([False, True], repeat=2))

ops={'AND':  ('∧', (lambda a,b:bool(operator.and_(a,b)))),
     'OR':   ('∨', (lambda a,b:bool(operator.or_(a,b)))),
     'NOT':  ('¬', (lambda a,b:bool(operator.not_(a)))),
     'NOR':  ('↓', (lambda a,b:bool(operator.not_(operator.or_(a,b))))),
     'XOR':  ('↮', (lambda a,b:bool(operator.xor(a,b)))),
     'XNOR': ('↔', (lambda a,b:bool(operator.not_(operator.xor(a,b))))),
     'NAND': ('↑', (lambda a,b:bool(operator.not_(operator.and_(a,b)))))}

pad = lambda x:(str(x) if len(str(x)) == 5 else str(x) + ' ')

for i in ops.keys():
    print i +',',ops[i][0],'\n' + '-'*25

    for j in table:
        print '|', pad(j[0]), '|', pad(j[1]), '|', pad(ops[i][1](j[0],j[1])), '|'

    print '-'*25, '\n'

I don't care much about the speed, but how can I improve the style and follow the PEP guidelines  in this program?

You should make a class. This lets you consolidate the implementation to be consistent, and simplifies a number of the details. You can also implement __str__ and __repr__ to make it easier to print them.  Use string formatting for this

class LogicalOperator(object):

    def __init__(self, name, symbol, functor):
        self.name = name
        self.symbol = symbol
        self.functor = functor

    def __call__(self, a, b):
        return bool(functor(a, b))

    def __str__(self):
        return self.name

    def __repr__(self):
        return "{}, {}".format(self.name, self.symbol)

Inheriting from namedtuple makes this easier (thanks to Caridorc for pointing out in the comments).

from collections import namedtuple

class LogicalOperator(namedtuple('LogicalOperator', 'name symbol functor')):
    # no __init__ now
    # rest is unchanged

This way you've separated your concerns. The class encapsulates "what is a logical operator and how do I make it print pretty", and then we use it as necessary to get value and formatting for it.

Then we can still create all of our logical operators, but we do it with classes and a tuple instead of a dict and tuples.

ops = (
    LogicalOperator('AND', '∧', operator.and_),
    LogicalOperator('OR', '∨', operator.or_),
    LogicalOperator('NOT', '¬', lambda a, _: not a),
    LogicalOperator('NOR', '↓', lambda a, b: not (a or b)),
    LogicalOperator('XOR', '↮', operator.xor),
    LogicalOperator('XNOR', '↔', lambda a, b: not (a ^ b)),
    LogicalOperator('NAND', '↑', lambda a, b: not (a and b))
)

A better way to pad strings is to use string.rjust. An even better way is to use the Format Specification Mini-Language.

Then we can put it all together.

if __name__ == '__main__':
    column_width = 25
    table = list(itertools.product([False, True], repeat=2))
    for op in ops:
        print(op)
        print("-"*column_width)
        for left, right in table:
            print("|{:^5}|{:^5}|{:^5}|".format(left, right, op(left, right)))
        print("-"*column_width)

Also, you could use more whitespace throughout.  Makes it easier to read.


Wet Chairs problem from Australian Informatics Olympiad I'm trying to solve this problem from the Australian Informatics Olympiad here.

(These are past questions which I'm completing for the purpose of revision, if you are worried about the integrity of the competition, and unfortunately the organisation doesn't provide answers).

Basically, when given an input with: a list of "wet" or "dry" chairs, the number of people needed to be seated, and the number of people who will ONLY sit on dry chairs, you must determine the shortest possible distance between the two people on the ends (the shortest possible distance in total taken up by the group from first person to last).

The solution I found looped through each of the chairs, and for each wet chair allocated someone who was not picky, unless there were no more of those - in which case I simply skipped the chair.  For each dry chair I allocated one picky person, or a non-picky person. There's quite a lot of looping - I've tried to refuse to the size of the data with certain conditional statements - however my solution still timed-out on many of the larger inputs (the max time allowed is 1 second for you code to complete), despite getting the correct outputs.

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;

public class Solution {

    public static void main(String[] args){
        try {
            BufferedReader bufferedReader = new BufferedReader(new FileReader("chairsin.txt"));
            String[] lineOne = bufferedReader.readLine().split(" ");
            int numberOfChairs = Integer.parseInt(lineOne[0]);
            int totalFriends = Integer.parseInt(lineOne[1]);
            int niceFriends = Integer.parseInt(lineOne[2]);
            int pickyFriends = totalFriends - niceFriends;
            String[] chairs = bufferedReader.readLine().split("");
            int shortestLength = numberOfChairs;

            for(int x = 0; x < (chairs.length - totalFriends); x++){
                int easyFriends = niceFriends;
                int hardFriends = pickyFriends;
                for(int y = x; y < chairs.length; y++){
                    if ((y - x + 1) > shortestLength){
                        break;
                    }
                    if(chairs[y].equalsIgnoreCase("w")){
                        if(easyFriends>0){
                            easyFriends--;
                        }
                        else
                            continue;
                    }
                    else{
                        if(hardFriends>0){
                            hardFriends--;
                        }
                        else if(easyFriends>0){
                            easyFriends--;
                            continue;
                        }
                    }
                    if(hardFriends == 0 && easyFriends == 0){
                        shortestLength = y - x + 1;
                        break;
                    }
                }
            }

            PrintWriter printWriter = new PrintWriter(new BufferedWriter(new FileWriter("chairsout.txt")));
            printWriter.print(shortestLength);

            bufferedReader.close();
            printWriter.close();

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

Overall, I'm just looking for ways to improve my solution - be it by optimization, or using a completely different method/algorithm to solve this problem.

A O(N) algorithm would be the sliding window method. Sliding Window means that you keep track of 2 indexes, the front and the back of the "window".

Let every dry chair be a 1 and every wet chair be a 0. At first, let front = 1 and back = 0.

While front is not out of bound (i.e. not more than C, 1-indexed), check if the number of dry chairs in the window is equal to N-K. If so, update the minimum length of the window if needed and increment the back by one. Subtract 1 to the running sum of dry chairs if the chair "removed" is a dry chair.

If not, then increment front by one. Add 1 to the running sum of dry chairs if the chair "added" is a dry chair

A pseudo-code is shown below

while front <= C
if dry == N-K                                  //If there are enough dry chairs
    if front - back < min                      //Update minimum if necessary
        min = front - back
    back = back + 1                            //Decrease size of "window"
    dry = dry - chairs[back]                   //Update number of dry chairs

else                                           //If there are not enough dry chairs
    if (front == C) break
    front = front + 1                          //Increase the size of the window (Make sure that front does not go out of bounds!)
    dry = dry + chairs[front]                  //Update number of dry chairs

Why is it O(N)? Because front is incremented exactly N times  by the time the loop ends and back is incremented at most N times. You might want to check this link for another similar explanation

What is Sliding Window Algorithm? Examples?


Make simple calculations The problem is instead of doing what is stated in the if statements, does the else line like the if statement is false. However, when printed the variables i am working with, seems i have managed to get the desired expressions. I can't see a problem with the code either so if you experienced coders can please help me. Btw there is probably better ways to do this but I am new to java so no harsh comments please. Thank you.

import java.util.Scanner;

public class Exercise13 {
    public static void main(String[] args) {
        Scanner read = new Scanner(System.in);

        System.out.println("Please enter a mathematical expression in the form '<operand> <operator> <operand>'.");

        String expression = read.nextLine();
        int spacePos1 = expression.indexOf(' ');
        int spacePos2 = expression.lastIndexOf(' ');

        String val1 = expression.substring(0, spacePos1);
        String val2 = expression.substring(spacePos2 + 1);
        String operator = expression.substring(spacePos1 + 1, spacePos2);

        int num1 = Integer.parseInt(val1);
        int num2 = Integer.parseInt(val2);

        if (operator == "+")
            System.out.println("The answer is " + (num1 + num2));
        else if (operator == "-")
            System.out.println("The answer is " + (num1 - num2));
        else if (operator == "*")
            System.out.println("The answer is " + (num1 * num2));
        else if (operator == "/" && num2 !=0)
            System.out.println("The answer is " + (num1 / num2));
        else System.out.println("The answer is " + (num1 % num2));

        System.out.println(num1);
        System.out.println(num2);
        System.out.println(operator);
    }
}

When comparing actual values use .equals instead of ==

Pay attention to the == java operator, it compares the value of two object references instead of the actual value.

For example your variables are stored in memory like this:

0x000 "string1" //stored in String operator
0x001 "string2"
0x002 "string3"
0x003 "string1" //stored in String temp
0x004 "string2"
0x005 "string3"

Performing operator == temp translates to "is 0x000 equal to 0x003?", while performing operator.equals(temp) translates to "is 'string1' equal to 'string1' ?"

In other words, use == only if you want to check if two objects are exactly the same objects in the same location in memory


Array-based MinHeap implementation in Java This is my implementation of MinHeap in Java.

public class MinHeap {

        int[] heap;
        int pos;
        int size;

        public MinHeap(){
            this.size = 2;
            heap = new int[size];
            this.pos = 0;

        }

        private void minHeapify(int index){

            int child = index;
            int parent = child/2;

            while(heap[parent] > heap[child] && parent > 0){
                int temp = heap[parent];
                heap[parent] = heap[child];
                heap[child] = temp;

                child = parent;
                parent = child/2;
            }

        }
        public void add(int item){

            if(pos == heap.length) resize();
            heap[pos++] = item;

            minHeapify(pos-1);

        }
        public void delete(int item){
            boolean found = false;
            int start = 0;
            for(int i = 0; i< heap.length; i++){
                if(heap[i] == item){
                    found = true;
                    start = i;
                    break;
                }
            }
            if(found == false) throw new IllegalStateException("Item doesn't exist.");
            pos--;
            for(int i= start; i< pos; i++){
                heap[i]= heap[i+1];
            }
            for(int i=pos; i > 0; i--){
                minHeapify(i);
            }
        }

        public int min(){

            return heap[1];
        }

        private void resize(){

            size = size*2;
            int[] curr = new int[size];
            for(int i=0; i< heap.length; i++){
                curr[i] = heap[i];
            }
            heap = curr;

        }
    }

Invite comments and suggestions to improve.

There are numerous "microstyle" issues; for example:

superfluous blank lines
missing spaces before / after symbols; e.g

  for(int i=0; i< heap.length; i++){

should be

  for (int i = 0; i < heap.length; i++) {

if(found == false) should be if (!found)
redundant (and inconsistent) use of this in the constructor.

And there are no javadocs!

Also, this looks like a bug:

    public int min(){
        return heap[1];   // should this be zero?
    }

Have you >>tested<< the code thoroughly?


A simple log that tracks hours worked and and a python script that calculates wages I wrote a simple Python script that will calculate income for a given month and hourly wage by parsing the contents of a log file. The instructions for how to use it is in my GitHub account here. The script is used in the command line with the following command

python calculate_hours.py MY_LOG.txt MONTH YEAR HOURLY_WAGE

MY_LOG.txt is the text file containing the date and the number of hours worked on that day
MONTH and YEAR are the full (not abbreviated) month and year for which you'd like the income calculated (e.g. December 2015)
HOURLY_WAGE is, well, your hourly wage (e.g. 25)

The log file must be formatted in a specific way. An entry in the log should look like the following

**Date**: 21 December 2015
**Time**: 5pm - 7pm, 12-1am
**Hours**: 2, 1

**Notes**:
+ Debugged script written yesterday
+ Checked data

The script depends on the information after **Date** and **Hours** to calculate income.

It's very simple and I was wondering if there was a way to improve it. The script looks for patterns in the log file to get the date and the hours worked.

import re
import argparse

def wage_calculator(log_txt_file, month, year, wage):

    date_pattern = "\*\*Date\*\*"  # pattern identifying the date in the file
    hours_full = []

    with log_txt_file as f:

        for line in log_txt_file:

            if re.search(r"{0}".format(date_pattern), line):  # go to the relevant line

                if re.search(month, line) and re.search(str(year), line):  # within line, go to desired month/year

                    # skips two lines to the line containing the number of hours worked
                    f.next()
                    hours_line = f.next()

                    hours_list_str = re.findall(r'[-+]?\d*\.*\d+', hours_line)  # put hours in a list
                    hours_list = [float(x) for x in hours_list_str]

                    hours_full += hours_list

                    sum_hours_date = sum(hours_list)

                    print line.rstrip()
                    print "Hours logged: " + str(hours_list)
                    print "Total hours for the day " + str(sum_hours_date) + "\n"

    hours_total = sum(hours_full)

    print "Total hours worked in {0} {1} is {2}".format(month, year,
                                                        hours_total)
    print "At ${0}/hr, your total wage for {1} {2} is ${3}".format(wage, month
                                                                   , year, hours_total * wage)

def main():

    parser = argparse.ArgumentParser()

    parser.add_argument("file",
                        help="Text file containing hours logged (e.g. ra_hours.txt)",
                        type=argparse.FileType('r')
                        )
    parser.add_argument("month",
                        help="The month for which we want the income",
                        type=str)
    parser.add_argument("year",
                        help="Enter year",
                        type=int)
    parser.add_argument("wage",
                        help="Enter hourly wage",
                        type=float)

    args = parser.parse_args()

    wage_calculator(args.file, args.month, args.year, args.wage)

if __name__ == '__main__':
    main()

I think regex is a bit much for this scale of project. I decided to try rewriting your function without using regex, and I think the result is much cleaner and more Pythonic.

In addition, I replaced your hours_full  list of lists with a single number that tracks the total. This greatly simplifies the code.

However, the general structure is essentially the same.

def wage_calculator(log_txt_file, month, year, wage):
    date_pattern = "**Date**: "  # pattern identifying the date in the file
    hours_pattern = "**Hours**: " # pattern identifying the hours in the file
    hours_total = 0.0

    with log_txt_file as f:
        for line in log_txt_file:
            if line.startswith(date_pattern):
                if month in line and str(year) in line:
                    f.next() # skip a line
                    # skip the start of the line to just get a list of hours
                    hours_line = f.next()[len(hours_pattern):]
                    hours = sum(map(float, hours_line.split(', ')))

                    hours_total += hours

                    print line.rstrip()
                    print "Hours logged: " + hours_line
                    print "Total hours for the day " + str(hours) + "\n"

    print "Total hours worked in {0} {1} is {2}".format(month, year,
                                                        hours_total)
    print "At ${0}/hr, your total wage for {1} {2} is ${3}".format(wage, month
                                                                   , year, hours_total * wage)

Beyond this, I would look into more error handling and data cleanup. For example, verify that the **Hours** line matches the **Time** line.

Also a .txt doesn't seem suitable for this data, maybe use a .csv?


strend, function that checks the occurence of a pattern at the end of a string Write a function strend(s, t) which returns 1 if the string t occurs at the end of s, and 0 otherwise.

Here is my solution:

unsigned int strend(char *source, char *pattern) {
    char *saver = pattern;

    while(*source)
        source++;

    while(*saver)
        saver++;

    while(saver >= pattern)
        if(*saver-- != *source--)
        return 0;

    return 1;
}

There are 2 while loops that will run until source and saver will point to the end of the string that they are representing. You will notice the presence of the variable saver, at the end of the second while loop this variable will have the value pattern + X, where X represent the number of charachters in the string pattern.

By using the variable saver I know when to stop the third loop. When saver is less than pattern that means that every charachter in the string pattern was compared with the coresponding one in source. At each iteration of the loop I check if the current charachter that saver points to is equal to the coresponding charachter in source.

Here is my second version:

unsigned int strend(char *source, char *pattern) {
    char *saver = pattern + strlen(pattern);
    source += strlen(source);

    while(saver >= pattern)
        if(*source-- != *saver--)
            return 0;
    return 1;
}

There is a dependency between my second implementation of strend and strlen. I, also, have some problems with the naming of the variable saver. What would be a more suggestive name for its purpose?

The exercise can be found at page 121 in K&R second edition.

I see no point in avoiding the use of Standard Library functions.  Learning to
use them is part of becoming a proficient C programmer.  Hence using strlen
is good and reinventing strcmp or memcmp, as you have done, is bad.  Making the return
value unsigned is just extra typing/noise and achieves nothing.  Here is a
simplified version:

int strend1(const char *str, const char *pattern)
{
    size_t plen = strlen(pattern);
    size_t slen = strlen(str);
    return (plen <= slen)
           && !memcmp(pattern, str + slen - plen, plen) ? 1 : 0;
}

Note that the parameters are const.


Project Euler 28: sum of spiral diagonals using recursion I've solved Project Euler #28 using recursion.

  If this 5 × 5 spiral pattern is extended to 1001 × 1001, what would be the sum of the red diagonals?

  $$\begin{matrix}
\color{red}{21} & 22 & 23 & 24 & \color{red}{25} \\
20 &  \color{red}{7} &  8 &  \color{red}{9} & 10 \\
19 &  6 &  \color{red}{1} &  2 & 11 \\
18 &  \color{red}{5} &  4 &  \color{red}{3} & 12 \\
\color{red}{17} & 16 & 15 & 14 & \color{red}{13}
\end{matrix}$$

Is this a good enough solution, or is there some other way to optimize the code?

static long GetSumofDiagonals(int cubeSize)
    {
        long sum = 0;
        if (cubeSize == 1)
            return 1;
        else
        {
            // As the diagonal numbers follows a sequence
            // UR = n^2 - 1;
            // UL = UR - (n - 1)
            // LL = UL - (n -1 )
            // LR = LL - (n - 1)
            // This works only for odd n
            long UpperRight = (long) Math.Pow(cubeSize, 2);
            long UpperLeft = UpperRight - (cubeSize - 1);
            long LowerLeft = UpperLeft - (cubeSize - 1) ;
            long LowerRight = LowerLeft - (cubeSize - 1);
            sum = UpperRight + UpperLeft + LowerLeft + LowerRight;
            return sum + GetSumofDiagonals(cubeSize - 2);
        }
    }

Performance is okay (linear), but I think the code can be simplified by using a loop instead of recursion, and by going from the inside-out instead of from the outside-in.

Here's how I did it, in pseudo-code:

sum = 1   # running sum
last = 1  # last number
delta = 2 # delta between numbers

for layer in 1 to (sidelength - 1) / 2:
    for num in 1 to 4:
        last += delta
        sum += last
    delta += 2 # increases by two with each layer

return sum

Also, I think this is one of those probems that can actually be calculated directly using pencil and paper, without any loops. Why don't you check out the problem forum there?


Transforming a list into a dictionary I have a simple list and want to filter it by a specific key. But I guess my for loop solution is not an efficient way. I'm searching an other way or one best practice.

types = [
    {
        'name':'John',
        'surname':'Doe',
        'age':36,
        'type':'customer'
    },
    {
        'name':'Jane',
        'surname':'Doe',
        'age':31,
        'type':'supplier'
    }
]

Normally I'm iterating over on list and determining according to "type" field.

new_types = {}
for t in types:
    new_types[t['type']] = t

My purpose is also below but I'm looking another iterating and filtering way for upper loop. I have created the result and expectation is:

{
    'customer':{
        'type':'customer',
        'age':36,
        'surname':'Doe',
        'name':'John'
    },
    'supplier':{
        'type':'supplier',
        'age':31,
        'surname':'Doe',
        'name':'Jane'
    }
}

Whenever you have that pattern, use a dict comprehension.

new_types = {t['type']: t for t in types}

That is more elegant and expressive, but no more performant than your original code. No matter what, you need to transform every item in the list. There's not much you can do to change that fact.


Cleaning up a string I always prefer to minimize the code, but sometimes I get into conflict with other people about the DRY concept.

In this simple example, which one would be a good choice?

function str_cleaner($str)
{
    $str = trim($str);
    $str = str_replace(".", "", $str);
    $str = str_replace(",", "", $str);
    $str = str_replace("-", "", $str);
    $str = str_replace("/", "", $str);
    return $str;
}

function str_cleaner($str)
{
    $chars = array('.', ',', '-', '/');
    foreach($chars as $char) {
        $str = str_replace($char, '', trim($str));
    }
    return $str;
}

I would say that the first one is a bit more readable, while the second one is a bit more extendable.

Note though that the functions are not actually the same. For example . a . will produce different results.

Also, personally, I would extract the array from the body of the second function to make it actually reusable:

$str = "my string";
$chars = array('.', ',', '-', '/');
$cleaned_str = str_cleaner($str, $array);

function str_cleaner($str, $chars)
{
    foreach($chars as $char) {
        $str = str_replace($char, '', trim($str));
    }
    return $str;
}

Of course, this is also more complex than it needs to be, as str_replace actually accepts an array:

$chars = array('.', ',', '-', '/');
$cleaned_str = str_replace($chars, "", $str);


Maximum path problem using Scala I am very new to Scala. I have done an exercise using Scala to solve the maximum path problem.

Basically, I have a triangle of integers, I want to find the path from the top to bottom which the numbers on the route produces the largest sum. For example:

       5
    12    3
  2    4     9
1    9    12    7

Should return:

5 -> 12 -> 4 -> 12
Sum = 33

Could someone please help review the code and make some suggestions about:

Better algorithm
Writing better scala code

Here is the code:

import scala._

class MaxPath {
  def sumTree(input : List[List[Int]], tempResult : List[List[Int]], currentlevel : Int) : List[List[Int]] = {
    if(currentlevel == input.size) return tempResult

    val updatedRow : List[Int] = currentlevel match {
      case 0 => input(currentlevel)
      case _ => {
        val lastRow = tempResult(currentlevel - 1)
        val currentRow = input(currentlevel)

        val newHead = currentRow.head + lastRow.head
        val newLast = currentRow.last + lastRow.last
        val middleSection = currentRow.drop(1).dropRight(1)

        val newMiddle : List[Int] = for {
          (value, index) <- middleSection.zipWithIndex
          middle = value + Math.max(lastRow(index), lastRow(index + 1))
        } yield middle

        newHead::newMiddle:::List(newLast)
      }
    }

    sumTree(input, tempResult:::List(updatedRow), currentlevel + 1)
  }

  def traceBack(input : List[List[Int]], transformed : List[List[Int]], index : Int, currentLevel : Int) : List[Int] = {
    if(currentLevel == input.size) return Nil

    val row = input(currentLevel)
    val rowTransformed = transformed(currentLevel)
    val max = index match {
      case -1 => {
        val maxIndex = rowTransformed.zipWithIndex.maxBy(_._1)._2
        (row(maxIndex), maxIndex)
      }
      case x  => {
        val rowSize = row.size
        x match {
          case 0 => (row.head, 0)
          case `rowSize` => (row.last, 0)
          case _ => {
            val maxIndex = List(rowTransformed(index-1), rowTransformed(index)).zip(List(index - 1, index)).maxBy(_._1)._2
            (row(maxIndex), maxIndex)
          }
        }
      }
    }

    traceBack(input, transformed, max._2, currentLevel + 1) ::: List(max._1)
  }

  def calcualte(input : List[List[Int]]) : (List[Int], Int) = {
    val transformed = sumTree(input, Nil, 0)
    val path = traceBack(input.reverse, transformed.reverse, -1, 0)
    (path, path.sum)
  }
}

object MaxPath {
  val input : List[List[Int]] = List(
    List(5),
    List(12, 3),
    List(2, 4, 9),
    List(1, 9, 12, 7)
  )

  def main(args : Array[String]) = {
    val mp = new MaxPath
    val result = mp.calcualte(input)

    println(result._1.mkString(" -> "))
    println("Sum = " + result._2)
  }
}

In terms of algorithm, I think it is best to focus on the answer you are after.  In This case you want the sum, but I notice your result returns the path and the sum.

If you want to focus on the sum think about starting from the bottom, and accessing the best path in terms of the bottom two rows and merging the results up.

Here is an example:

5
12, 3
2, 4, 9
1, 9, 12, 7

merge last 2 lines (lines 3 and 4)

2 + 9, 4 + 12, 9 + 12

merge the next line (line 2 and the merged result of 3 and 4)

12 + (4 + 12), 3 + (9 + 12)

merge the fist line (line 1 and the merged result of the 2, 3, and 4)

5 + (12 + (4 + 12))

some specific observations about your code

If you are going to access an element by index, use an indexed collection like Vector
Consider using fold or reduce instead of recursion with an accumulator
Props for using immutable variables and data structures

here is a link to my solution to the problem:
https://gist.github.com/dholbrook/6244448#file-euler18-scala


Generic implementation of the Quick-Union algorithm with Path Compression Working my way through Algorithms, Fourth edition by Robert Sedgewick & Kevin Wayne. I implemented a generic version of the Quick-Union algorithm and added Path Compression (not described in the book) to it.

Mainly I'm looking for feedback on these two aspects and whether I might have overlooked a side effect.

public class WeightedQuickUnionWithPathCompression<T> {
    private final Map<T, T> components = new HashMap<>();
    private final Map<T, Integer> treeSizes = new HashMap<>();

    public WeightedQuickUnionWithPathCompression(T[] components) {
        for (T component : components) {
            this.components.put(component, component);
            this.treeSizes.put(component, 1);
        }
    }

    public void connect(T leftComponent, T rightComponent) {
        Objects.requireNonNull(leftComponent);
        Objects.requireNonNull(rightComponent);

        if (areConnected(leftComponent, rightComponent)) {
            return;
        }

        T leftComponentTree = find(leftComponent);
        T rightComponentTree = find(rightComponent);

        int leftTreeSize = getTreeSize(leftComponentTree);
        int rightTreeSize = getTreeSize(rightComponentTree);
        if (leftTreeSize < rightTreeSize) {
            components.put(leftComponentTree, rightComponentTree);
            treeSizes.put(rightComponentTree, leftTreeSize + rightTreeSize);
        } else {
            components.put(rightComponentTree, leftComponentTree);
            treeSizes.put(leftComponentTree, leftTreeSize + rightTreeSize);
        }
    }

    private T find(T component) {
        while (!component.equals(components.get(component))) {
            components.put(component, components.get(components.get(component))); // Path compression
            component = components.get(component);
        }
        return component;
    }

    private int getTreeSize(T component) {
        return treeSizes.get(component);
    }

    public boolean areConnected(T leftComponent, T rightComponent) {
        Objects.requireNonNull(leftComponent);
        Objects.requireNonNull(rightComponent);
        return find(leftComponent).equals(find(rightComponent));
    }
}

In find() and areConnected(), you should be able to use == rather than .equals() since you are comparing references, not contents.

Your find() loop is a bit inefficient:

You do components.get(component) once during the while condition test, and again during path compression.
You do components.put(component, …) during path compression, then immediately lookup components.get(component) to advance the loop.

The remedy is to introduce two temporary variables, which I call parent and grandparent.

I've chosen to write it as a for loop with side-effect assignments, but of course you can choose to use a while loop without side-effect assignments.

private T find(T c) {
    // Lookup with path compression
    for (T parent, grandparent; (parent = components.get(c)) != c; c = grandparent) {
        components.put(c, grandparent = components.get(parent));
    }
    return c;
}

connect() calls areConnected(), which performs redundant work.  I suggest…

public void connect(T leftComponent, T rightComponent) {
    Objects.requireNonNull(leftComponent);
    Objects.requireNonNull(rightComponent);

    T leftComponentTree = find(leftComponent);
    T rightComponentTree = find(rightComponent);

    if (leftComponentTree == rightComponentTree) {
        return;     // Already connected
    }

    …
}


Quicksort - follow-up This is a follow-up to

Quicksort function

Like the previous code, this does not optimize for containers with large numbers of duplicate elements. How can I improve this code, particularly with iterators?

template<typename C>
void insertion_sort(C low, C high) {
    for (auto i = low; i < high; ++i) {
        for (auto j = i; j != low && *j < *(j-1); --j) {
            std::swap(*j, *(j-1));
        }
    }
}

template<typename C, typename I>
I partition(C& c, I low, I high) {
    auto p = *low;
    I i = low;
    I j = high;
    while (true) {
        while (i+1 != std::end(c) && *(++i) < p) {
            if (i == high) {
                break;
            }
        }
        while (p < *(--j)) { }
        if (i >= j) {
            break;
        }
        std::swap(*i, *j);
    }
    std::swap(*low, *j);
    return j;
}

template<typename C, typename I>
void sort(C& c, I low, I high) {
    if (high <= low+15) {
        insertion_sort(low, high);
        return;
    }
    I p = partition(c, low,high);
    sort(c, low, p);
    sort(c, p+1,high);
}

template<typename C>
void sort(C& c) {
    if (std::begin(c) == std::end(c)) {
        throw std::out_of_range("no size");
    }
    if (c.size() == 1) {
        return;
    }
    std::random_shuffle(std::begin(c), std::end(c));
    sort(c, std::begin(c), std::end(c));
}

Sorting an empty array is perfectly valid.

if (c.size() == 0) {
    throw std::out_of_range("no size");
}

The result id an empty array. An exception is not the correct way to go.

OK optimize. An array of size 0 or 1 is already sorted.

if (c.size() == 1) {  // I would make this c.size() <= 1 (as I would remove the exception)
    return;
}

Special casing the std::end(c) just looks wrong.

   while (i+1 != std::end(c) && *(++i) < p) {

I would say that if i+1 == high would be a better test. Also adding the ++ and * operators into the expression makes it really hard to read. I would go with:

    while ((i < high) && (*i < p)) { ++i;}

The decrementing the j pointer should look exactly the same as the increment.

This is not tested (so may have bugs).
But this is how I would write partition.

// Note: this function require std::distance(low, high) >= 3
// Note: it is called from sort when std::distance(low, high) > 15
template<typename I>
std::pair<I,I> partition(I low, I high) {
    --high;
    I   pivot   = high;

    I   highBot = low;
    I   lowTop  = high;
    --lowTop;
    while (highBot < lowTop) {
        while ((highBot < high)  && (*highBot <= *pivot))   { ++highBot;}
        while ((lowTop >= low)   && (*pivot < *lowTop))     { --lowTop;}
        if (highBot < lowTop) {
            std::swap(*highBot, *lowTop);
        }
    }
    // At this point highBot and lowTop have just passed each other.
    // Thus highBot points to the first element greater than *pivot
    // and lowTop points to the last element smaller or equal to *pivot
    swap(*highBot, *pivot);

    // Now: highBot points to the pivot value and the value that was
    // at highBot is in the top end of the high range.

    // We want to to remove pivot value from the range of elements to be sorted.
    // so we want to sort the two ranges likes this:
    //
    //      [low..lowTop+1)
    //      highBot                 The pivot point removed from further sorting.
    //                              Note: lopTop+1 == highBot
    //      [highBot+1..high)
    //

    // Small optimization
    // If the value at lowTop is also equal to the pivot value
    // Then we can remove it from further sorting.
    for(; (lowTop >= low) && (*p == *lowTop); --lowTop) {}

    return std::make_pair(lowTop+1, highBot+1);
}

Usage:

std::pair<I,I> part = partition(low, high);
sort(c, low, part.first);
sort(c, part.second, high);


Finding first recurring letter in each string of a given set First input is the number of strings ex: 5
Second input is a set of strings ex: hello world how are you

Output the letter in the string ex: 'l' in string 1

Below is my code:

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

void find_first_recurring_letter(char **str,int r,int c);

int main()
{
    int r,i;
    printf("Enter the number of strings to be entered");
    scanf("%d",&r);
    /*dynamic allocation for str*/
    char **str=(char**)malloc(r*sizeof(char*));
    for(i=0;i<r;i++)
        str[i]=(char*)malloc(50*sizeof(char));
    for(i=0;i<r;i++)
    {
        printf("Enter the string %d",i+1);
        gets(str[i]);
    }
    for(i=0;i<r;i++)
        printf("%s\n",str[i]);
    find_first_recurring_letter(str,r,50);
    for(i=0;i<r;i++)
       free(str[i]);
    return 0;
}

void find_first_recurring_letter(char **str,int r,int c)
{
    char *ptr,*sptr,ch[r];
    int indx[r];
    int i;
    for(i=0;i<r;i++)
    {
        indx[i]=0;
        ch[i]='\0';
        for(ptr=&str[i][0];*ptr!='\0';ptr++)
        {
            for(sptr=&ptr[1];*sptr!='\0';sptr++)
            {
                if(*ptr==*sptr)
                    ch[i]= *ptr;
                    indx[i]=1;
            }
        }
    }
    for(i=0;i<r;i++)
    {
        if(ch[i]!='\0')
            printf("'%c' in string %d\n",ch[i],indx[i]+1);
    }

}

Better names

The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.

Unused variables

We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.

Strictly speaking, we also never use indx[i]. If ch[i] is '\0',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace

        printf("'%c' in string %d\n",ch[i],indx[i]+1);

by

        printf("'%c' in string %d\n", ch[i], 2);

without changing the programs logic.

Simpler pointer logic

Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.

Again, we could call them current and next or similar.

By the way, all indx[i] are 1  (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.

Get rid of additional memory

If we printf the recurring character, we can get rid of ch and indx:

            if(*ptr==*sptr)
                printf("....");
                indx[i]=1;

Always use braces

Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.

You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.

If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.

Prefer late C99 declarations if possible

That way we can keep the scope of our variables short, e.g.

for(int i = 0; i < number_of_strings; ++i) {
    for(char *current = str[i]; *current != '\0'; current++) {
        for(char *next = current + 1; *next != '\0'; next++) {
            if(*current == *next) {
                printf("'%c' in string %d\n", *current, i);
            }
        }
    }
}

It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.

Use const type* for inputs that are not supposed to change

That way we cannot accidentally modify your input values, e.g.

str[i][k] = '\0'

would not compile if str was const char**.

Check malloc's return

We should really check whether malloc returns NULL.

Split functions

find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:

const char * find_first_recurring_letter(const char * haystack) {
    for(const char * current = haystack; *current != '\0'; current++) {
        for(const char * next = current + 1; *next != '\0'; next++) {
            if(*current == *next) {
                return current;
            }
        }
    }
    return NULL;
}

void find_first_recurring_letters(const char ** strings, int number_of_strings) {
    for(int i = 0; i < number_of_strings; i++) {
        const char * result = find_first_recurring_letter(strings[i]);

        if(result) {
            printf("'%c' in string %d\n", *result, i);
        }
    }
}

We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.


Generating 2 comma-separated lists from list of key value pairs Could this code:

var a = new StringBuilder();
var b = new StringBuilder();
var kvps = new List<KeyValuePair<Guid, object>>
{
    new KeyValuePair<Guid, object>(Guid.NewGuid(), 1),
    new KeyValuePair<Guid, object>(Guid.NewGuid(), "hello")
};
for (var c = 0; c < kvps.Count; c++)
{
    a.Append(kvps[c].Key);
    b.Append(kvps[c].Value);
    if (c >= kvps.Count - 1) continue;
    a.Append(",");
    b.Append(",");
}

be improved in terms of performance to achieve this:

a = e1b7978c-127f-4a5f-a17b-737e03484172,cbbd1573-950c-4ca4-94b7-72a08b4a61d0
b = 1,hello

Performance here will only matter if the input List is large. There are two things that can be done to make the output loop faster:

initialize the starting capacity of the StringBuilder to match your expectations of what size it will need to be.

In this case, you know the GUUID's are 36 characters long, with a comma, makes it 37. You should initialize that StringBuilder to ... new StringBuilder(37 * kvps.Count);. Similarly, if you have an approximate size of the average length for the other Stringbuilder, then double-it, and use it as the initial capacity constructor.
If the loop happens (very) often, you can get a very slight performance improvement by doing the following (removing the if condition from inside the loop):

foreach (KeyValuePair kv in kvps)
{
    a.Append(kv.Key);
    a.Append(",");
    b.Append(kv.Value);
    b.Append(",");
}
// remove the trailing comma.
if (kvps.Count > 0) {
    a.Length = a.Length - 1;
    b.Length = b.Length - 1;
}

It should go without saying that the names a and b are bad names for the StringBuilder instances ....


Parsing and summarizing an aviation METAR (US) report I have a list of objects, which contains field rawData (eg rawData="KLAX 141353Z 06003KT 6SM BR FEW002 FEW008 BKN280 12/12 A3007 RMK AO2 SLP182 T01170117 $"). In my case rawData is METAR report from which i want to extract the most severe weather condition (clouds, fog, rain, snow).
Currently I am applying parseWeather to each object of my list.

METAR string can contain multiple instance of same tag e.g. (BRN065 BRN033),  it even can contain all of them in same time . To my output - I want to display "simplified" worst weather condition, where the worst, for the current task, is considered as fog, then snow, then other weather conditions.

To answer why i return same type as argument - initial argument have some pre parsed detailed report from FlightAware service. However I have different requirenment for simplified report, than provided. Thus function parseWeather is a side effect function which fills extra fields.

  private PollFlightResponse.ResultEntity.MetarEntity parseWeather(
      PollFlightResponse.ResultEntity.MetarEntity d) {

    int severity = 0; //how bad is weather from 0 to 2
    String wCode = ""; // map key
    String weather = ""; // human readable weather
    if (d.getRawData().contains("FG")) {
      wCode = "FG";
    } else if (d.getRawData().contains("+SN")) {
      wCode = "+SN";
    } else if (d.getRawData().contains("SN")) {
      wCode = "SN";
    } else if (d.getRawData().contains("-SN")) {
      wCode = "-SN";
    } else if (d.getRawData().contains("+RA")) {
      wCode = "+RA";
    } else if (d.getRawData().contains("-RA")) {
      wCode = "-RA";
    } else if (d.getRawData().contains("RA")) {
      wCode = "RA";
    } else if (d.getRawData().contains("VV")) {
      wCode = "VV";
    } else if (d.getRawData().contains("OVC")) {
      wCode = "OVC";
    } else if (d.getRawData().contains("BKN")) {
      wCode = "BKN";
    } else if (d.getRawData().contains("SCT")) {
      wCode = "SCT";
    } else if (d.getRawData().contains("FEW")) {
      wCode = "FEW";
    } else if (d.getRawData().contains("NSC")) {
      wCode = "NSC";
    } else if (d.getRawData().contains("CLR")) {
      wCode = "CLR";
    }

    if (TextUtils.isEmpty(wCode)) {
      wCode = "CLR";
    }
    weather = SearchActivity.conditions.get(wCode);
    severity = SearchActivity.severity.get(wCode);
    if (d.getRawData().contains("MPS")) {
      int index = d.getRawData().indexOf("MPS");
      String wind = d.getRawData().substring(index - 2, index);
      int s = Integer.parseInt(wind);
      if (s == 0) {
        //no wind does not affect results
      } else if (s < 3) {
        weather += ", Light wind";
      } else if (s < 10) {
        weather += ", Windy";
        if (severity < 1) severity = 1;
      } else {
        weather += ", Strong wind";
        if (severity < 1) severity = 2;
      }
    }
    d.setGeneraWeather(weather);
    d.setWeatherSeverity(severity);
    return d;
  }

where conditions and severity is static public hashmaps

  private void generateWeatherMap() {
    conditions.clear();
    severity.clear();

    //Strings
    conditions.put("FG", "Fog");
    conditions.put("+SN", "Heavy snow");
    conditions.put("SN", "Snow");
    conditions.put("-SN", "Light snow");
    conditions.put("+RA", "Heavy rain");
    conditions.put("RA", "Rain");
    conditions.put("-RA", "Light rain");
    conditions.put("CLR", "Clear sky");
    conditions.put("NSC", "Little clouds");
    conditions.put("FEW", "Few clouds");
    conditions.put("SCT", "Scattered clouds");
    conditions.put("BKN", "Broken clouds");
    conditions.put("OVC", "Overcast");
    conditions.put("VV", "Heavy precipitation");
    //Values
    severity.put("FG", 2);
    severity.put("+SN", 2);
    severity.put("SN", 1);
    severity.put("-SN", 1);
    severity.put("+RA", 1);
    severity.put("RA", 1);
    severity.put("-RA", 1);
    severity.put("CLR", 0);
    severity.put("NSC", 0);
    severity.put("FEW", 0);
    severity.put("SCT", 1);
    severity.put("BKN", 1);
    severity.put("OVC", 2);
    severity.put("VV", 2);
  }

A question before

It is not clear to me how yor code works, but could as I'm not an expert of this METAR format.

The weather you find is just the last you get from the input string.

If you have, for example, an Heavy show you will miss it as you could even have a Overcast.

This part is much a question, than an answer, sorry.

Assuming your code is correct and the input string contains only one weather condition, here are my considerations.

Advise 1

You should use a for loop with the keys of SearchActivity.conditions, and break as soon as you find the conditions, avoiding matching all others.

So for example:

String weather = SearchActivity.conditions.get("CLR");

for (String condition : SearchActivity.conditions.keySet()) {
   if (d.getRawData().contains(condition)) {
       weather = SearchActivity.conditions.get(condition);
       break;
   }
}

Of course if you really need the last match, than you could not use this loop, as the Set don't preserve the order of the keys.

In this case, if you still want to have a loop, you should create an array with the conditions keys.

Advise 2

You should use constant for the condition labels:

final String CLR = "CLR";
final String FOG = "Fog";
... so on...

And use those constants in your code.

EDIT

For example in your if else:

if (d.getRawData().contains(FOG)) {
    wCode = FOG;
} else if (...
} else {
    wCode = CLR;
}

A review on the code

Your if else statement is not correct, as the last check could be just an else:

....
} else if (d.getRawData().contains("NSC")) {
  wCode = "NSC";
} else {
  wCode = "CLR";
}

As after you check if the wCode variable is empty and the you force CLR, that is the last condition.

if (TextUtils.isEmpty(wCode)) {
  wCode = "CLR";
}


Parsing a file for a game Is this code good enough, or is it stinky?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.IO;

namespace DotNetLegends
{
    public class LogParser
    {
        /// <summary>
        /// Returns a populated Game objects that has a list of players and other information.
        /// </summary>
        /// <param name="pathToLog">Path to the .log file.</param>
        /// <returns>A Game object.</returns>
        public Game Parse(string pathToLog)
        {
            Game game = new Game();

            //On actual deployment of this code, I will use the pathToLog parameter.
            StreamReader reader = new StreamReader(@"D:\Games\Riot Games\League of Legends\air\logs\LolClient.20110121.213758.log");
            var content = reader.ReadToEnd();

            game.Id = GetGameID(content);
            game.Length = GetGameLength(content);
            game.Map = GetGameMap(content);
            game.MaximumPlayers = GetGameMaximumPlayers(content);
            game.Date = GetGameDate(content);

            return game;
        }

        internal string GetGameID(string content)
        {
            var location = content.IndexOf("gameId");
            var gameID = content.Substring(location + 8, 10);
            gameID = gameID.Trim();
            return gameID;
        }

        internal string GetGameLength(string content)
        {
            var location = content.IndexOf("gameLength");
            var gamelength = content.Substring(location + 13, 6);
            gamelength = gamelength.Trim();
            var time = Convert.ToInt32(gamelength) / 60;
            return time.ToString();
        }

        internal string GetGameMap(string content)
        {
            var location = content.IndexOf("mapId");
            var gameMap = content.Substring(location + 8, 1);
            switch (gameMap)
            {
                case "2":
                    return "Summoner's Rift";
                default:
                    return "nul";
            }
        }

        internal string GetGameMaximumPlayers(string content)
        {
            var location = content.IndexOf("maxNumPlayers");
            var maxPlayers = content.Substring(location + 16, 2);
            maxPlayers = maxPlayers.Trim();
            return maxPlayers;
        }

        internal string GetGameDate(string content)
        {
            var location = content.IndexOf("creationTime");
            var creationDate = content.Substring(location + 14, 34);
            creationDate = creationDate.Trim();
            return creationDate;
        }
    }
}

You have a lot of undescriptive magic numbers and code repetition whilst retrieving the contents of a field. You could eliminate the repetition and make those numbers a little more meaningful by introducing a single method:
protected string GetFieldContent(string content, string field,
    int padding, int length)
{
    var location = content.indexOf(field);
    padding += field.Length;

    var fieldVal = content.Substring(location + padding, length);
    fieldVal = fieldVal.Trim();
    return fieldVal;
}

Use it like so:
internal string GetGameMaximumPlayers(string content)
{
    var maxPlayers = GetFieldContent(content, "maxNumPlayers", 3, 2);
    return maxPlayers;
}

Something to note here is the padding value has changed. You no longer need to include the length of the field name itself and can just describe the number of junk characters afterwards.
Padding length
Upon examining your code I noticed one peculiarity - the fields have inconsistent, magical padding lengths:

gameID padding: 2
gameLength padding: 3
mapId padding: 3
maxNumPlayers padding: 3
creationTime padding: 2

As a symptom of these being magic numbers I have no idea why this is the case. This is one of the many reasons to avoid magic numbers like the plague: it's difficult to understand their meaning. I'll trust you to evaluate whether varying padding lengths is necessary, or whether you can just assume a constant padding for all fields.
If we can assume a constant padding amount for all fields then we can change the code further a little bit to make your life easier. There are two steps to this change.
First, give your LogParser class a private field:
private const var defaultPadding = 2
Second, GetFieldContent can be refactored to produce this:
protected string GetFieldContent(string content, string field, int length)
{
    var location = content.indexOf(field);
    var padding = defaultPadding + field.Length;

    var fieldVal = content.Substring(location + padding, length);
    fieldVal = fieldVal.Trim();
    return fieldVal;
}

Then getting the contents of a field becomes simpler:
var maxPlayers = GetFieldContent(content, "maxNumPlayers", 2);


Accepting user input for tickets and tips I've done tutorials as well as searches. I need assistance in simplifying this code. Please don't do it for me- instead inform me of what, where and why you are trying to simplify or write less code. I'm already working on an if loop to exit on enter. This is just version 3 for me.

## Program gets the date, Employee ID, and amounts.
## It then places them into a text file which can be ported to excel.

## Get Date
import datetime
date = str(datetime.date.today())

## Get Employee I.D.
empnum = raw_input('Please enter Employee Number\n')

## Lists for the Users input.
amnt = []
amntt =[]

## Get Users Input and add to the appropriate list
num1 = raw_input('Please enter Ticket amount.\n')
amnt.append(num1)
numt1 = raw_input('Please enter tip amount.\n')
amntt.append(numt1)

num2 = raw_input('Please enter Ticket amount.\n')
amnt.append(num2)
numt2 = raw_input('Please enter tip amount.\n')
amntt.append(numt2)

num3 = raw_input('Please enter Ticket amount.\n')
amnt.append(num3)
numt3 = raw_input('Please enter tip amount.\n')
amntt.append(numt3)

num4 = raw_input('Please enter Ticket amount.\n')
amnt.append(num4)
numt4 = raw_input('Please enter tip amount.\n')
amntt.append(numt4)

There's no such thing as an "If" loop, but perhaps you meant a While loop? That is exactly what you need. Otherwise this looks like a pretty good start.

Next you might consider:

adding some error checking:

is the value entered a number?
What happens if the user includes currency signs?
what happens if the user enters nothing?
Is the employee number a real employee number?
what happens if the user enters a sentence instead of a number?

Are there always exactly 4 tickets or can there be more/less?


Read and output a json object with the least server processing expenditure possible On my server I have a large json that is accessed by several applications. One application only needs a certain part of the json, so presenting the whole json to the application is an unnecessary increase of bandwith. (Original JSON: about 40 KB, Optimized JSON: about 500 b)

What I came up with is a php script to which every application sends the application name. The script then reads the json on the server and prints out the shrinked json for the application to read. It works, but I'm concerned about wheter the php script could possibly put the server processing capabilities to its limits. Is there a more efficient solution over mine?

Note:
The part before the "ids" object is needed by all applications, that's why the php script just echo it out.
If an application fails to send its name to the server, the server just outprints the raw json.

The raw JSON as it exists on the server:

{
    "a": 0,
    "d": 0,
    "pD": "ok",
    "p": "ok",
    "an": "123",
    "slider_1": ["One Slider", "https://example.com/1"],
    "slider_2": ["Two Slider", "https://example.com/2"],
    "ids": {
        "Application One": {
            "info one": "210799155",
            "info two": "1544002021777"
        },
        "Application Two": {
            "info one": "211061341",
            "info two": "0000000",
        },
.
.
.
much more Applications

The PHP script to optimize:

<?php

//Read and decode the json file on the server
$txt = fread(fopen("json", "r"),filesize("json"));
$json =json_decode($txt,true);

//Echo the part needed by every applicaition
echo '{
    "a": 0,
    "d": 0,
    "pD": "ok",
    "p": "ok",
    "an": "123",
    "slider_1": ["One Slider", "https://example.com/1"],
    "slider_2": ["Two Slider", "https://example.com/2"],
    "ids": { "' . $_GET["id"] . '" :';

$s = json_encode($json["ids"][$_GET["id"]]);

//check if the application failed to send the name
if ($s === "null"){
ob_end_clean();
echo $txt;
exit();
}else{
//write the application specific part of the json
echo $s;
}

echo "}}";

The code can be slightly improved, but it won't make much of a difference overall. This whole approach seems a bit weird. Why does the JSON file exist in the first place?

Here is the 'improved' version:

<?php

// read the json file on the server
$txt = file_get_contents("json");

// get the id of the application
$id = filter_input(INPUT_GET, "id", FILTER_SANITIZE_STRING);
if (isset($id)) {
    // decode json
    $json = json_decode($txt, true);
    // remove all other ids from the list
    $appInfo = $json["ids"][$id];
    unset($json["ids"]);
    $json["ids"][$id] = $appInfo;
    // encode and return json
    echo json_encode($json);
}
else {
  // simply echo the content
  echo $txt;
}

I sanatized the input by using filter_input(). That's not really needed, but always a good idea. I also simplified the code a bit more.

This is only a tiny piece of code, so there's not much I can do. I have a feeling that more could be achieved if we knew why you do this in the first place. What is this JSON used for?

=============================================

I couldn't resist proposing the code below for the "filechanger".

<?php

// read the json file on the server
$txt = file_get_contents("json");
// decode json
$json = json_decode($txt, true);
// get all application information
$appInfos = $json["ids"];
// write the to separate files
foreach ($appInfos as $appId => $appInfo) {
    // set application info
    unset($json["ids"]);
    $json["ids"][$appId] = $appInfo;
    // create file name
    $filename = str_replace(" ", "_", $appId) . ".json";
    // encode and save json
    file_put_contents($filename, json_encode($json));
}


Smart Pointers Queue Implementation To practice around with C++11 smart pointers I was trying to implement a simple Queue to go beyond a simple Linked List. The fact that the _first and _last nodes are pointed to twice at runtime made me decide to implement them as shared_ptr.
I was wondering if this was the correct way to implement the data structure.

#include <iostream>
#include <memory>

template <class T>
class Queue
{
private:
    class Node
    {
    public:
        const T _data;
        std::shared_ptr<Node> _next;
        Node(const T& t) : _data{ t } {}
    };

    std::shared_ptr<Node> _first;
    std::shared_ptr<Node> _last;
    unsigned _size;
public:
    Queue() : _size{ 0 }, _first { nullptr }, _last{ nullptr } {}
    ~Queue() {}
    void add(const T& item);
    const T remove();
    const unsigned size();
    const T peek();
    bool isEmpty();
};

template <class T>
bool Queue<T>::isEmpty()
{
    return _first == nullptr;
}

template <class T>
void Queue<T>::add(const T& item)
{
    std::shared_ptr<Node> new_end = std::make_shared<Node>(item);
    if (_last)
    {
        _last->_next = new_end;
    }
    _last = new_end;
    if (!_first)
    {
        _first = _last;
    }
    ++_size;
}

template <class T>
const T Queue<T>::remove()
{
    if (_first == nullptr) return NULL;
    auto ptr = std::move(_first);
    _first = ptr->_next;
    if (!_first)
    {
        _last.reset();
    }
    T val = ptr->_data;
    ptr.reset();
    --_size;
    return val;
}

template <class T>
const unsigned Queue<T>::size()
{
    return _size;
}

template <class T>
const T Queue<T>::peek()
{
    if (_first) return _first->_data;

    return NULL;
}

Consistency
Always use the same technique when doing the same thing. Do use multiple ways to do the same thing.
You use nullptr in half your code and NULL in the other half. Be consistent and move to the modern nullptr.
Design
Not sure I agree with creating a container by using a smart pointer. These are both types of memory management. Using one to implement the other seems a bit counterproductive. But others have done it so why not.
Design Semantics
Your design does not do what I would expect on a copy.
Queue<int>  q1;
q1.add(1);
Queue<int>  q2(q1);
q1.add(2);

// In the case I would expect q2 to be a copy of q1 before we add 2.
// I would expect the two lists to be independent (If I wanted the
// to represent the same queue I would have used a reference).
// Your class both objects refer to the same underlying list. Thus
// at the end both lists have two elements in them.

Note: You basically relied on the rule of zero. When you need to implement the rule of five (three).
Rule of Zero: You don't do resource management because that is done by a resource class you use (in your case std::shared_ptr). The default constructors/assignment/destructors use the members versions automatically and thus save you from doing work.
Rule of Five: You override the default constructor/assignment/destructors to handle resource management and define the copy/move semantics of your class.
Leading Underscore.
Your use of leading underscore is technically fine. BUT the rules for using a leading underscore are more complex than most people know so prefer to avoid its use.
Also it looks ugly as hell. If you are trying to separate local variables from member variables a better technique is to give the variables obvious descriptive names.
Error:
const T Queue<T>::remove()
{
    if (_first == nullptr) return NULL;

This only works if the type T can be constructed by passing NULL (since NULL can be converted to 0 anything that takes an integer can also be constructed).
If you there is nothing to remove from the Queue then your code (that calls the Queue) has an error in it so in this case you should be throwing an exception of something along these lines.
Overkill
This is a lot of extra work:
T val = ptr->_data;
ptr.reset();
--_size;
return val;

You can replace this with:
--_size;
return ptr->_data;
// The destructor of ptr (which is a smart  pointer) will be called
// and tidy up the data and resources).