Is there a better way to convert to a specific type with reflection? Rather than doing what is essentially a large switch statement for every possible type, is there a better more generic way of converting to a specific type with reflection?  I've looked up TypeConverter but don't understand the documentation.

if (header.Property.PropertyType == typeof(Int32))
{
  header.Property.SetValue(instanceOfTrade, value.ToInt(), null);
}
else if (header.Property.PropertyType == typeof(decimal))
{
  header.Property.SetValue(instanceOfTrade, value.ToDecimal(), null);
}
else if (header.Property.PropertyType == typeof(DateTime))
{
  header.Property.SetValue(instanceOfTrade, value.TryToDateTime(), null);
}
else
{
  header.Property.SetValue(instanceOfTrade, value.ToString(), null);
}

You could use an extension method (if this is common), or a regular generic method with a "IConvertible" constraint on the desired value then call "Convert.ChangeType" in your SetValue call.
static class ObjectExtensions
{
    public static void SetPropertyValue<T>(this object obj, string propertyName, T propertyValue)
        where T : IConvertible
    {
        PropertyInfo pi = obj.GetType().GetProperty( propertyName );

        if( pi != null && pi.CanWrite )
        {
            pi.SetValue
            (
                obj,
                Convert.ChangeType(propertyValue, pi.PropertyType),
                null
            );
        }
    }
}

class TestObject
{
    public string Property1 { get; set; }
    public int Property2 { get; set; }
}

void Main()
{
    TestObject o = new TestObject();

    // Propery1 == null, Property2 == 0
    o.SetPropertyValue( "Property1", 1 );
    o.SetPropertyValue( "Property2", "123" );
    // Propery1 == 1, Property2 == 123
}

Obviously no error handling and this is assuming you want it available on all types, so I just threw it in an "ObjectExtensions" class so it'll be visible on all types.
Just adjust the constraints to fit your exact needs, or just throw it in a regular class if you don't want to use extensions.


Is this Python pathfinder efficient enough? I'd like to see if my code follows common practice, or is already efficient or fast enough.

from random import randint
w=[[2,0,0,0,0],[0,0,2,0,0],[0,0,1,0,0],[0,0,0,0,0],[0,0,0,0,0]] #1 represent Mr.Pacman, #2 represent food.
for y in xrange(len(w)): #Finding the coordinate of Mr.Pacman
    for x in xrange(len(w[0])):
        if (w[y])[x]==1:
            hor=x
            ver=y
            break

horbound=len(w[0])
verbound=len(w)
def letsCout(xCurrent,yCurrent,xTarget,yTarget,xBoundary,yBoundary,checkedSquare):
    if (xCurrent+1==xTarget and yCurrent==yTarget) or (xCurrent-1==xTarget and yCurrent==yTarget) or (xCurrent==xTarget and yCurrent+1==yTarget) or (xCurrent==xTarget and yCurrent-1==yTarget):
        return 1
    else:
        battle=[9999,9999,9999,9999]
        d=[]
        d.extend(checkedSquare)
        d.append([xCurrent,yCurrent])
        if xCurrent-1>-1 and [xCurrent-1,yCurrent] not in d: #going left
                battle[0]=letsCout(xCurrent-1,yCurrent,xTarget,yTarget,xBoundary,yBoundary,d)+1
        if xCurrent+1<xBoundary and [xCurrent+1,yCurrent] not in d: #going righ
                battle[1]=letsCout(xCurrent+1,yCurrent,xTarget,yTarget,xBoundary,yBoundary,d)+1
        if yCurrent-1>-1 and [xCurrent,yCurrent-1] not in d: #going up
                battle[2]=letsCout(xCurrent,yCurrent-1,xTarget,yTarget,xBoundary,yBoundary,d)+1
        if yCurrent+1<yBoundary and [xCurrent,yCurrent+1] not in d: #going down
                battle[3]=letsCout(xCurrent,yCurrent+1,xTarget,yTarget,xBoundary,yBoundary,d)+1
        battle.sort()
        return battle[0]
#
happy=[9999,9999,9999,9999]
for y in xrange(len(w)): #Checking the coordinate of a food. If multiple food exist, the method will find the food with the smallest cost.
    for x in xrange(len(w[0])):
        if (w[y])[x]==2:
            if hor-1>-1: #going left
                if (w[ver])[hor-1]==2:
                    happy[0]=1
                else:
                    candidate=[happy[0],letsCout(hor-1,ver,x,y,horbound,verbound,[[hor,ver]])+1]
                    candidate.sort()
                    happy[0]=candidate[0]
            if hor+1<horbound: #going right
                if (w[ver])[hor+1]==2:
                    happy[1]=1
                else:
                    candidate=[happy[1],letsCout(hor+1,ver,x,y,horbound,verbound,[[hor,ver]])+1]
                    candidate.sort()
                    happy[1]=candidate[0]
            if ver-1>-1: #going up
                if (w[ver-1])[hor]==2:
                    happy[2]=1
                else:
                    candidate=[happy[2],letsCout(hor,ver-1,x,y,horbound,verbound,[[hor,ver]])+1]
                    candidate.sort()
                    happy[2]=candidate[0]
            if ver+1<verbound: #going down
                if (w[ver+1])[hor]==2:
                    happy[3]=1
                else:
                    candidate=[happy[3],letsCout(hor,ver+1,x,y,horbound,verbound,[[hor,ver]])+1]
                    candidate.sort()
                    happy[3]=candidate[0]

wow=["left","right","up","down"]
while True: #The decision. The lowest cost direction for next move will be picked. If there is tie for lowest cost, I want to pick it randomly
    poop=randint(0,3)
    for x in xrange(4):
        if happy[poop]>happy[x]+1:
            break
    else:
        print wow[poop]
        break

I honestly see this project as a dynamic programming task.  Is this code good enough?

About your code following common practices, you may read about PEP8.

You have far too many lines with more than 80 characters, you even have a line with 180+ characters: Does not fit my screen.

You may, as PEP8 states, put spaces around binary operators.

You also have an undocumented function with a name that does not helps me.

Seems you have copy/pasted some code many times, that's not DRY, and that may be bad, you may win some readability by using a function.

You should not use this kind of control flow:

for y in xrange(len(w)):
    for x in xrange(len(w[0])):
        if (w[y])[x]==2:
            if hor-1>-1: #going left
                if (w[ver])[hor-1]==2:

That's freaking unreadable, and I'll NOT read it. Avoid imbricate so many control flow.


Add lines from files to corresponding dictionaries I have to iterate through multiple text files. For each file I read its contents and append each line to its corresponding dictionary to then build a JSON file.
Each text file has the following structure:

Line 1: The title key.
Line 2: The name key.
Line 3: The date key.
Line 4: The feedback key.

Here is an example of two of these files:
001.txt

Great Customer Service
John
2017-12-21
The customer service here is very good. They helped me find a 2017 Camry with good condition in reasonable price. Compared to other dealers they provided the lowest price. Definitely recommend!

002.txt

You will find what you want here
Tom
2019-06-05
I've being look around for a second handed Lexus RX for my family and this store happened to have a few of those. The experience was similar to most car dealers. The one I ended up buying has good condition and low mileage. I am pretty satisfied with the price they offered.

My approach is successful but I wonder if there is a better and faster approach of joining each line to its corresponding dictionary.
Additionally do I need to write with open('file', 'r') for each file? Even when I use os.listdir() I still have the same issue.
import json

l1 = []
l2 = []

with open("C:/Users/user/Desktop/001.txt") as file1, open("C:/Users/user/Desktop/002.txt") as file2:
    for line1, line2 in zip(file1, file2):
      if not line1.isspace() and not line2.isspace():
        l1.append(line1.rstrip())
        l2.append(line2.rstrip())

Dict = {}

Dict['dictio1'] = {'title': "", "name": "", "date": "", "feedback": ""}

Dict['dictio2'] = {'title': "", "name": "", "date": "", "feedback": ""}

Dict['dictio1']["title"] = l1[0]
Dict['dictio1']["name"] = l1[1]
Dict['dictio1']["date"] = l1[2]
Dict['dictio1']["feedback"] = l1[3]

Dict['dictio2']["title"] = l2[0]
Dict['dictio2']["name"] = l2[1]
Dict['dictio2']["date"] = l2[2]
Dict['dictio2']["feedback"] = l2[3]

with open('file.json', 'w') as file_json:
    json.dump(Dict, file_json, indent=2)

{
  "dictio1": {
    "title": "Great Customer Service",
    "name": "John",
    "date": "2017-12-21",
    "feedback": "The customer service here is very good. They helped me find a 2017 Camry with good condition in reasonable price. Campared to other dealers they provided the lowest price. Definttely recommend!"
  },
  "dictio2": {
    "title": "You will find what you want here",
    "name": "Tom",
    "date": "2019-06-05",
    "feedback": "I've being look around for a second handed Lexus RX for my family and this store happened to have a few of those. The experience was similar to most car dealers. The one I ended up buying has good condition and low mileage. I am pretty satisfied with the price they offered."
  }
}

There are some ways you can improve your code:

Rather than building a dictionary and then manually assigning each value you can assign to l1[0] etc straight away.
Dict['dictio1'] = {'title': "", "name": "", "date": "", "feedback": ""}
Dict['dictio1']["title"] = l1[0]
Dict['dictio1']["name"] = l1[1]
Dict['dictio1']["date"] = l1[2]
Dict['dictio1']["feedback"] = l1[3]

Dict["dictio1"] = {
    "title": l1[0],
    "name": l1[1],
    "date": l1[2],
    "feedback": l1[3],
}

You should use a for loop over the paths and have the with inside it. Only building one dictionary at a time.
for key, path in ...:
    with open(path) as f:
        lines = []
        for line in f:
            if not line.isspace():
                lines.append(line.rstrip())
        Dict[key] = {
            "title": l1[0],
            "name": l1[1],
            "date": l1[2],
            "feedback": l1[3],
        }

We can use a list comprehension to build lines with some sugar.
lines = [line.rstrip() for line in f if not line.isspace()]

Putting this all together we can get:
data = {}
paths = [
    ("dictio1", "C:/Users/user/Desktop/001.txt"),
    ("dictio2", "C:/Users/user/Desktop/002.txt"),
]
for key, path in paths:
    with open(path) as f:
        lines = [line.rstrip() for line in f if not line.isspace()]
        data[key] = {
            "title": lines[0],
            "name": lines[1],
            "date": lines[2],
            "feedback": lines[3],
        }

with open('file.json', 'w') as file_json:
    json.dump(data, file_json, indent=2)

I would recomend you change your JSON structure to remove the outer dictionary and instead use a list. This would make all your code simpler not only building it here but consuming it later.
This would look like:
data = []
paths = [
    "C:/Users/user/Desktop/001.txt",
    "C:/Users/user/Desktop/002.txt",
]
for path in paths:
    with open(path) as f:
        lines = [line.rstrip() for line in f if not line.isspace()]
        data.append({
            "title": lines[0],
            "name": lines[1],
            "date": lines[2],
            "feedback": lines[3],
        })

with open('file.json', 'w') as file_json:
    json.dump(data, file_json, indent=2)


two integers one line calculator I created this simple one line calculator for two numbers only where user can input any two numbers and and operator in between

for example 20+45 and computer will return result of 65.

using System;
using System.Collections.Generic;
using System.Text;
using System.Threading.Tasks;

namespace SimpleCalculator
{
    class Program
    {
        static void Main(string[] args)
        {
            ShowOutput();
            string user_input = UserInput();
            int result = PerformCalculation(InputToList(user_input));
            Console.WriteLine($"{user_input}={result}");
            Console.Read();
        }

        static void ShowOutput()
        {
            Console.WriteLine("Enter numbers followed by operation eg. 1+2-4");
        }

        static string UserInput()
        {
            string User_input = Console.ReadLine();
            return User_input;
        }

        static string[] InputToList(string input)
        {
            string number1 = "";
            string number2 = "";
            string Oprt = ""; //Mathematical operator
            string[] Arithmetic = new string[3];
            int n = 0;
            foreach (char charecter in input)
            {
                int num;
                bool isNumerical = int.TryParse(charecter.ToString(), out num);
                n += 1;
                if (isNumerical)
                {
                    number1 += num;
                }
                else
                {
                    Oprt = charecter.ToString();
                    Arithmetic[0] = number1;
                    Arithmetic[1] = Oprt;
                    for(int i = n; i <= input.Length - 1; i++)
                    {
                        number2 += input[i];
                    }
                    Arithmetic[2] = number2;
                }

            }
            return Arithmetic;
        }

        static int PerformCalculation(string[] Input)
        {
            int result = 0;
            switch (Input[1])
            {
                case "+":
                    result = Int32.Parse(Input[0]) + Int32.Parse(Input[2]);
                    break;
                case "-":
                    result = Int32.Parse(Input[0]) - Int32.Parse(Input[2]);
                    break;
                case "*":
                    result = Int32.Parse(Input[0]) * Int32.Parse(Input[2]);
                    break;
                case "/":
                    result = Int32.Parse(Input[0]) / Int32.Parse(Input[2]);
                    break;
            }
            return result;
        }
    }
}

I'll go through your program from top to bottom, mentioning some details.

static void Main(string[] args)
{
    ShowOutput();
    string user_input = UserInput();
    int result = PerformCalculation(InputToList(user_input));
    Console.WriteLine($"{user_input}={result}");
    Console.Read();
}

Starting with the Main method is good practice. The reader gets an overview about what the whole program is supposed to do.

Starting the program with ShowOutput() is confusing. The usual sequence of events is: Input — Processing — Output. By showing the output first, you are reversing this usual sequence of events.

static void ShowOutput()
{
    Console.WriteLine("Enter numbers followed by operation eg. 1+2-4");
}

Instead of ShowOutput this method should better be named ShowPrompt. This is more specific, and prompting the user clearly belongs to the input phase.

static string UserInput()
{
    string User_input = Console.ReadLine();
    return User_input;
}

Typically method names start with a verb, like in ShowOutput above. The method UserInput should rather be called ReadLine since that is exactly what happens here.

static string[] InputToList(string input)
{
    string number1 = "";
    string number2 = "";
    string Oprt = ""; //Mathematical operator
    string[] Arithmetic = new string[3];

Now it is getting more complicated, and inconsistencies appear. Some of your variables start with a lowercase letter, some with an uppercase letter. They should all start with a lowercase letter. That's by convention.

The usual abbreviation for operator is op. The letters prt remind me of the PrtScr key on the keyboard, which means Print Screen.

From reading the code until here, I have no idea what the variable Arithmetic might be. Granted, it's not easy to find a name for it. It could be something like "calculation atoms", "calculation parts", "words", "things", "tokens". When continuing to read the program, a better name might become apparent. This variable should then be renamed.

    int n = 0;
    foreach (char charecter in input)
    {
        int num;
        bool isNumerical = int.TryParse(charecter.ToString(), out num);
        n += 1;
        if (isNumerical)
        {
            number1 += num;
        }
        else
        {
            Oprt = charecter.ToString();
            Arithmetic[0] = number1;
            Arithmetic[1] = Oprt;
            for(int i = n; i <= input.Length - 1; i++)
            {
                number2 += input[i];
            }
            Arithmetic[2] = number2;
        }
    }

This code is long and tricky and fragile. In the upper part you parse number1 until you find a character (not charecter) that is not numeric. In that moment you save the current number1 into the result array. You can be lucky that you do this in exactly this moment, because later the variable number1 will be overwritten again, as soon as the number2 is parsed.

    return Arithmetic;
}

There must be some simpler way of expressing this idea. Imagine you explain this to a human friend. You would hopefully not choose to explain the code above, but some simpler code. To parse an expression that consists of numbers and operators:

parse a number
parse an operator
parse a number
continue with step 2

This is a high-level view on the whole topic of parsing an expression. That's how your code should look. The method should be called TryParseExpr. The current name InputToList is too unspecific.

static int PerformCalculation(string[] Input)

As Zoran already mentioned in his answer, performing a calculation on strings is inefficient and sounds strange. Calculations should be performed on numbers.

Converting the string parts into numbers and operators should be done by the TryParseExpr method I suggested above.

{
    int result = 0;
    switch (Input[1])
    {
        case "+":
            result = Int32.Parse(Input[0]) + Int32.Parse(Input[2]);
            break;
        case "-":
            result = Int32.Parse(Input[0]) - Int32.Parse(Input[2]);
            break;
        case "*":
            result = Int32.Parse(Input[0]) * Int32.Parse(Input[2]);
            break;
        case "/":
            result = Int32.Parse(Input[0]) / Int32.Parse(Input[2]);
            break;
    }
    return result;
}

This style of int result = 0; ...; result = the actual result; ...; return result; leads to long code. In most cases the code becomes easier to understand when the result is not saved in a variable but returned directly. Such as in:

static int Calculate(int left, char op, int right)
{
    switch (op)
    {
        case '+':
            return left + right;
        case '-':
            return left - right;
        case '*':
            return left * right;
        case '/':
            return left / right;
        default:
            throw new ArgumentException($"unknown operator {op}");
    }
}

This is as simple as it gets. There are no unnecessary arrays, and each parameter has the correct data type.

In your original code you suggest to the user that they may enter 1+2-4, but this is something your current code cannot handle. Given the above Calculate method, it's not too difficult to extend the calculation to an arbitrary amount of numbers and operators:

static int Calculate(IReadOnlyList<int> nums, IReadOnlyList<char> ops)
{
    int res = nums[0];
    for (int i = 0; i < ops.Count; i++)
        res = Calculate(res, ops[i], nums[i + 1]);

    return res;
}

In this method the calculation is performed strictly from left to right. The usual operator precedence (* before +) is ignored. That's only for simplicity. It could be added later.

This extended Calculate method can be used like this:

// 1 + 2 - 4
Console.WriteLine(Calculate(new List<int> {1, 2, 4}, new List<char> {'+', '-'}));

Now the remaining task is to let the user enter a line and convert this line into these two lists of numbers and operators. This is the job of a lexer. A lexer takes a string (or another input source) and repeatedly looks at the beginning to split off a small piece of data, such as a number or an operator.

The general idea I outlined above in the 4 steps can be written in C# like this:

public bool TryParseExpr(out List<int> nums, out List<char> ops)
{
    nums = new List<int>();
    ops = new List<char>();

    if (!TryParseInt(out int firstNum))
        return false;
    nums.Add(firstNum);

    while (TryParseOp(out char op))
    {
        ops.Add(op);

        if (!TryParseInt(out int num))
            return false;
        nums.Add(num);
    }

    return true;
}

Each of the paragraphs in this method corresponds roughly to one of the steps from above. Here they are again, for comparison:

parse a number
parse an operator
parse a number
continue with step 2

Now the only missing part are the basic building blocks, TryParseInt and TryParseOp. These I present together with the whole program that I built from your code:

using System;
using System.Collections.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Tests
{
    [TestClass]
    public class Program
    {
        [TestMethod]
        public void Test()
        {
            TestOk("1", 1);
            TestOk("12345", 12345);
            TestOk("12345+11111", 23456);
            TestOk("2147483647", int.MaxValue);
            TestOk("1+2+3+4+5+6", 21);
            TestOk("1+2-3+4-5+6*5", 25);

            TestError("2147483648", "2147483648");
            TestError("a", "a");
            TestError("1+2+3+4+5+a", "a");
        }

        static void TestOk(string input, int expected)
        {
            Lexer lexer = new Lexer(input);
            Assert.AreEqual(true, lexer.TryParseExpr(out List<int> nums, out List<char> ops));
            int result = Calculate(nums, ops);
            Assert.AreEqual(expected, result);
        }

        static void TestError(string input, string expectedRest)
        {
            Lexer lexer = new Lexer(input);
            Assert.AreEqual(false, lexer.TryParseExpr(out List<int> nums, out List<char> ops));
            Assert.AreEqual(expectedRest, lexer.Rest);
        }

        static int Calculate(IReadOnlyList<int> nums, IReadOnlyList<char> ops)
        {
            int res = nums[0];
            for (int i = 0; i < ops.Count; i++)
                res = Calculate(res, ops[i], nums[i + 1]);

            return res;
        }

        static int Calculate(int left, char op, int right)
        {
            switch (op)
            {
                case '+':
                    return left + right;
                case '-':
                    return left - right;
                case '*':
                    return left * right;
                case '/':
                    return left / right;
                default:
                    throw new ArgumentException($"unknown operator {op}");
            }
        }
    }

    // The lexer takes a string and repeatedly converts the text at the
    // current position into a useful piece of data, like a number or an
    // operator.
    //
    // To do this, it remembers the whole text and the current position
    // of the next character to read. It also remembers the length of the
    // text, but this is only for performance reasons, to avoid asking for
    // text.Length again and again.
    class Lexer
    {
        private readonly string text;
        private int pos;
        private readonly int end;

        public Lexer(string text)
        {
            this.text = text;
            end = text.Length;
        }

        public string Rest => text.Substring(pos);

        public void SkipSpace()
        {
            while (pos < end && char.IsWhiteSpace(text[pos]))
                pos++;
        }

        public bool TryParseInt(out int num)
        {
            int i = pos;

            // The number may have a single sign.
            if (i < end && (text[i] == '-' || text[i] == '+'))
                i++;

            // After that, an arbitrary number of digits.
            while (i < end && char.IsDigit(text[i]))
                i++;

            // The TryParse handles the case of too many digits (overflow).
            bool ok = int.TryParse(text.Substring(pos, i - pos), out num);
            if (ok)
                pos = i;

            return ok;
        }

        public bool TryParseOp(out char op)
        {
            if (pos < end)
            {
                switch (text[pos])
                {
                    case '+':
                    case '-':
                    case '*':
                    case '/':
                        op = text[pos];
                        pos++;
                        return true;
                }
            }

            op = '\0';
            return false;
        }

        public bool TryParseExpr(out List<int> nums, out List<char> ops)
        {
            nums = new List<int>();
            ops = new List<char>();

            if (!TryParseInt(out int firstNum))
                return false;
            nums.Add(firstNum);

            while (TryParseOp(out char op))
            {
                ops.Add(op);

                if (!TryParseInt(out int num))
                    return false;
                nums.Add(num);
            }

            return true;
        }
    }
}

You can play around with this code by adding more and more test cases. There's also a method called SkipSpace that is currently unused. To allow the user to enter 1 + 2 - 4 as well, your parsing code should skip the space before and after each number or operator.


Dictionary application I am creating an application which needs to validated user input to ensure the words being entered are real words. What improvements if any can I make?

class MyDictionary {

    Vector<String> words = new Vector<>();

    public MyDictionary() {
        URL url;
        try {
            url = new URL("http://www.example.com/hugewordlist.txt");
            URLConnection uc = url.openConnection();
            BufferedReader br = new BufferedReader(new InputStreamReader(uc.getInputStream()));
            char[] buffer = new char[1024];
            int i = 0;
            StringBuffer b = new StringBuffer();
            int readBytes = 0;
            while ((readBytes = br.read(buffer, i, 1024)) != -1) {
                i += 1024;
                b.append(buffer, 0, readBytes);
            }
            for (String w : b.toString().split("\n")) {
                words.add(w);
            }
            br.close();
        } catch (MalformedURLException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public boolean hasWord(String word) {
        return words.contains(word);
    }

    public int size() {
        return words.size();
    }
}

What do you want to do when there are no words? The current solution isn't satisfactory at all. You're simply failing silently, and hasWord will always return false. By the way, if there's an exception in the middle of of the loop, will your file be closed?
Searching in a Vector takes O(n) time. This means that if you double the size of the vector, it will take twice as long. Other containers such as TreeSet have O(log n) access time: doubling their size will only slow the process by one iteration. HashSet has O(1) access time. Your lookups will be way faster.
Do you know that in a standard text unknown words account for ~10% of your text?


Insertion sorting an int array I'd like to improve this Insertion sort code

package com.arun.sort;

import java.util.Arrays;

public class InsertionSort {

public static void main(String[] args) {

    int[] arr = { 9, 4, 6, 2, 1, 7 };

    insertionSort(arr);
    System.out.println("Elements after sorting :" + Arrays.toString(arr));

}

public static int[] insertionSort(int[] arr) {

    int value, hole;

    for (int i = 1; i < arr.length; i++) {
        value = arr[i];
        hole = i;

        while (hole > 0 && arr[hole - 1] > value) {
            arr[hole] = arr[hole - 1];
            hole--;
        }

        arr[hole] = value;
    }
    return arr;
   }
 }

Step 1: extract to a method

In the current code you have a hardcoded array, and the main logic follows right after. It's hard to test this way. What if you want to see if the implementation works with a different set of numbers? You have to rewrite the array. Better to extract the main logic into its own, independent method:

int[] sort(int[] arr) {
    // ...
}

Now you can test with multiple different inputs easier:

arr = new int[]{2, 5, 1, 8, 12, 3, 7};
InsertionSort.sort(arr);
System.out.println(Arrays.toString(arr));

arr = new int[]{4, 3, 2, 1, 2};
InsertionSort.sort(arr);
System.out.println(Arrays.toString(arr));

Step 2: convert print statements to proper unit tests

The problem with print statements is that every time you change something and rerun, you have to re-verify the output of each statement. Unit tests can automate the verification step, and converting is easy enough to do:

private void sort(int[] arr) {
    InsertionSort.sort(arr);
}

@Test
public void testMixedValues() {
    int[] arr = {2, 5, 1, 8, 12, 3, 7};
    sort(arr);
    assertEquals("[1, 2, 3, 5, 7, 8, 12]", Arrays.toString(arr));
}

@Test
public void testDecreasingValues() {
    int[] arr = {4, 3, 2, 1};
    sort(arr);
    assertEquals("[1, 2, 3, 4]", Arrays.toString(arr));
}

Btw, I didn't type the expected strings in the assertEquals. I wrote the test cases first, with "" as the expected values, and ran the tests. All the tests failed, of course, but the error messages told me the actual values that were different from the expected "". I verified that they are correct and copy-pasted the correct texts into the test cases.

Now you can make changes and the test cases will flag an error if something breaks. Unless you do something really horrible, typically only a few of the test cases will break, and you don't need to reverify the others that are still working, which makes debugging a lot easier.

Return void instead of int[]

It might be better to return void instead of int[]. The problem with returning int[] is that the caller might wonder if the returned array is a new array or not. It is not a new array. For example in this code:

int[] arr1 = { 9, 4, 6, 2, 1, 7 };
int[] arr2 = InsertionSort.sort(arr1);
arr2[3] = -1;

Both arr1 and arr2 will be modified, which might be counter-intuitive.

The fact that java.util.Collections.sort() also returns void suggests that it's better that way.

Minor things

The method name insertionSort is redundant, because the word "insertion" is already in the class name. How about simply sort?


Pairing Labels with Images when building a UI In a prototype 4X space game that I am working on recently, I have two different scroll panes to show the ships and the stars of the player.  The ships are displayed in a vertical scroll pane on the right side of the screen, and the stars are displayed in a horizontal scroll pane running along the bottom. Here is a picture for the sake of clarity:

With the vertical scroll pane, displaying the labels along with the images is very straightforward. When iterating over the list of ships, a label is added for each ship, then a row, then the ship image.

The horizontal scroll pane presents a problem, however.  I want to have the label displayed above the corresponding image.  Accomplishing this is a bit more complicated than simply adding a row between each element in the table.

I thought about having two lists, iterating over the first list and getting the correct element from the second list based on the index.  Instead of doing this, I went with a Map<Label, Image>.  I'm not sure about this decision, and would love feedback on this approach.  It feels wrong to be using a Map here, because I will never be using it to look up the values based on the keys.

First, I have this method that creates the contents that will be used when building the scroll pane:

private void createContents() {
    this.contents = new HashMap<Label, Image>();

    for (Star star : this.game.getPlayerStars()) {

        Image starImage = new Image(this.libGDXGame.starTextures.get(star.type));
        starImage.addListener(new StarClickListener(star) {
            @Override
            public void clicked(InputEvent event, float x, float y) {
                StarScrollPane.this.gameScreen.playerClickedStar(star);
            }
        });

        Label starLabel = new Label(star.type.name(), this.libGDXGame.skin);

        this.contents.put(starLabel, starImage);
    }
}

Then I have another method that actually creates the table and adds the contents to it like this:

    for (Label label : this.contents.keySet()) {
        this.starTable.add(label);
    }

    this.starTable.row();

    for (Image image : this.contents.values()) {
        this.starTable.add(image).width(width).height(height);
    }

By adding all of the labels to the table, then one row, then all of the images, they line up perfectly.

A HashMap does not guarantee any order whatsoever, so that is a bad idea to use.

Your Image and Label belongs together, so make a class where they actually do belong together.

public class StarView {
    private final Label label;
    private final Image colorImage;
    // constructor and stuff...
}

private void createContents() {
    this.contents = new ArrayList<StarView>();

    for (Star star : this.game.getPlayerStars()) {

        Image starImage = new Image(this.libGDXGame.starTextures.get(star.type));
        starImage.addListener(new StarClickListener(star) {
            @Override
            public void clicked(InputEvent event, float x, float y) {
                StarScrollPane.this.gameScreen.playerClickedStar(star);
            }
        });

        Label starLabel = new Label(star.type.name(), this.libGDXGame.skin);

        this.contents.add(new StarView(starLabel, starImage));
    }

    for (StarView starView : this.contents) {
        this.starTable.add(starView.label);
    }

    this.starTable.row();

    for (StarView starView : this.contents) {
        this.starTable.add(starView.image).width(width).height(height);
    }
}

This could be combined with @Vogel's suggestion about changing the layout manager. Or you could make the StarView extend Actor and put it in a horizontal group.


Click handlers for edit and save buttons I have the following code from a JavaScript file:

$("body").on("click",".edit",function(){
    var list=$(this).closest("a").data("list");
    var seira=$(this).closest("a").data("order");
    //alert(navbar_edit[list][seira]);
    var edit=navbar_edit[list][seira];
    navbar_edit[list][seira]=prompt("Change property",edit);
    exportLists();

});
$("body").on("click",".edit-save",function(){
    var property=$(this).closest("a").text();
    property=property.substring(0,property.length-4);
    alert(property);
    var list=$(this).closest(".list-group").attr("id");
    list=list.substring(list.length-1);
    navbar_edit[list].push(property);
    exportLists();
});

As you can see, I use $("body") 3 times to execute 3 different actions. Is there any way to use it only once so I can reduce my code?

A few notes:

Always use .slice over .substring (Yes .slice works on strings.), it will support negative selection af strings, eg "abc".slice(0, -1) // "ab"
Storing reused values / objects in variables cleans up the code a lot, eg:

var a = $(this).find('.element').attr('a');
var b = $(this).find('.element').attr('b');
// The above can be written as:
var $element = $(this).find('.element');
var a = $element.attr('a');
var b = $element.attr('b');
// This is faster and is IMO easier to read.

Meaningful variable names, dont use names like seira when you are referring to an index in an array call it index or list_index etc. (I assume its meaningful in another language but IMO all code should be written in english with english comments, and english variable names.)

Code:

$(document.body).on('click', '.edit', function() {
    var $a = $(this).closest('a');
    var list_id = $a.data('list');
    var index = $a.data('order');

    var current_value = navbar_edit[list][index];
    var new_value = prompt('Change Property', current_value);

    navbar_edit[list][index] = new_value;

    exportLists();
});
$(document.body).on('click', '.edit-save', function() {
    var $a = $(this).closest('a');
    var $listGroup = $(this).closest('.list-group');

    var value = $a.text().slice(0, -4);
    var list_id = $listGroup.attr('id').slice(0, -1); // remove last char

    navbar_edit[list_id].push(value);

    exportLists();
});


AngularJS constants depending on other constants - any established patterns? I'd like to be able to define Angular constants in terms of other constants.

It's not possible to inject dependencies into Angular constant services.  When I want one of my constants to depend on another of my constants, I use a provider service.  This allows my constants to have dependencies (on other constants), but still be used in my .config method call.

This sets off my code smell for two reasons.

I'm using a full-blown call to provider when I'm really just defining a constant.  It's just that some constants are defined in terms of other constants, and I want to be able to use any of these constants in my .config call.
I'm injecting providers into my .config call and then manually calling their $get methods.  This seems to just be bypassing some of the tools Angular gives me.  It seems to me if I'm calling $get in my own code, it may be a sign that I'm fighting the framework.

Here's some sample code that shows the above in action.  It defines a constant service that's an enumeration type, then it defines another constant service that depends on the enumeration type, and last it calls the .config method, injecting the dependent constant service:

app.constant('authLevel', {
    Public: "Public",
    Admin: "Admin",
    SuperAdmin: "SuperAdmin"
});

app.provider("routes", function (authLevel) {

    var routes = [
        {
            path: "/",
            controller: "mapsController",
            templateUrl: "templates/mapsView.html",
            navBarLabel: "LEFT_NAV_BAR.MAPS_NAV_LABEL",
            navBarIconClass: "fa-map-marker",
            authLevels: [authLevel.Public, authLevel.Admin, authLevel.SuperAdmin],
        },
        {
            path: "/markers",
            controller: "markersController",
            templateUrl: "templates/markerListView.html",
            navBarLabel: "LEFT_NAV_BAR.TABLE_NAV_LABEL",
            navBarIconClass: "fa-table",
            authLevels: [authLevel.SuperAdmin],
        }
    ];

    this.$get = function () {
        return routes;
    };
});

app.config(function ($routeProvider, routesProvider) {

    var routes = routesProvider.$get();

    for (var i = 0; i < routes.length; i++) {
        var route = routes[i];
        $routeProvider.when(route.path, {
            controller: route.controller,
            templateUrl: route.templateUrl
        });
    }

    $routeProvider.otherwise({ redirectTo: "/" });
});

Am I right to have my code smell go off here?

Are there any sort of established patterns for defining Angular constants that use previously defined constants, and can still be used in the .config method?

Using an IIFE seems to be the tidiest way I've found to do it. This allows you to define your constant as a function (that is immediately invoked) that returns your constant object so you can use it as normal, i.e: LABELS.appName

.constant('LABELS', (function () {
    var self = {};
    self.appName = 'MyApp';
    self.companyName = 'MyCompany';
    self.appHeader = self.appName + ' | ' + self.companyName;
    return self;
})());//function is immediately invoked here

References:

SO answer
Use functions (IIFE) to define AngularJS constants and values


Store account numbers as text I wrote this code to make sure account numbers (accounting), which are numbers, are stored as text in the range. Replacing r.value by r.text does not seem to work on whole ranges, so I need to loop. Any better idea?

Sub num2text(r As Range, Optional keepFormat As Boolean)
'change numbers to text, optionally leaving the numberFormat unchanged
    Dim oldFormat
    Dim c As Range
    For Each c In r
        If IsNumeric(c) Then
            If keepFormat Then oldFormat = c.NumberFormat   'save num fmt
            c.NumberFormat = "@"
            c.Value = c.Text
            If keepFormat Then c.NumberFormat = oldFormat   'restore num fmt
        End If
    Next c
End Sub

Sub num2text_test()
    num2text Range("b10:b100")
End Sub

I updated the code to include @TinMan's remarks.
I also added a safety net by intersecting the range in parameter with the UsedRange to avoid scanning 1 million empty cells if a full column is provided as a parameter.

The most effective change is probably using .SpecialCells(xlCellTypeConstants, xlNumbers): I tried on about 100.000 rows in various configurations:
- all numbers: makes little difference
- just a few numbers: goes from 18 sec to 0.02 sec!

Finally I turned of recalc during the macro.

Sub num2text(rng As Range, Optional keepFormat As Boolean)
'change numbers to text, optionally leaving the numberFormat unchanged
    Dim oldFormat As String, t
    Dim c As Range, i As Long, r As Range
    On Error GoTo finito    'in case selection contains no numeric constants
    Set r = Intersect(rng, rng.Parent.UsedRange).SpecialCells(xlCellTypeConstants, xlNumbers)
    On Error GoTo 0
    Application.Calculation = xlCalculationManual
    For Each c In r
        If keepFormat Then oldFormat = c.NumberFormat   'save num fmt
        t = c.Text
        c.NumberFormat = "@"
        c.Value = t
        If keepFormat Then c.NumberFormat = oldFormat   'restore num fmt
    Next c
    Application.Calculation = xlCalculationAutomatic
finito:
End Sub

Sub selection2text()
    num2text Selection
End Sub


Bash script for tagging the current git workspace including uncommitted changes I use the following script to autotag the current workspace while building even if there are changed (not committed) files:

#!/bin/bash

tagname="autobuild-test1"
commitname="autocommit for tag $tagname"

set -e
head="`git rev-parse HEAD`"
test -n "$head"
git commit -m "$commitname" -a
taghead="`git rev-parse HEAD`"
git tag "$tagname" "$taghead"
#or just `git tag "$tagname" HEAD` for the 2 lines above
git reset "$head"

The idea: I commit everything which is not committed to a 'temporary commit' and tag its hash. After doing this, I revert to the original state by resetting to the previous HEAD.

In other words: I try to do git commit -a without touching HEAD, staging, stashing and workspace.

Pros:

this also works if the workspace is dirty
the script is very flexible
the script doesn't touch any files, doesn't do any checkout or remove operations

Cons:

the staging state is completely cleared after running the script
probably more state modifications which I didn't find yet
it is a script. I would prefer a single command for this use case
no proper error handling. if the script fails, HEAD might be changed

This script was inspired by cvs2git which uses the same strategy in cases where it's not clear to which revision a tag belongs to.

Is there anything you would change to make this script more rubust and/or to remove some of the cons?

With the input of @EtanReisner, I found a possible way to the solution:

That's our current situation:

$ git status
On branch master
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

    new file:   c
    new file:   e

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    deleted:    c
    modified:   e

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    d

I try now to stash all this:

$ git stash --all

Now we can get the hash of it:

$ git rev-parse refs/stash
9a208ed215032b4e47cb1493fc5f16a4a5ea78c1

Or tag it directly (this works)

$ git tag tagged-workspace-01 refs/stash

Now I unstage everything

$ git stash pop --index

This worked quite well:

$ git status
On branch master
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

    new file:   c
    new file:   e

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   e

Untracked files:
  (use "git add <file>..." to include in what will be committed)

    d

As you can see, under some (very unusual) conditions it happens that the revert is not 100% perfect (the not-staged removal of the file c which is also in the index as a new file), but so far it's the best solution I've found.


Simple server using async and HttpListener I have a program that controls a custom machine. I wanted to add some simple HTTP comms so I could control it from the network or other programs.

My server class is:

public class Server
{
    private volatile bool stop = true;
    private Action<string> methodOne;

    public Server(Action<string> methodOne)
    {
        this.methodOne= methodOne;
    }

    public async Task StartAsync()
    {
        var prefix = "http://localhost:5005/";
        HttpListener listener = new HttpListener();
        listener.Prefixes.Add(prefix);
        try
        {
            listener.Start();
            stop = false;
        }
        catch (HttpListenerException hlex)
        {
            return;
        }
        while (listener.IsListening)
        {
            var context = await listener.GetContextAsync();
            try
            {
                await ProcessRequestAsync(context);
            }
            catch (Exception ex)
            {
                Console.WriteLine("# EXCEPTION #   " + ex.StackTrace);
            }
            if (stop == true) listener.Stop();
        }
        listener.Close();
    }

    public void Stop()
    {
        stop = true;
    }

    private async Task ProcessRequestAsync(HttpListenerContext context)
    {
        // Get the data from the HTTP stream
        var body = await new StreamReader(context.Request.InputStream).ReadToEndAsync();
        HttpListenerRequest request = context.Request;

        if (request.RawUrl.StartsWith("/methodOne"))
        {
            //Get parameters
            var options = context.Request.QueryString;
            var keys = options.AllKeys;
            //Run function
            methodOne("some method parameter");
            //Respond
            byte[] b = Encoding.UTF8.GetBytes("ack");
            context.Response.StatusCode = 200;
            context.Response.KeepAlive = false;
            context.Response.ContentLength64 = b.Length;
            var output = context.Response.OutputStream;
            await output.WriteAsync(b, 0, b.Length);
            context.Response.Close();
        }
    }
}

I initialise the Server by passing it the delegate for the function in the main view model that will be called, then start itwith server.StartAsync();

VS2017 gives me some advice that I didn't await server.StartAsync(); so thread will continue, but that is the functionality I want.

Any problems with making a server like this, using async and having it run on the main thread? All the server does is call methods in the view model, and there will usually be just one connection. The advantage for me is that being all on the same thread I don't have to worry about dispatching etc.

Visual Studio will warn if you don't use the Task object returned from the function, either immediately with an await statement or by storing it in a variable.  The reason is that this is the only connection you have with the Task that you have started, and without it there is no way to find out if it has completed or to get the return value.

Stylistically, methodOne is not a good name for that variable, something like handler or callback would make more sense.  It also might make sense for the signature to be a little more complex, like accepting a dictionary of parameters and returning an object to serialize to the response.

The other thing to realize is that the called function doesn't know what you do or don't do with the Task, so its behavior can't depend on that.  Therefore "using async and having it run on the main thread" does not accurately describe what happens in your situation.  The task is run asynchronously on a background task either way, but in your case the main thread does not block on its completion.  If you step in with a debugger, you'll see that the code up to the first await (in the while loop) is executed on the calling thread, and at that point control returns to the caller, who could then maintain a reference to the Task if it wants.

As for whether async and Task are right for your job: it sounds like your needs are pretty modest and that there isn't a real argument to be made for or against.  If there aren't many requests coming in, it doesn't really matter how you handle them.  If there could be a lot coming in and you wanted to limit the impact on the rest of the system, maybe it would make sense to do the work synchronously on a single thread (that you create for that specific purpose).


Find the longest rectilinear path in a matrix with descending entries Problem (Skiing):

  Each number represents the elevation of that area of the mountain.

  From each area (i.e. box) in the grid, you can go north, south, east,
  west - but only if the elevation of the area you are going into is
  less than the one you are in (i.e. you can only ski downhill).

  You can start anywhere on the map and you are looking for a starting
  point with the longest possible path down as measured by the number of
  boxes you visit.

  And if there are several paths down of the same length, you want to
  take the one with the steepest vertical drop, i.e. the largest
  difference between your starting elevation and your ending elevation.

  Goal:

  Find the longest path following these rules. Longest path means the maximum number of nodes where we are always going down.

  If we have many longest paths with equal distance (number of nodes), then we should get the one with the maximum drop (value of 1st
  node - value of the last node).

The aim is to run this code for a 1000 x 1000 matrix. As for now, running for just a 4 x 4, it's taking minutes. Any idea on how to reduce the complexity regarding time and space?

myMatrix = [[4, 5, 2],[1, 1, 6],[8, 7, 5]]
def getAllValidSkiingPathsFrom(myMatrix):
    dctOfMatrix = {}
    for row in range(len(myMatrix)):
        for column in range(len(myMatrix[0])):
            currPoint = (column, row)
            dctOfMatrix[currPoint] = myMatrix[row][column]

    lstIndicesOfAllMatrixPoints = list(dctOfMatrix.keys())
    setAllPossiblePaths = set()

    from itertools import permutations
    for pathCandidate in permutations(lstIndicesOfAllMatrixPoints):
        lstPossiblePath = []
        prevIndexTuple = pathCandidate[0]
        lstPossiblePath.append(prevIndexTuple)
        for currIndexTuple in pathCandidate[1:]:
            if abs(currIndexTuple[0]-prevIndexTuple[0]) + abs(currIndexTuple[1]-prevIndexTuple[1]) > 1:
                break # current path indices not allowed in path (no diagonals or jumps)
            else:
                if dctOfMatrix[currIndexTuple] >= dctOfMatrix[prevIndexTuple]:
                    break # only "down" is allowed for "skiing"
                else:
                    lstPossiblePath.append(currIndexTuple)
                    prevIndexTuple = currIndexTuple
        if len(lstPossiblePath) > 1 and tuple(lstPossiblePath) not in setAllPossiblePaths:
            setAllPossiblePaths.add(tuple(lstPossiblePath))

    return setAllPossiblePaths, dctOfMatrix

setAllPossiblePaths, dctOfMatrix = getAllValidSkiingPathsFrom(myMatrix)

printedPath = []
bestPath = []
for path in setAllPossiblePaths:
    for point in path:
        printedPath.append(dctOfMatrix[point])
    if len(printedPath) > len(bestPath): # Looking for the path with a maximum distance
        bestPath = printedPath
    if len(printedPath) == len(bestPath): # If we have more than one path with a maximum distance we look for the drop
        if (printedPath[0]-printedPath[-1]) > (bestPath[0]-bestPath[-1]):
            bestPath = printedPath
    printedPath = []

print("Path -->", bestPath)
print("Distance -->", len(bestPath))
print("Drop -->", bestPath[0]-bestPath[-1])

Sample input matrix:

4 5 2
1 1 6
8 7 5

Output:

Path --> [8, 7, 1]
Distance --> 3
Drop --> 7

Explanation:

The longest path would be 8-7-1 as it has the maximum distance = 3.

There are other paths with the same maximum distance which are 8-7-5 and 5-4-1. However, 8-7-1 has the maximum drop = 7 (8-1).

As I said in the comment, it's probably faster to get the local minima and check the paths that start from those points. You can then get the path with the maximum length and reverse the points if you want the descending path.

I haven't tested this a lot, but it takes less than a second on my pc with a 4x4 matrix.

Here's the code (again, I've only tested this with a few inputs and there's a bit of code duplication, take it as a general guideline):

def get_next_ascending_value(row, column, matrix, size):
    current_value = matrix[row][column]
    max_steepness = -1
    coords = []
    if (column > 0) and (matrix[row][column-1] > current_value):
        if max_steepness < matrix[row][column-1]:
            max_steepness = matrix[row][column-1]
            coords = [row, column-1]
    if (row > 0) and (matrix[row-1][column] > current_value):
        if max_steepness < matrix[row-1][column]:
            max_steepness = matrix[row-1][column]
            coords = [row-1, column]
    if (column < size - 1) and (matrix[row][column+1] > current_value):
        if max_steepness < matrix[row][column+1]:
            max_steepness = matrix[row][column+1]
            coords = [row, column+1]
    if (row < size - 1) and (matrix[row+1][column] > current_value):
        if max_steepness < matrix[row+1][column]:
            max_steepness = matrix[row+1][column]
            coords = [row+1, column]

    return coords

def is_local_minimum(row, column, matrix, size):
    current_value = matrix[row][column]
    if (column > 0) and (matrix[row][column-1] < current_value):
        return False
    if (row > 0) and (matrix[row-1][column] < current_value):
        return False
    if (column < size - 1) and (matrix[row][column+1] < current_value):
        return False
    if (row < size - 1) and (matrix[row+1][column] < current_value):
        return False

    return True

def get_local_minimum(matrix, size):
    for row in range(len(matrix)):
        for column in range(size):
            if is_local_minimum(row, column, matrix, size):
                yield [row, column]

# [4, 5, 6]
# [1, 1, 6]
# [4, 5, 6]

# starting_matrix = [[4, 5, 6],[1, 1, 6],[4, 5, 6]]
# size = 3

# [4, 5]
# [1, 1]

# starting_matrix = [[4, 5],[1, 1]]
# size = 2

# [4, 5, 7, 9]
# [1, 1, 2, 5]
# [4, 5, 7, 9]
# [1, 1, 2, 5]

starting_matrix = [[4, 5, 7, 9],[1, 1, 2, 5],[4, 5, 7, 9],[1, 1, 2, 5]]
size = 4

for coords in get_local_minimum(starting_matrix, size):
    print(coords, starting_matrix[coords[0]][coords[1]])
    next_coords = get_next_ascending_value(coords[0], coords[1], starting_matrix, size)
    while (len(next_coords) > 0):
        print(next_coords, starting_matrix[next_coords[0]][next_coords[1]])
        next_coords = get_next_ascending_value(next_coords[0], next_coords[1], starting_matrix, size)
    print('---')


Is there a better, more efficient way to write this loop? While working with someone else's code I've isolated a time-killer in the (very large) script. Basically this program runs through a list of thousands and thousands of objects to find matches to an input (or many inputs) object. It works well when the number is below 50 but any input higher than that and it begins to take a looooong time. Is this just an issue with the database I'm running it against (30,000 things)? Or is this code written inefficiently?

for i in range(BestX):
       Tin=model[BestFitsModel[0][i]][2]
       Tau2=np.append(Tau2,model[BestFitsModel[0][i]][6])
       Dir=model[BestFitsModel[0][i]][0]
       Dir=Dir.replace('inp','out',1) ##open up correct .out file
       T=open(Dir,"rb")
       hold=''
       Tau=[]
       for line in T:
        if not line.strip():
         continue
        else:
         hold=line.split()
         Tau.append(hold)
       Tauline=0
       Taunumber=0
       print 'Loop'
       for j in range(5,len(Tau)-50):
        if 'RESULTS:' in Tau[j-5][0]:
         for k in range(j,j+50):
          if (Tau[k][1] == model[BestFitsModel[0][i]][6]):
           Tauline=i #line number of the tau
           Taunumber=k-(j) #Tau 1, 2, 3, etc.

   Dir=Dir.replace('out','stb',1)#.stb file
   Dir2=np.append(Dir2,Dir)
   F=open(Dir, "rb")
   hold=''
   Spectrum=[]
   for line in F:
    hold=line.split()
    Spectrum.append(hold)

BestX is a list of 30,000 objects (stars).

I'm not looking for an answer to this problem, I'm looking for code-critique...there has to be a way to write this so that it performs faster, right?

Thanks.

EDIT: Reading through the Python Performance Tips I've noticed one little snag (so far). Starting with the line Tau=[] and continuing for the next six lines. I can shorten it up by

Tau=[line.split() for line in T]

I'm gonna keep looking. Hopefully I'm on the right track.

Some guesswork here but I think what you want is actually something like this:

def match_something(file, match):
    """ Finds a line in file starting with 'RESULTS:' then checks the following 50
    lines for match; if it fails returns None, otherwise it returns
    how many lines down from the 'RESULTS:' heading the match is found
    """
    in_results_section = False
    results_counter = 0
    for line in file:
        if line.strip():
            if results_header_found:
                if line.split()[1] == match:
                    return results_counter  # found a match
                else:
                    results_counter += 1
                    if results_counter > 50:
                        return None
# I'm assuming in the above that there can only be one results section in each file

            elif line.startswith('RESULTS:'):
                results_header_found = True

matched_results = []
for i in range(BestX):
    thing = model[BestFitsModel[0][i]]

    # make variables with descriptive names
    directory, input_thing, match = thing[0], thing[2], thing[6]
    list_of_matches = np.append(list_of_matches, match)
    with open(directory.replace('inp', 'out', 1), 'rb') as file:
        m = match_something(file, match)
        if m is not None:
            matched_results.apppend((i, m))
    stb_directory = directory.replace('inp', 'stb', 1), 'rb')
    np.append(directory_list, stb_directory)
    with open(stb_directory, 'rb') as file:
        spectrum = [line.split() for line in file]

This uses a function match_something to go through the file. As soon as a result is found, the function exits (return i, results_counter) and then the main loop adds this result to a list of matched results. If no result is found within 50 lines of the line starting 'RESULTS:' then the function returns None, and the loop moves to the next line of BestFitsModel[0]. (If, on the other hand, there can be more than one results section within each file, then you need to alter this accordingly; it should carry on checking the rest of the file rather than returning, which will be slower.) I hope it should be fairly clear what is happening in this code; it should avoid looping through anything unnecessarily (assuming I've guessed correctly what it's supposed to do)

In the original code, it seems that the files are not closed (T.close()), which could cause problems later on in the programme. Using the with notation avoids the need for this in the above code.


Count words in a list of titles, with some cleanup I have a list of article titles, where I wish to count the number of occurrences for each word (and remove some words and characters). The input is in a .csv file where the titles are in column 'Titles'.

Maybe someone could help me do it more elegantly.

import numpy as np
import pandas as pd
#imports Counter, as we will need it later:
from collections import Counter

df = pd.read_csv("Article_titles.csv")
print (df.head(10))

#Selecting the titles into variable
titles = []
titles = df.Title

remove_words_list = ["at","of","a","and","in","for","the","to","with","on","using","an","after","from","by","use","review","upper","new","system"]
remove_characters_list = ".:,-%()[]?'"

huge_title_list = []
#create a list of all article titles:
for i in range(len(titles)):
  clean_title = titles[i].lower().translate({ord(i): None for i in remove_characters_list})
  huge_title_list.append(clean_title)

total_words_string = " ".join(huge_title_list)
#join all article titles into one huge string

querywords = total_words_string.split()
#split the string into a series of words

resultwords = [word for word in querywords if word not in remove_words_list]
#From stackoverflow

resultwords_as_list = list( Counter(resultwords).items())

#Convert resultwords_list to dataframe, then convert count to numbers and finally sorting.
resultframe = pd.DataFrame(np.array(resultwords_as_list).reshape(-1,2), columns = ("Keyword","Count"))
resultframe.Count = pd.to_numeric(resultframe.Count)
sortedframe = resultframe.sort_values(by='Count',ascending=False).reset_index(drop=True)
print(sortedframe[0:50])

example of Input:

Titles | other_field | other_field2

"Current status of prognostic factors in patients with metastatic renal cell carcinoma." |"asdf"|12
"Sentinel lymph node biopsy in clinically node-negative Merkel cell carcinoma: the Westmead Hospital experience." |"asdf"|15

desired output:

Word | Count

carcinoma | 2

cell | 2

biopsy | 1

clinically | 1

....

...

Very good first post! There are a few small things to call out. First, refrain from using pandas and numpy unless it's absolutely necessary. Both are fantastic tools which excel in certain areas, but generally speaking many stock Python scripts can be just as performant, much more portable, and a lot more clear on their own. For example, stock Python is fantastically suited for counting the frequency of hashable items (in fact, you used it with Counter). But if I wanted to do QR decomposition, I would absolutely reach for numpy as it already has an implementation of this and that implementation will likely be much faster than anything I can write. Always be sure to use the right tool for the job. Generally speaking, the more dependencies a piece of code has, the harder it is to compose with other things (harder could be quantifying programmer effort here--as in, the effort to install dependencies/debug version issues/etc.). So for this reason, I'm going to remove numpy and pandas from my answer.

#imports Counter, as we will need it later:
from collections import Counter

This is a useless comment. Of course that's what the following line does. And of course you're importing something because you need it. Don't just comment for the sake of commenting. Comment to explain the "why" not the "what."

You can do your csv reading with stock python. You should be using with contexts here to handle properly closing files (especially if exceptions are raised). csv.DictReader will be useful here as it is an iterator that yields dicts for each row. This allows us to use a generator comprehension to build the list of titles. If you are unfamiliar with generators, you should [read up on them}(https://stackoverflow.com/q/1756096/568785). The benefit they give you is that they don't build up a big list in memory. They only produce values upon request. In your code you have a lot of list and string manipulation (see the list comprehension of resultwords, the join/split dance of total_words_string and querywords--which is unnecessary--,and huge_title_list). All of these points can be bottlenecks, because they require building up in memory the entire state to that point. Instead, using a generator allows you to lazily defer the work until you need it (in this case, when you use Counter).

A good analogy for this is an assembly line. Imagine you had a factory building computers with 3 stops on the assembly line (we'll call them A, B, and C). At each stop, a worker adds one component to the computer (let's say motherboard, CPU, RAM). If we were to write this out using lists it would look like this:

computers_after_A = [add_motherboard(computer) for computer in bare_computers]
computers_after_B = [add_CPU(computer) for computer in computer_after_A]
computers_after_C = [add_RAM(computer) for computer in computer_after_B]

This looks innocent enough, but if you were to run your factory like this you'd have some real problems. Let's say the factory processes 1000 computers per day. The above code would be equivalent to the following:

The worker at A adds motherboards to each incoming computer and then adds the finished computers to a big stack.
Once worker A is done with all 1000 computers, the stack is pushed to worker B who takes one from each stack, adds the CPU and then makes a new stack of finished computers.
Once worker B is done with all 1000 computers, the stack of 1000 computers is pushed to worker C, who adds the RAM and then adds the computers to the finally done stack.

The problem with the above is that it is inefficient (worker B has no work until worker A completes all 1000 computers and worker C has no work until both workers A and B finish all 1000 computers) and requires you to have the room to store all 1000 computers at each location A, B, and C.

A much better approach is how factories really work. A constantly moving conveyor belt which moves the computers through one by one. At each location, the worker adds their respective component and then send each computer along individually to the next station. This would be equivalent to the following:

computers_after_A = (add_motherboard(computer) for computer in bare_computers)
computers_after_B = (add_CPU(computer) for computer in computer_after_A)
computers_after_C = (add_RAM(computer) for computer in computer_after_B)

With this approach we've replaced lists with generators. Now, computers are immediately available (instead of only available after everything finishes) and we don't need the space to build up 3 piles of 1000 computers.

Hopefully, this motivates why we should use generators. I'll be using them below.

There's no reason to use pandas to sort the counts from Counter. Counter has a most_common() method that will do this for you.

You had the right idea with using translate to replace unwanted characters. But you don't need to build the dictionary every time. Instead using string.maketrans and saving this "translation dictionary" will avoid doing lots of extra work.

remove_words_list should probably be called stop_list as this is the common parlance for words you want to exclude. Also, you should make this a frozenset so that word in stop_list is O(1) instead of an O(n) scan (what you currently do, which is really inefficient).

Typically, we include code that we want to run in a main() function and run it only if __name__ == '__main__'. This allows other code to include this file (and potentially use an utility functions it defines) without having the main() run.

With all that in mind, I'd refactor your code to something like this:

import csv
import string

STOP_WORDS = frozenset(('at', 'of', 'a', 'and', 'in')) # ...
EXCLUDE_CHARS = string.maketrans('', '', '.:,-%()[]?\'')

def main():
    with open('Article_titles.csv') as f: # consider accepting the filename as an argument (sys.argv)
        titles = (row['title'] for row in csv.DictReader(f, delimter='|'))
        words = (word.lower().translate(EXCLDUE_CHARS)
                 for title in titles for word in title.split())
        interesting_words = (word for word in words if word not in STOP_WORDS)
        frequencies = Counter(interesting_words)

        print(frequencies.most_common())

if __name__ == '__main__':
    main()

Now remember how I mentioned above choosing the right tool for the job? It turns out that what you're trying to do (word extraction) is pretty tricky. Natural language has all sorts of irregularities, odd formatting, and inconsistencies. Luckily, there's a fantastic Python library which has a fairly good (complicated) implementation for handling all this for you: nltk. This is a very appropriate use of a library, because this task is hard and nltk has put lots of work into building a robust implementation. The home page has an example of what you need:

>>> from nltk import word_tokenize
>>> word_tokenize('When this thing hits 88 miles per hour')
['When', 'this', 'thing', 'hits', '88', 'miles', 'per', 'hour']

I'll leave integrating nltk as an exercise for you, but it should only involve changing one line in my above code.


"Recycled numbers" challenge I wrote this program for the Google Code Jam problem, "Recycled numbers."

It took around 2 and half minutes to solve the small input set on my dual core, 1GB RAM system. But for the large input set it took around 25 minutes(processor usage was 100% for those 25 mins) to solve the problem.

Problem

  Let's say a pair of distinct positive integers (\$n\$, \$m\$) is recycled if
  you can obtain m by moving some digits from the back of n to the front
  without changing their order.

  For example, (12345, 34512) is a recycled pair since you can obtain
  34512 by moving 345 from the end of 12345 to the front. Note that n
  and m must have the same number of digits in order to be a recycled
  pair. Neither n nor m can have leading zeros.

  Given integers \$A\$ and \$B\$ with the same number of digits and no leading
  zeros, how many distinct recycled pairs (\$n\$, \$m\$) are there with \$A ≤ n <
> m ≤ B\$?

How can I make my program fast enough to solve the large input set within 8 minutes?

f=open('C-large.in')
noi=int(f.readline().rstrip('\r\n'))
i=1
ou=open('jam.out','w')
while i<=noi:
  re=f.readline().rstrip('\r\n')
  rs=re.split()
  a=int(rs[0])
  b=int(rs[1])
  ans=0
  c=[x for x in range(a,b+1)]
  for ind,x in enumerate(c):
      for y in c[ind+1:]:
          n=str(x)
          m=str(y)
          if len(n)==len(m) and len(n)>1 and len(m)>1:
             ki=1
             while ki <len(n):
                 temp=n[-1:-(ki)-1:-1][::-1]+n[:-(ki)]
                 if temp[0]!='0':
                  if temp==m:
                     ans+=1
                     break
                 ki+=1
          else :
              break

  print("Case #%s:"%(i),ans,file=ou)
  i+=1
ou.close()
f.close()

Small input:

50
156 927
167 978
484 954
160 990
135 916
160 939
103 914
173 958
100 101
124 945
181 976
101 932
101 935
170 928
171 984
126 973
283 788
667 667
384 750
153 934
173 993
185 930
100 100
178 993
21 31
178 970
168 958
170 992
104 989
34 70
162 935
289 289
185 993
10 99
120 953
184 917
179 936
108 967
6 7
160 958
115 925
137 972
188 921
10 99
118 966
100 972
167 944
136 983
107 926
118 996

Large input:

50
10000 10001
1781 7688
1007465 1919027
100000 100000
1032122 1995680
1216257 1504628
1216908 1951709
1021619 1930030
15048 53617
1007458 1962726
388 869
1067361 1995135
46739 73395
1077614 1958584
1031957 1958036
72634 74120
1063961 1954135
274937 720742
1 8
1057393 1928164
28 93
1000000 2000000
1032073 1940559
1089798 1970754
100000 999999
1434814 1780702
1072799 1964898
1059864 1967119
155569 345355
2578 4229
72 95
552 553
601415 711929
1275115 1433162
688042 856983
594203 673506
1172008 1490273
180289 925799
354653 354653
1195595 1669305
5908 5967
2083 7987
339 670
1015539 1982001
29229 96206
1006167 1522491
1117646 1117646
1021936 1950412
41215 83080
1033077 1969448

f=open('C-large.in')

Don't sue single letter variable names, its really hard to read

    noi=int(f.readline().rstrip('\r\n'))

Since the default is already to remove whitespace, why both with the \r\n parameter?

    i=1
    ou=open('jam.out','w')
    while i<=noi:

Use for loops, not while loops to count

      re=f.readline().rstrip('\r\n')

re is a really bad name because its the same as the regular expression module

      rs=re.split()
      a=int(rs[0])
      b=int(rs[1])

I'd use a,b = map(int, re.split())

      ans=0
      c=[x for x in range(a,b+1)]

Use c = list(range(a,b+1))

      for ind,x in enumerate(c):
          for y in c[ind+1:]:

That's gonna be inefficient. Firstly, you can easily express it using simple ranges

for x in range(a, b+ 1):
    for y in range(x + 1, b + 1):

and therefore avoid the slices.

But you can do much better. You are only interested in values of y which are recyclings of x. Instead of generating the rather large number of possible second numbers, try just generating the fairly small number of possible ways to recycle x.

              n=str(x)
              m=str(y)

Converting all your numbers to strings will be expensive. See if you can keep them as integers and just do math operations on them

              if len(n)==len(m) and len(n)>1 and len(m)>1:
                 ki=1
                 while ki <len(n):

Again, use for loops to count

                     temp=n[-1:-(ki)-1:-1][::-1]+n[:-(ki)]

temp should be banned variable name. All variables are temporary, use a more helpful name.

                     if temp[0]!='0':
                      if temp==m:
                         ans+=1
                         break
                     ki+=1
              else :
                  break

      print("Case #%s:"%(i),ans,file=ou)

Normally, we use %d for numbers

      i+=1
    ou.close()
    f.close()


Coding Practice - Float* -> Vector3* cast If you were given the following:

#include <cassert>

struct vec {
    float x,y,z;
    vec(float x, float y, float z) : x(x), y(y), z(z) {}
};

int main() {
    float verts[] = {1,2,3,4,5,6,7,8,9}; // length guaranteed to be a multiple of 3
    assert(sizeof(vec) == sizeof(float) * 3);

    // Option 1
    for (int i = 0; i < 3; ++i) {
        auto a = reinterpret_cast<vec*>(verts)[i];
        // ...
    }

    // Option 2
    for (int i = 0; i < 9; i += 3) {
        vec a(verts[i], verts[i+1], verts[i+2]); // -O3 averts the copy, so d/w
        // ...
    }

    return 0;
}

Which is considered better practice?

The advantage to the latter option is of course far more explicit, and carries all the advantages/disadvantages that contains; but may not be portable depending on the assertions you can make.

I would go with a 3rd option that is closest to option 2. But instead of taking the three parameters separately in the constructor, take an array of floats.

struct vec {
    float x,y,z;
    vec(float fv[]) : x(fv[0]), y(fv[1]), z(fv[2]) {}
};

int main() {
    float verts[] = {1,2,3,4,5,6,7,8,9};

    for (int i = 0; i < 9; i += 3) {
        const vec a(&verts[i]);
        // ...
    }

    return 0;
}


Print how many times a sequence of characters appears in a string Given a string, count how many times a sequence of characters (in this code, the sequence is "ei") appears until the user types '#', then print the amount. I would like to know how to improve the code.

#include <stdio.h>
#include <inttypes.h>

int main(void)
{
    char ch, cur_ch, last_ch;
    cur_ch = '\0';
    uint16_t count = 0;

    puts("Enter a string (# to finish):");
    while ((ch = getchar()) != '#')
    {
        last_ch = cur_ch;
        cur_ch = ch;

        if (last_ch == 'e' && cur_ch == 'i')
            count++;
    }

    printf("There %s %" PRIu16 " %s.\n", count == 1 ? "is" : "are", count,
    count == 1 ? "ei" : "ei's");

    return 0;
}

Missed detection of EOF: getchar() returns an int with values in the range of unsigned char and the negative value of EOF.  Saving that result in a char loses distinctiveness should getchar() return EOF due to stdin becoming closed or input error.

//char ch;
//while ((ch = getchar()) != '#')
int ch;
while ((ch = getchar()) != '#' && ch != EOF)

Why uint16_t?  The input stream can be excessively long, and certainly more than 65,535.  Suggest a wider type.  (Good that code uses an unsigned integer.)

// uint16_t count = 0;
unsigned long long  = 0;

Code could do this with 2 variables rather than 3.

int ch;
previous = EOF;

while ((ch = getchar()) != '#' && ch != EOF) {
  if (previous == `e` && ch == `i') {
    count++;
  }
  previous = ch;
}

Recommend {}

// if (last_ch == 'e' && cur_ch == 'i')
//     count++;
if (last_ch == 'e' && cur_ch == 'i') {
    count++;
}

Grammar: Code goes though work to print are vs. is yet does not make correct with ei's vs. ei.


Photo rating function I want to create a photo rating function. If I click thumbs up then votes increase, and if I click thumbs down then votes decrease.

function pictures(state = [], action) {
        switch(action.type) {
            case ADD_PICTURE:
                return [{
                    id: action.id,
                    picture: action.picture,
                    votes: 0
                    }
            , ...state];
            case LIKE_UP_PICTURE:
                return state.map(picture => {
                    if(picture.id === action.id) {
                    return {...picture, votes: picture.votes + 1}
                    }
                return picture;
                });
            case LIKE_DOWN_PICTURE:
                return state.map(picture => {
                    if(picture.id === action.id) {
                    return {...picture, votes: picture.votes - 1}
                    }
                return picture;
                });
            default:
                return state;
        }
    }

There's a few things you can do and different approaches depending on your personal preference. You could:

Extract the cases to pure functions

Extracting the logic to methods could help with readability and ease testing.

const addPicture = (state, action) => [
  {
    id: action.id,
    picture: action.picture,
    votes: 0,
  },
  ...state,
]

const likeUpPicture = (state, action) =>
  state.map(picture => {
    if (picture.id === action.id) {
      return {
        ...picture,
        votes: picture.votes + 1,
      }
    }

    return picture
  })

const likeDownPicture = (state, action) =>
  state.map(picture => {
    if (picture.id === action.id) {
      return {
        ...picture,
        votes: picture.votes - 1,
      }
    }

    return picture
  })

const pictures = (state = [], action) => {
  switch (action.type) {
    case ADD_PICTURE:
      return addPicture(state, action);
    case LIKE_UP_PICTURE:
      return likeUpPicture(state, action);
    case LIKE_DOWN_PICTURE:
      return likeDownPicture(state, action);
  }
}

Object destructuring/Object shorthand

Destructuring parameters could help signal that from a given action we are only concerned with certain properties. Shorthand notation can help keep the functions concise and reduce duplication.

const addPicture = (state, { id, picture }) => [
  {
    id,
    picture,
    votes: 0,
  },
  ...state,
]

const likeUpPicture = (state, { id }) =>
  state.map(picture => {
    if (picture.id === id) {
      return {
        ...picture,
        votes: picture.votes + 1,
      }
    }

    return picture
  })

const likeDownPicture = (state, { id }) =>
  state.map(picture => {
    if (picture.id === id) {
      return {
        ...picture,
        votes: picture.votes - 1,
      }
    }

    return picture
  })

Extract common logic

Since updating a certain element given an id is a common operation extracting the logic to a method might help reduce duplication and increase readability.

const updatePicture = (pictures, predicate, callback) =>
  pictures.map(picture => predicate(picture) ? callback(picture) : picture)

const likeUpPicture = (state, action) =>
  updatePicture(state, picture => picture.id === action.id, picture => ({
    ...picture,
    votes: picture.votes + 1,
  }))

const likeDownPicture = (state, action) =>
  updatePicture(pictures, picture => picture.id === action.id, picture => ({
    ...picture,
    votes: picture.votes - 1,
  }))

Define your reducer another way

For example, using createReducer can result in a compact reducer definition

const initialState = []

const pictures = createReducer(initialState, {
  [ADD_PICTURE]: addPicture,
  [LIKE_UP_PICTURE]: likeUpPicture,
  [LIKE_DOWN_PICTURE]: likeDownPicture,
})

Normalize State Shape

I've defined the methods inline for brevity but you could extract them as above. The official Redux link above gives a good explanation on why you might want to consider normalizing your state shape.

const byId = createReducer({}, {
  [ADD_PICTURE]: (state, { id, picture }) => ({
    ...state,
    [id]: {
      ...picture,
      id,
      votes: 0,
    }
  }),

  [LIKE_UP_PICTURE]: (state, { id }) => ({
    ...state,
    [id]: {
      ...state[id],
      votes: state[id].votes + 1,
    },
  }),

  [LIKE_DOWN_PICTURE]: (state, { id }) => ({
    ...state,
    [id]: {
      ...state[id],
      votes: state[id].votes - 1,
    }
  })
})

const allIds = createReducer([], {
  [ADD_PICTURE]: (state, { id }) => [
    ...state,
    id,
  ],
})

const pictures = combineReducers({
  allIds,
  byIds,
})


Alternative to integer_sequence trick I hate working with std::integer_sequence. Each time I want to write a function foo I have to write an another function foo_impl where the actual stuff is happening. Therefore I'm polluting the space with additional function and the code is kind of scattered around. I came up with an alternative and I would love to know your opinion.

Let's define integral_sequence<N>. Here we use the damned std::integer_sequence for the last time!

template <std::size_t... I>
constexpr auto integral_sequence_impl(std::index_sequence<I...>) {
  return std::make_tuple(std::integral_constant<std::size_t, I>{}...);
}

template <int N>
auto integral_sequence = integral_sequence_impl(std::make_index_sequence<N>{});

To demonstrate its use, lets redo the example code for std::integral_sequence:

// Convert array into a tuple
template <typename T, std::size_t N>
decltype(auto) a2t(const std::array<T, N> &a) {
  auto impl = [&](auto... I) { return std::make_tuple(a[I]...); };

  return std::apply(impl, integral_sequence<N>);
}

// pretty-print a tuple
template <class Ch, class Tr, class... Args>
decltype(auto) operator<<(std::basic_ostream<Ch, Tr> &os,
                          const std::tuple<Args...> & t) {

  auto impl = [&](auto... I) {
    ((os << (I == 0 ? "" : ", ") << std::get<I>(t)), ...);
  };

  os << "(";
  std::apply(impl, integral_sequence<sizeof...(Args)>);
  return os << ")";
}

int main() {
  std::array<int, 4> array = {1, 2, 3, 4};

  // convert an array into a tuple
  auto tuple = a2t(array);
  static_assert(
      std::is_same<decltype(tuple), std::tuple<int, int, int, int>>::value, "");

  // print it to cout
  std::cout << tuple << '\n';
}

The trick with integral_sequence is kind of the same as with integer_sequence. However, because it can be used with generic lambda you do not have to define an extra function, all code can be neatly together and it reduces visual clutter.

So what do you think? Are there any disadvantages?

The only problems I can see now is that it is only c++17 and integral_sequence<400> takes really long time to compile.

Looks good to me! I mean, I'm not likely to convert to it because of its heavyweightness — you noted its use of std::apply, but it also pulls in all of <tuple> every time you use it, too. Also, it's arguably coincidental that your expressions a[I] and std::get<I>(t) work in this context, since what's actually happening in both cases is an implicit conversion from std::integral_constant<size_t, K> to size_t. In some contexts, the user might need to explicitly cast int(I) or whatever.

You could remove the std::apply dependency and simplify the user's job at the same time by writing a helper function apply_to_sequence<N>(lambda), as in:

// Convert array into a tuple
template <typename T, std::size_t N>
auto a2t(const std::array<T, N>& a) {
  return apply_to_sequence<N>([&](auto... Is) {
      return std::make_tuple(a[Is]...);
  });
}

(Oh yeah, I think your use of decltype(auto) in this function was bogus and could result in dangling references. Certainly auto would be fine, so I recommend it.)

// pretty-print a tuple
template <class Ch, class Tr, class... Args>
auto& operator<<(std::basic_ostream<Ch, Tr>& os,
                          const std::tuple<Args...>& t)
{
  os << "(";
  apply_to_sequence<sizeof...(Args)>([&](auto... Is) {
    ((os << (Is == 0 ? "" : ", ") << std::get<Is>(t)), ...);
  });
  return os << ")";
}

Writing apply_to_sequence from scratch is left as an exercise for the reader, but the trivial C++17 implementation is just this:

template<size_t N, class F>
decltype(auto) apply_to_sequence(const F& fn) {
    return std::apply(fn, integral_sequence<N>);
}

Having hidden the implementation details apply and integral_sequence from the reader, we are now free — if we so desire — to replace them with things that are

faster
C++14-compatible
<tuple>-free

One stylistic note: I prefer to name my parameter packs with plurals, so, singular auto I but plural auto... Is. That's the logic behind my renaming above.


HashMap implementation in Python Here's my implementation of a bare-bones HashMap in Python.
Expected behavior, it returns None if a key is missing,
When a key is inserted the second time it just overwrites the first value.

class HashMap:
    def __init__(self):
        self.store = [None for _ in range(16)]
        self.size = 0

    def get(self, key):
        key_hash = self._hash(key)
        index = self._position(key_hash)
        if not self.store[index]:
            return None
        else:
            list_at_index = self.store[index]
            for i in list_at_index:
                if i.key == key:
                    return i.value
            return None

    def put(self, key, value):
        p = Node(key, value)
        key_hash = self._hash(key)
        index = self._position(key_hash)
        if not self.store[index]:
            self.store[index] = [p]
            self.size += 1
        else:
            list_at_index = self.store[index]
            if p not in list_at_index:
                list_at_index.append(p)
                self.size += 1
            else:
                for i in list_at_index:
                    if i == p:
                        i.value = value
                        break

    def __len__(self):
        return self.size

    def _hash(self, key):
        if isinstance(key, int):
            return key
        result = 5381
        for char in key:
            result = 33 * result + ord(char)
        return result

    def _position(self, key_hash):
        return key_hash % 15

class Node:
    def __init__(self, key, value):
        self.key = key
        self.value = value

    def __eq__(self, other):
        return self.key == other.key

if __name__ == '__main__':
    hashmap = HashMap()
    hashmap.put(2, 12)
    hashmap.put('asd', 13)
    hashmap.put(2, 11)
    print(hashmap.get(2))
    print(hashmap.get('asd'))
    print(hashmap.get('ade'))

Invite comments and reviews.

Initialization of store as a list of None does not really buy you anything in terms of efficiency, but complicates the code quite a bit. I recommend to initialize it with empty lists instead.
put iterates the list twice. A single pass is sufficient:

    for i in list_at_index:
        if i == p:
            i.value = p.value
            return
    list_at_index.append(p)
    self.size += 1

store has 16 slots, but key_hash % 15 returns only 15 possible values. The last slot is never used.
Absence of delete is somewhat questionable.


User Logged onto Windows I created a Windows Service that Tracks which App Server a user is actually connected to through a load balancer.

It retrieves the user that is logged into windows on that machine, then cross references the Active Directory (AD) account to return the first name, last name and E-mail (for notification through the website created for this project)

There is one system on the Network where users only remote into the system, the Vista System, for some reason it is returning with Values that say "LOCAL SERVICE" is Logged into that machine, it's weird and the Security Guys say that it is impossible for the "LOCAL SERVICE" account to be logged into Windows.

Here is how I get the user that is logged into Windows

ManagementScope ms = new ManagementScope("\\\\.\\root\\cimv2");
ObjectQuery query = new ObjectQuery("select * from win32_computersystem");
ManagementObjectSearcher searcher = new ManagementObjectSearcher(ms, query);

foreach (ManagementObject mo in searcher.Get())
{
    _username = mo["username"].ToString();
}
// remove the domain part from the username
string[] usernameParts = _username.Split('\\');
_username = usernameParts[usernameParts.Length - 1];

I only post this because, it isn't broken, it does exactly what it is supposed to.

is this an Edge Case that I can fix, or is the user actually "logged in" to this machine

Is there something that I can do to make this code more efficient?

I have cross posted this on SuperUser Migrated cross post to StackOverflow

As per my second comment, below is a possible solution. This solution involves a different querying method that will work for both console sessions and remote sessions. This method looks at all processes that are the explore.exe process, which is the user shell for each user that is logged into the server.  Once armed with the list of those processes, getting the owner for it is another WMI query (GetProcessOwner method).

// what to do if multiple users logged in?
foreach (var p in Process.GetProcessesByName("explorer"))
{
    _username = GetProcessOwner(p.Id);
}

// remove the domain part from the username
var usernameParts = _username.Split('\\');

_username = usernameParts[usernameParts.Length - 1];

...

    public static string GetProcessOwner(int processId)
    {
        var query = "Select * From Win32_Process Where ProcessID = " + processId;
        ManagementObjectCollection processList;

        using (var searcher = new ManagementObjectSearcher(query))
        {
            processList = searcher.Get();
        }

        foreach (var mo in processList.OfType<ManagementObject>())
        {
            object[] argList = { string.Empty, string.Empty };
            var returnVal = Convert.ToInt32(mo.InvokeMethod("GetOwner", argList));

            if (returnVal == 0)
            {
                // return DOMAIN\user
                return argList[1] + "\\" + argList[0];
            }
        }

        return "NO OWNER";
    }


C++ Data-Oriented Modules using namespaces and extern I have been coding in a certain style for some time now, where I try to avoid classes and stick to pure data structures whenever possible. Basically every module of my program has its own namespace in which all the data structures and functions are declared.

I started doing this because I wanted to focus on the actual data that gets processed and wanted to abstract data as little as possible. Performance is very important to me and I would like to develop a minimal, cache- and compiler-friendly style and I need someone to help me understand what I actually do by structuring my code like this.

So this is how I would structure the functionality of loading 3D meshes into my program for example. The actual code is very abbreviated because right now I would like to only focus on the structure with the namespace and the external declarations.

The header Mesh.h

#include "OpenGL.h" //a higher level namespace containing context information
namespace OpenGL{
    namespace Mesh{

struct Vertex{
    vec3 pos;
    vec3 normal;
    vec2 uv;
};

struct Material{
    vec3 amb;
    vec3 diff;
    vec3 spec;
    float shininess;
};
struct Mesh{
    unsigned int indexOffset;
    unsigned int indexCount;
    unsigned int materialIndex;
};
extern vector<Vertex> allVertices;
extern vector<unsigned int> allIndices;
extern vector<Material> allMaterials;
extern vector<Mesh> allMeshes;

extern string MESH_DIR;
extern vector<string> meshNames; //find mesh index by name

void setMeshDirectory(string pDirectory);

void loadAllMeshes();
Mesh loadMesh(string pFilename);
void addMesh(string pFileName);
    }
}

The code file

#include "Mesh.h"
#define DEFAULT_MESH_DIR "..//assets//meshes//"
//define all external variables
vector<Vertex> allVertices = {};
vector<unsigned int> allIndices = {};
vector<Material> allMaterials = {};
vector<Mesh> allMeshes = {};

string MESH_DIR = DEFAULT_MESH_DIR;
vector<string> meshNames = {};

void OpenGL::Mesh::setMeshDirectory(string pDirectory){
    MESH_DIR = pDirectory;
}

void OpenGL::Mesh::addMesh(string pFileName){
    auto it = find(meshNames.begin(), meshNames.end(), pFileName);
    if(it == meshNames.end()){
        meshNames.push_back(pFileName);
    }else{
    //error
    }
}

void OpenGL::Mesh::loadAllMeshes(){
    allMeshes.resize(meshNames.size()); //resize allMeshes array in advance
    for(unsigned int m = 0; 0 < meshNames.size(); ++m){
        allMeshes[m] = loadMesh(meshNames[m]);
    }
}

OpenGL::Mesh::Mesh OpenGL::Mesh::loadMesh(string pFilename){
    Mesh mesh;
    mesh.indexOffset = allIndices.size();
    unsigned int indexCount;
    unsigned int materialIndex;

    //use assimp to actually load the mesh from an .obj, .3DS, .fbx (etc) file
    //fill allMaterials, allVertices and allIndices with the data from this mesh

    mesh.indexCount = indexCount;
    mesh.materialIndex = materialIndex;
    return mesh;
}

Your header file is missing include guards. Either put #pragma once at the start, or use the more portable:

#ifndef MESH_H_
#define MESH_H_

// ... rest of header

#endif

MESH_H_ has to be unique for the file.

namespace OpenGL{

If you don't own OpenGL, I'd recommend not naming your namespace after it. If they release OpenGL in a namespace, your code wouldn't be compatible. Try to use a namespace for your project. Similarly, I'd recommend not naming your header OpenGL.h, unless you put the header in a directory.

extern vector<Vertex> allVertices;
extern vector<unsigned int> allIndices;
extern vector<Material> allMaterials;
extern vector<Mesh> allMeshes;

extern string MESH_DIR;
extern vector<string> meshNames; //find mesh index by name

I'm assuming that in OpenGL.h, you have using namespace std;. Don't do this. It is especially bad in a header.

Also, if you find yourself using extern globals, you are almost certainly doing it wrong. These should either not exist at all (as in the calling code takes care of it), or they should be wrapped in a class that the calling code passes around.

These globals and the following point out that you really need to define a class. Maybe something like this:

class MeshManager { // this name is suspicious; "Manager" in a name is usually bad.
    std::vector<Vertex> vertices;
    std::vector<unsigned int> indices;
    std::vector<Material> materials;
    std::vector<Mesh> meshes;

    std::string meshDirectory;
    std::vector<std::string> meshNames;

public:
    void setMeshDirectory(std::string directory);
    void loadAllMeshes();

    Mesh loadMesh(std::string filename);
    void addMesh(std::string filename);
};

Note that this design isn't the best. You still need a constructor, and possibly accessors on the member variables. Also, setMeshDirectory and loadAllMeshes might be better as a constructor to MeshManager, or a static factory function if you prefer that style.