DP solution to min triangle path I am studying for interviews for various companies. I wrote a solution to this problem, but if an experience programmer looks at it I am sure there are certain areas the code can be made in to a more beautiful code.

  Given a triangle, find the minimum path sum from top to bottom. Each
  step you may move to adjacent numbers on the row below.

  For example, given the following triangle

[
     [2],
    [3,4],
   [6,5,7],
  [4,1,8,3]
]

  The minimum path sum from top to bottom is 11 (i.e., 2 + 3 + 5 + 1 = 11).

My DP solution:

public class Solution {
public static int minimumTotal(List<List<Integer>> triangle) {
        if(triangle==null){
            return -1;
        }
        int [][] dp=new int[triangle.size()][triangle.size()];
        for(int i=0;i<triangle.size();i++){
            Arrays.fill(dp[i],0);

        }
        getMin(triangle,0,0,dp);
        return dp[0][0];
    }

    public static int getMin(List<List<Integer>> tri, int level, int index, int [][]dp){
        if((level+1)==tri.size()){
            dp[level][index]=tri.get(level).get(index);
            return dp[level][index];
        }else{
            int left=-1;
            int right=-1;
            if(dp[level+1][index]!=0){
                left=dp[level+1][index];
            }else{
                left=getMin(tri, level+1, index,dp);
            }
            if(dp[level+1][index+1]!=0){
                right=dp[level+1][index+1];
            }else{
                right=getMin(tri, level+1, index+1,dp);
            }
            dp[level][index]=tri.get(level).get(index)+Math.min(left,right);

            return dp[level][index];
        }
    }
}

Code Style

Although not the most important aspect of programming, I'd recommend using a bit more spaces in your code.

Just a few random example lines:

dp[level][index]=tri.get(level).get(index);
if(dp[level+1][index+1]!=0){
    right=dp[level+1][index+1];
dp[level][index]=tri.get(level).get(index)+Math.min(left,right);

(Note that these four lines do not belong together in the original code)

These would look better as:

dp[level][index] = tri.get(level).get(index);
if (dp[level + 1][index + 1] != 0) {
    right = dp[level + 1][index + 1];
dp[level][index] = tri.get(level).get(index) + Math.min(left, right);

This does improve readability quite a bit, and is the Java conventions.

Exceptions for exceptional cases

if(triangle==null){
    return -1;
}

Instead of returning the special value -1 here, which could be a valid output if your input contained some negative numbers, it is better to throw an exception.

if (triangle == null) {
    throw new NullPointerException("triangle cannot be null");
}

Or, using a Java method call as of Java 7:

Objects.requireNonNull(triangle, "triangle cannot be null");

Last index

When checking if an index is the last index in a list:

if((level+1)==tri.size()){

Then it is more common to compare the actual index, rather than the size:

if (level == tri.size() - 1) {

And as you see, you can remove one set of parameters as well.

Initialization

As all int-arrays are initialized to zero by default, this code does absolutely nothing:

for (int i = 0; i < triangle.size(); i++) {
    Arrays.fill(dp[i], 0);
}

Working backwards

Sometimes when faced with a challenge, it can be a good idea to think: What if I work backwards? Sometimes working from the other direction can be a big advantage. A different approach to this problem would be to work backwards.

     [2],
    [3,4],
   [6,5,7],
  [4,1,8,3]

Start at the bottom row. On this row, we can find that the 8 is irrelevant as it is right in the middle of two much smaller numbers that can be reached instead, from the same parents that can reach the 8. The same way, we can see that the 4 is a much worse choice than the 1. So the information we want from that row is [*, 1, *, 3].

Now on to the next row! [6, 5, 7]. We find out that the 6 is totally irrelevant as the 5 can also be reached from the same parent (the 3 above). Additionally, the only child of 5 is 1, but the child of 7 is 3. As 1 + 5 < 3 + 7, 1 + 5 does seem like a better route so far. But to be safe, I would store the sum for each of the current 'nodes'. So for the 5, store 1 + 5 = 6. For the 7, store 3 + 7 = 10.

On to the next row! [3, 4]. Here we can find out that the 3 is much better, as the only way the 4 is relevant is because 3 + 7 = 10. But as 1 + 5 + 3 = 9 is smaller than 3 + 7 + 4 = 14, and the 3 and 4 are neighbors, we can totally forget about the 3 + 7 + 4 = 14 path, as it is now irrelevant thanks to our superior 1 + 5 + 3 = 9 path.

Last row, [2]. Here we don't have any choice, so we just add the 2 to our current 1 + 7 + 3 = 9 path and we end up with 11.

Edit: after some debugging of your code, your approach is actually quite fast and very similar to what I started doing here. We just don't use the same order when iterating over the triangle. But if you want to see what I ended up with, take a look at my question.

The fact that I thought your code was slower than it actually was might be an indication about the readability of your code. I think that by getting rid of the recursion the way I did the actual algorithm used is more apparent, and thus it is more clear that the time complexity is not \$O(2^n)\$ which it looks like at first glance.


ActiveRecord SQL search with a filter based on parameters For work, my colleague and I sketched this method up as a brain dump of how we want this method to work. What I am trying to figure out is how can we break this method into smaller components so that it is easier to test to write tests for this. I am trying to avoid putting this into methods where they are coupled by parameters.

def self.for_venue(venue, options = {})
  filter = options[:filter]

  date_conditions = case options[:date]
  when 'upcoming' then  'e.datetime > :datetime'
  when 'past'     then  'e.datetime < :datetime'
  end

  sql_query = [
    "SELECT #{search_fields}",
    "FROM events e #{search_joins}",
    "WHERE " +  [date_conditions, "e.venue_id = #{venue.id}", "e.deleted = 0"].reject(&:nil?).join(" AND ")
  ]
  if options[:limit].present?
    sql_query << "LIMIT #{options[:limit]}"
    sql_query << "OFFSET #{options[:offset]}" if options[:offset].present?
  end
  sql_query << "ORDER BY #{options[:order]}" if options[:order].present?

  sql = ActiveRecord::Base.prepare_sql([sql_query.join(" "), { :datetime => Time.now.beginning_of_day }])
  results = SlaveDB.connection.select_all(sql)

  filter ? filter.select(results) : results
end

Even if you don't want to load your data into ActiveRecord, you can use AR to prepare your SQL statement using to_sql:

def self.for_venue(venue, options = {})

  filter = options[:filter]

  date_conditions = case options[:date]
  when 'upcoming' then  "datetime > ?"
  when 'past'     then  "datetime < ?"
  end

  sql_query = Event
    .where(:venue_id => venue.id, :deleted => 0)
    .where(date_conditions, Time.now.beginning_of_day)
    .joins(search_joins)
    .limit(options[:limit])
    .offset(options[:offset])
    .order(options[:order])
    .to_sql

  results = SlaveDB.connection.select_all(sql_query)

  filter ? filter.select(results) : results

end


Stack using a Linked List I thought implementing a stack using a linked list was a good option since an array implementation involves reallocation of array size and so on.

I have implemented basic Push, Pop and Print operation: What best practices can I follow?

class StackusingLinkedList
{
    public Node<int> Head;
    int Nodecount = 0;
    public void Push(int data)
    {
        if (Nodecount == 0)
        {
            Head = new Node<int>(data);
            Nodecount++;
            return; ;
        }
        var temp = new Node<int>(data);
        temp.Next = Head;
        Head = temp;
        Nodecount++;
    }
    public void Pop()
    {
        var temp = Head;
        Head = temp.Next;
    }
    public void Print()
    {
        var prtemp = Head;
        StringBuilder sb = new StringBuilder();
        while (true)
        {
            if (prtemp == null)
            {
                break;
            }
            sb.Append("|"+prtemp.Data+"|");
            var temp = prtemp;
            prtemp = temp.Next;
        }
        Console.WriteLine(sb.ToString());
    }
}

class Node<T>
{
    public Node<T> Next { get; set; }

    public T Data { get; set; }

    public Node(T data)
    {
        Data = data;
    }

}

Major Issues

The class access modifier does not allow public access, so your class is not usable by third party consumers. Make it public.
Your class should be generic to be usable.
You should always provide unit tests when providing a class to consumers of your API.
Pop is ill-defined. It does not return anything. It can throw a NullReferenceException when the stack is empty. It does not decrement the count. It should throw InvalidOperationException when empty.
A class representing a collection should always implement the correct interfaces. Implement IEnumerable<T> to let consumers iterate the items. Implement IReadOnlyCollection<T> to provide a property Count. Don't be fooled by its name, collections implementing this interface are not stipulated to be immutable. They provide a Count to indicate eager iteration is provided (Interesting Discussion about IReadOnlyCollection).

Review

The name of a class should tell you what it is, not how it's implemented. Call it LinkedStack (conform naming convention LinkedList).
Node could be made public, but then its state should be private or internal at best. Given the few operations you provide, it could be made private. Consider making it public if you would provide mehods as Remove(Node<T> node). This would remove a specific node, even if multiple nodes hold the same value as the specified node.
Push has a redundant creation of a node. Create the node before the first if statement.
Print has no purpose in this class. It should be an extension method declared elsewhere.
Node should be a nested class for better encapsulation.
A property IsEmpty, as suggested in another answer, is syntactic sugar that improves readability.

Refactored

In this solution the Node remains private. Some API's allow it to be public.

public class LinkedStack<T> : IReadOnlyCollection<T>, IEnumerable<T>
{
    private class Node
    {
        internal Node next;
        internal T Value { get; }

        public Node(T value) => Value = value;
    }

    private Node head;

    public void Push(T item)
    {
        var node = new Node(item);
        if (IsEmpty)
        {
            head = node;
        }
        else
        {
            node.next = head;
            head = node;
        }
        Count++;
    }

    public T Pop()
    {
        if (IsEmpty)
        {
            throw new InvalidOperationException();
        }
        var value = head.Value;
        head = head.next;
        Count--;
        return value;
    }

    public int Count { get; private set; }
    public bool IsEmpty => Count == 0;

    public IEnumerator<T> GetEnumerator()
    {
        var node = head;
        while (node != null)
        {
            yield return node.Value;
            node = node.next;
        }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

Unit Tests

[TestClass]
public class LinkedStackTests
{
    [TestMethod]
    public void LinkedStackTest()
    {
        var stack = new LinkedStack<int>();

        stack.Push(0);
        stack.Push(0);
        stack.Push(1);

        Assert.AreEqual(3, stack.Count);

        Assert.AreEqual(1, stack.Pop());
        Assert.AreEqual(0, stack.Pop());
        Assert.AreEqual(0, stack.Pop());

        Assert.AreEqual(0, stack.Count);
    }
}


My yld NPM - callbacks/promises alternative I'd like feedback on my callbacks/promises alternative, please.

The yld repository

var yld;

yld = (function () {
'use strict';

var slice, clearer, defer, prepare, yld;

slice = Array.prototype.slice;

clearer = {
    yld: {
        value: undefined
    },
    throw: {
        value: undefined
    }
};

Object.freeze(clearer);

defer = typeof process === 'object' && typeof process.nextTick === 'function' ? process.nextTick : function nextTick(closure) {
    setTimeout(closure);
};

prepare = function* (parent) {
    var proto, generator, fnGenerator, response;

    proto = {
        yld: function (fn) {
            var parent;

            parent = this;

            return function () {
                var generator, proto, fnGenerator;

                generator = prepare(parent);
                proto = generator.next().value;
                generator.next(generator);
                fnGenerator = fn.apply(proto, arguments);
                generator.next(fnGenerator);

                return Object.create(proto, clearer);
            };
        },
        next: function (value) {
            defer(function () {
                generator.next(value);
            });
        },
        nextCb: function () {
            var value;

            value = slice.call(arguments);

            defer(function () {
                generator.next(value);
            });
        },
        throw: function(error) {
            defer(function() {
                fnGenerator.throw(error);
            });
        }
    };

    if (parent !== undefined) {
        proto.parent = Object.create(parent, clearer);
    }

    generator = yield proto;
    fnGenerator = yield null;

    while (true) {
        response = yield defer(function () {
            fnGenerator.next(response);
        });
    }
};

yld = function (fn) {
    return function () {
        var generator, proto, fnGenerator;

        generator = prepare();
        proto = generator.next().value;
        generator.next(generator);
        fnGenerator = fn.apply(proto, arguments);
        generator.next(fnGenerator);

        return Object.create(proto, clearer);
    };
};

return yld;
}());

if (typeof module === 'object' && module.exports !== undefined) {
    module.exports = yld;
}

Interesting question;

I honestly can't follow the code, and I tried a while. I like to think of myself as above average in JavaScript, which means that from a readability/maintainability perspective you have a problem. It might be related to the fact that you have no comments in there ;)

Some minor style tips

For easier reading you can merge your var declarations with initial assignments, this

var parent;

parent = this;

becomes then

var parent = this;

You make a shortcut for Array.prototype.slice, but you use it only once, I would throw away the shortcut and simply go for value = Array.prototype.slice.call(arguments);.
There is no need to keep creating a function in your loop, just create the function outside of your loop.

while (true) {
   response = yield defer(function () {
      fnGenerator.next(response);
   });
}


Text box intuitive user-interaction using focus listeners This code aims to replace labels used to explain a text box's container. This is done by adding a default string when the text box is considered empty to explain the purpose of the text box. However, when the text box is not considered empty, the text box will behave normally.

This is what I have written to solve my issue, I am posting it here for feedback and tips regarding anything to do with my code.

private void jTextFieldFocusGained(FocusEvent evt) {
    if(jTextField.getText().equals(TEXT_DEFAULT))
    {
        jTextField.setForeground(FOCUSED_COLOR);
        jTextField.setText(TEXT_FOCUSED);
    }
}
private void jTextFieldFocusLost(FocusEvent evt) {
    if(jTextField.getText().equals(TEXT_FOCUSED))
    {
        jTextField.setText(TEXT_DEFAULT);
        jTextField.setForeground(COLOR_DEFAULT);
    }
}
private final String TEXT_FOCUSED = "";
private final String TEXT_DEFAULT = "Type text here...";
private final Color COLOR_FOCUSED = Color.BLACK;
private final Color COLOR_DEFAULT = Color.LIGHT_GRAY;

It is in fact pretty good code, just some small nitpicks:
Order
The fields should go before the methods; in fact, it should be near the front of the class, only after the public static final and private static final fields.
Conventions
This piece of code:

private void jTextFieldFocusGained(FocusEvent evt) {
    if(jTextField.getText().equals(TEXT_DEFAULT))
    {
        jTextField.setForeground(FOCUSED_COLOR);
        jTextField.setText(TEXT_FOCUSED);
    }
}
private void jTextFieldFocusLost(FocusEvent evt) {
    if(jTextField.getText().equals(TEXT_FOCUSED))
    {
        jTextField.setText(TEXT_DEFAULT);
        jTextField.setForeground(COLOR_DEFAULT);
    }
}

In the if statement, if you don't put a new line before the method brace, you shouldn't put a new line before the if brace.

You are inconsistent in order. You set the foreground color first in one, and the text in another. Be consistent.

You should put 2 new lines between methods, and any non-related lines.

It is recommended to have a space after the if.

Naming
evt should be event: names are concise names that shouldn't be abbreviated.
jTextField could just be textField. I know you are using a JTextField, but textField is a more readable name.
Final Code:
private final String TEXT_FOCUSED = "";
private final String TEXT_DEFAULT = "Type text here...";
private final Color COLOR_FOCUSED = Color.BLACK;
private final Color COLOR_DEFAULT = Color.LIGHT_GRAY;

private void textFieldFocusGained(FocusEvent event) {
    if (textField.getText().equals(TEXT_DEFAULT)) {
        textField.setForeground(FOCUSED_COLOR);
        textField.setText(TEXT_FOCUSED);
    }
}

private void textFieldFocusLost(FocusEvent event) {
    if (textField.getText().equals(TEXT_FOCUSED)) {
        textField.setForeground(COLOR_DEFAULT);
        textField.setText(TEXT_DEFAULT);
    }
}


Converting decimal integers to a string representation in an arbitrary base between 2 and 26 This code takes an integer and returns a string representing the value in a different base. The value for the base can range between 2 and 26. I have had someone already look over this code and they told me that I was committing a "cardinal sin" of memory management and referred me to the documentation for malloc(). I am confused as to what he is referring to. The only thing I have come up with so far is that I do not check to see if memory allocation succeeds when using calloc() or malloc(). Are there any other things that could or should be improved?

char int_to_char(int number){
    if (number > 9) return (char)(((int)'A') + number - 10);
    else return (char)(((int)'0') + number);
}

int change_base(char* output, int buffer_size, int decimal_number, int base){
    //check for valid parameters
    if((base < 2) || (base > 26)) return -1;        //range error

    //ready variables
    int output_i = 0;
    int tmp_string_i = 0;
    int dividend;
    char remainder;
    char * tmp_string = calloc(buffer_size, sizeof(char));
    memset(output, '\0', buffer_size*sizeof(char));

    //check for negative input
    if(decimal_number < 0){
        output[0] = '-';
        dividend = -decimal_number;
        output_i++;
    }
    else dividend = decimal_number;

    //find digits
    while(dividend / base != 0){
        remainder = int_to_char(dividend % base);
        dividend = dividend / base;
        tmp_string[tmp_string_i] = remainder;
        tmp_string_i++;
        if(tmp_string_i + 1 > buffer_size){     //+1 for the extra negative sign
            free(tmp_string);
            return -2;      //buffer size error
        }
    }
    //add last digit to string
    remainder = int_to_char(dividend);
    tmp_string[tmp_string_i] = remainder;

    //copy tmp_string to output in reverse order
    for(; tmp_string_i >= 0; tmp_string_i--){
        output[output_i] = tmp_string[tmp_string_i];
        output_i++;
    }
    free(tmp_string);
    return 0;
}

I have also used this for testing:

int main(){
    char * output = calloc(100, sizeof(char));
    int ret_val = 0;

    //test cases
    ret_val = change_base(output, 100, 37, 8);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, -45, 8);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, 33945877, 8);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, 32, -7);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, 37, 4);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, 37, 23);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, 37, 2);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 100, -375212123, 26);
    printf("%s\t%d\n", output, ret_val);
    ret_val = change_base(output, 5, -375212123, 26);
    printf("%s\t%d\n", output, ret_val);
    char * output2 = calloc(5, sizeof(char));
    ret_val = change_base(output2, 5, 37, 2);
    printf("%s\t%d\n", output2, ret_val);

    free(output);
    free(output2);
    return 0;
}

The cardinal sin

An assignment tmp_string[tmp_string_i] = remainder; may write beyond an allocated memory. Consider a case of buffer_size being equal to 0. You must test tmp_string_i before the assignment. I don't know what exactly someone had in mind, but you are definitely committing one.
Unnecessary variables

decimal_number is passed by value; you can mutilate it as much as you want. There is no need to have dividend.

Reversal can be done in-place. There is no need for tmp_string at all. tmp_string_i shall also be gone.
Finding digits

Executing division dividend / base twice per iteration looks wrong; special-case of the last digit looks also wrong. Using do-while solves both wrongs:

    do {
        output[output_i++] = int_to_char(dividend % base);
        number = number / base;
    } while (number != 0);

No raw loops

Most loops represent an important algorithm, which deserves to be factored out in a function of its own, for further reuse and for giving it a name. In your case, the loop under the //copy tmp_string to output in reverse order comment implements an algorithm known as reverse.

All that said, this is

Misc

The returned string should be terminated by '\0', or its actual length be returned.
The code happily converts numbers to a base up to 36 (10 digits and 26 letters). I see no reason to limit a base to 26.
Return codes 0,-1,-2 should be defined as named constants.
Size variables (e.g. buffer_size) should be unsigned, or better yet of type size_t.
The decimal_number parameter is not decimal. It is just number. Need to be renamed accordingly.


Unused variable clean-up I'm fairly new to programming and I'm learning python.
I've defined a function for a dice roll and set it up to roll multiple.
def d20(num):
    rolls = 0
    for i in range(num):
        rolls += randint(1,20)
    return rolls

Thonny and my assistant tab gives me a warning:
Line 3: Unused variable 'i'

The code works fine but I like a clean workspace. I was wondering if there is an alternate way to write the range loop or is this just something I need to learn to deal with?


A program that uses three methods to reverse and print an array I have this code that uses three methods to do certain tasks. One creates an array, another reverses it and the other one prints it out(it was required to use three methods). A person recommended me to upload my code here as there are a few things that need to be improved.
They said that I do not need to return arrays if I modify them in-place, and GenerateNum could allocate the array itself instead of requiring an array with the correct size to be passed. However as a beginner I didn't quite catch what they were trying to tell me to change in the code. Please help me.
using System;

class Program
{
    static void Main(string[] args)
    {
        string getnums = Console.ReadLine();
        int getnum = Convert.ToInt32(getnums);
        int[] array1 = new int[getnum];

        GenerateNum(getnum,array1);
        RevArray(array1);
        PrintArray(array1);
    }
    static int[] GenerateNum(int getnums, int[] arrayy)
    {

        for (int index = 0; index < getnums; index++)
        {
            arrayy[index] = index;
        }
        return arrayy;
    }
    static int[] RevArray(int[] arrayy)
    {
        for (int index = 0; index < (arrayy.Length)/2; index++)
        {
            int a = arrayy[index];
            arrayy[index] = arrayy[arrayy.Length - index-1];
            arrayy[arrayy.Length-index-1] = a;
        }
        return arrayy;
    }
    static void PrintArray(int[] arrayy)
    {
        Console.WriteLine(string.Join(",",arrayy));
    }
}

Let's review your code line by line
string getnums = Console.ReadLine();

The name getnums is not really a good name for a string variable. When you name a variable please try to use noun(s) (w/o adjective). Methods should start with verbs.

int getnum = Convert.ToInt32(getnums);

This is quite error-prone. What if the user types three? Your application will crash with a FormatException.
Please prefer int.TryParse and remember that all input is evil (until proven otherwise).

int[] array1 = new int[getnum];

Yet again array1 is not really a good name. Someone who reads your code will not know what was your intent with this variable. Try to write code that is understandable for others as well.
This is quite error-prone again, because getnum can contain negative number. The array allocation attempt might result in a OverflowException.
Consider to use ArrayPool if you try to allocate an quite big array.

static int[] GenerateNum(int getnums, int[] arrayy)

The arrayy is really a bad name here, you are using Hungarian notation which should be avoided.
You don't need to pass getnums because you can get the Length of the array.
With the current implementation the PopulateArray might be a better name.
Depending on your design the responsibility to allocate memory can be moved over here as well.

for (int index = 0; index < getnums; index++)

Feel free to use array.Length instead of getnums

arrayy[index] = index;

If you design your application in the way that this method should allocate the memory as well then you could rewrite this whole method with the following command:
return Enumerable.Range(0, getnums).ToArray();

int[] RevArray(int[] arrayy)

As it was stated by others as well abbreviation does not add anything here. On the contrary it might make confusion because it could mean other things as well not just reserve, like reveal, revive, review, revisit, etc...

for (int index = 0; index < (arrayy.Length)/2; index++)

The Array class defines a Reverse method, which can do the whole in-place replacement on your behalf without reinventing the wheel.
Here you are using the Length property which is good. I suspect that this code is based on this SO answer. I strongly encourage you to add there a comment where you mention the source of your code.


Listing EC2 instances I've got this massive nested for loop with a few if and else statements in nodeJs. The output of all of this is just string which is combined from different values out of JSon object. What I want to do is rewrite it in more efficient way as JSlint plugin for sublime is complaining about complexity of it:

var instancesList = '';
    ec2.describeInstances(params,function(err, data) {
        if (err){
            console.log(err, err.stack);
        } else {
                for (var i in data.Reservations){
                    for (var j in data.Reservations[i].Instances[0].Tags){
                        if(data.Reservations[i].Instances[0].Tags[j].Key == 'Name'){
                            instancesList+= 'Name: ' + data.Reservations[i].Instances[0].Tags[j].Value +
                                           '  id: ' +data.Reservations[i].Instances[0].InstanceId +
                                           '  Status: ' + data.Reservations[i].Instances[0].State.Name +'\n';
                        }
                    }
                }
        }
console.log(instancesList);

Any help, ideas or a link on how to deal with these kind of situations would be greatly appreciated.

Honestly, your loop doesn't seem that complex. I would probably ignore this warning in your case, at least until it grew in complexity or the surrounding function grew in size.

In any case, here are some things you could change:

a. Add a variable to replace the five instances of data.Reservations[i].Instances[0].

b. Off the top of my head, I'm pretty sure an EC2 instance is only going to have one "Name" tag. You can extract this loop into a separate function if you feel comfortable with this assumption.

c. If your lint rules / personal style allows for multiple returns, you can reduce the depth by returning from the error.

var instancesList = '';
ec2.describeInstances(params,function(err, data) {
  if (err) return console.log(err, err.stack);

  for (var i in data.Reservations){
    var ins = data.Reservations[i].Instances[0]
    var name = findInstanceName(ins)
    if(name === null) continue;

    instancesList+= 'Name: ' + name +
                    '  id: ' +ins.InstanceId +
                    '  Status: ' + ins.State.Name +'\n';
  }
  console.log(instancesList);
}

function findInstanceName(ins){
  for(var j in ins.Tags){
    if(ins.Tags[j].Key === 'Name'){
      return ins.Tags[j].Value;
    }
  }
  //I'm not sure skipping the instance is the best choice.
  //You might instead return "<Unknown>" so you can know you
  //have an unnamed instance running.
  return null;
}

Brief aside: If this script is only to list the instances for you, you might find it useful to check out the aws-cli and the jq tool:

aws ec2 describe-instances | jq -r '.Reservations[].Instances[] | [(.Tags | from_entries | .Name), .InstanceId, .State.Name] | @tsv' | column -t -s $'\t'

Output:

hello-world      i-a1a1a1a1  running
jenkins          i-b2b2b2b2  running
production       i-c3c3c3c3  running
qa               i-d4d4d4d4  stopped
testing          i-e5e5e5e5  terminated


Comparator Implementation I am trying to compare Entries by their frequencies using Comparator. I wanna know if this piece of code would work for this case?

    // Creates entries
private class Entry {
    private String word;
    private long count;

    public Entry(String w, long c) {
        word = w;
        count = c;
    }

    public String getWord() { return word; }
    public long getCount() { return count; }

    public Entry(String entry) {
        String parts[] = entry.split("\t");
        word = parts[1].trim();
        count = new Long(parts[0].trim());
    }

    public String toString() {
        return "(" + word + "," + count + ")";
    }

}

    // compare Entry objects by their frequencies
private static Comparator<Entry> countComparator() {
    return new Comparator<Entry>() {
        public int compare(Entry e1, Entry e2) {
            if(e1.getCount() > e2.getCount()) {
                return 1;
            }
            else if(e1.getCount() < e2.getCount()) {
                return -1;
            }
            else {
                return 0;
            }
        }
    };
}

The comparator should work fine, however return Integer.compare(int, int) avoid the re-invention of the wheel.


Binary addition with strings The following method will add two strings of any length as binary numbers assuming the characters 1 and 0. I made this for fun in my spare time.  Improvements are not critical but I would like to hear them. I would also like to know if there is a more efficient algorithm to simulate a ALU.

static string BinAdd(string a, string b)
{
    if (a.Length < b.Length)
    {
        string c = a;
        a = b;
        b = c;
    }

    StringBuilder sb = new StringBuilder(a.Length);

    bool addLeft = false;

    for (int i = a.Length - 1, j = b.Length - 1; i >= 0; i--, j--)
    {
        char c = '1';
        bool add = addLeft;
        addLeft = false;

        if (j >= 0)
        {
            if (a[i] == '1' && b[j] == '1')
            {
                c = '0';
                addLeft = true;
            }
            else if (a[i] == '0' && b[j] == '0')
            {
                c = '0';
            }
            else if (add)
            {
                addLeft = true;
            }
        }
        else
        {
            if (a[i] == '0')
            {
                c = '0';
            }
            else if (add)
            {
                addLeft = true;
            }
        }

        sb.Append(add ? (c == '1' ? '0' : '1') : c);
    }

    if (addLeft)
    {
        sb.Append('1');
    }

    char[] cx = new char[sb.Length];
    sb.CopyTo(0, cx, 0, sb.Length);
    Array.Reverse(cx);

    return new string(cx);
}

I'm aware that no validation is done, but I didn't intend to.

strings of any length as binary numbers

Why are they strings? Something like bool[] would fit much better and it would also make your code simpler.

string a, string b

Those are not very good names. maybe use something like left and right or first and second?

In production code, you should verify that parameters fit your requirements (in your case, that the string contains only '0's and '1's) and throw an exception otherwise.

if (a.Length < b.Length)
{
    string c = a;
    a = b;
    b = c;
}

A simpler way to write this would be to use recursion:

if (a.Length < b.Length)
{
    return BinAdd(b, a);
}

bool addLeft = false;

A better name for this variable would be carry.

char c = '1';
bool add = addLeft;
addLeft = false;

if (j >= 0)
{
    if (a[i] == '1' && b[j] == '1')
    {
        c = '0';
        addLeft = true;
    }
    else if (a[i] == '0' && b[j] == '0')
    {
        c = '0';
    }
    else if (add)
    {
        addLeft = true;
    }
}
else
{
    if (a[i] == '0')
    {
        c = '0';
    }
    else if (add)
    {
        addLeft = true;
    }
}

sb.Append(add ? (c == '1' ? '0' : '1') : c);

All this logic could be simplified by a lot by using a variable to count the ones (including carry). Something like (not tested):

int sum = 0;

if (j >= 0 && b[j] == '1')
    sum++;

if (a[i] == '1')
    sum++;

if (addLeft)
    sum++;

sb.Append(sum % 2);

addLeft = sum / 2 == 1;

char[] cx = new char[sb.Length];
sb.CopyTo(0, cx, 0, sb.Length);
Array.Reverse(cx);

return new string(cx);

Instead of this, you could start with List<char> instead of StringBuilder, Reverse() that and then create a string out of it (possibly using string.Join(null, digits), assuming you name the list digits).


Good usage of MailboxProcessor/Agent in class rather than mutable field? I've written a simple Simple Service Discovery Protocol (SSDP) module.  This module contains a class, Session that you create and then use to issue commands and subscribe to interesting events.  I include the relevant code snippets below, but you can also view the entire SSDP module gist.

The usage looks like this:

//Fire up a session.
let ssdpSession = Ssdp.Session.Start(1985)
//Startlistening
ssdpSession.MessageReceived.Add(fun msg -> printfn "%s" msg)
//Do an MSearch.
let search = { MSearch.SearchTarget = All; DeviceResponseDelay = OneSecond; }
ssdpSession.BroadcastMSearch(search)

It is intended that you hang on to the Session instance for as long as you need it.  However, the Session is listening on one or more IP addresses for the lifetime of your usage, and these IP addresses may change (for example, you renew an IP DHCP lease).  The Session class uses System.Net.NetworkInformation.NetworkChange.NetworkAddressChanged to detect this.

The question: What would be the elegant/idiomatic options to handle the changing of the IP and hence UdpClient listener set?  I am trying to avoid mutable fields, but I wonder if I'm going too far with my approach below (just tell me, am I an astronaut!?).  Are there elegant/idiomatic approaches for this?

What I have done so far, is run an MailboxProcessor (Agent) that fires up the UdpClient listeners and then waits for a message to fire up a new set (or tear them all down for good).  The current set is passed to the recursive function running in the agent, so we always have it to hand when a change is requested or it is time to kill things off.

It looks like this:

let udpClientListeningController =
        Agent.Start(fun inbox ->

            let rec loop (runningClients:UdpClient seq) = async {
                let! msg = inbox.Receive()

                let closeRunningClients () =
                    runningClients |> Seq.iter (fun client -> client.ProtectedClose())

                match msg with

                | ListenForSearchResponsesAndNotifyAdverts (port, ipAddresses) ->

                    closeRunningClients ()

                    let uniAndMulticastUdpClients = seq {
                        let portIPPairs = ipAddresses |> Seq.zip (seq { yield port })
                        yield! portIPPairs |> Seq.map createUnicastClient
                        yield! portIPPairs |> Seq.map (snd >> createMulticastClient)
                    }

                    //Start em up.
                    uniAndMulticastUdpClients
                    |> Seq.iter (udpClientReceiveLoopAsync >> Async.Start)

                    return! loop uniAndMulticastUdpClients

                | StopListening ->
                    closeRunningClients ()
                    return! loop Seq.empty
            }
            loop Seq.empty
        )

Your code looks pretty good to me - I would only change some minor details.

I'd replace UdpClient seq with UdpClient list, because lazy sequences can be subtle. In your code, this doesn't seem to be a problem, but they can cause unexpected GC issues (by capturing a reference) and tricky performance behavior (by re-evaluating things).
I'd probably write the creation of clients differently - I think the following is easier to understand:

let uniAndMulticastUdpClients =
  [ for ip in ipAddresses do
      yield createUnicastClient(ip, port)
      yield createMulticastClient(ip) ]

You're calling closeRunningClients in both branches, so you can extract it and move it before the match construct (and, in fact, you can just use the Seq.iter call without defining a function for it).


C# Pseudo-Code improvement/peer revision I have written this Pseudo-Code for the following program, I'm wondering if this might be descriptive enough for anyone to follow up and perform more or less the same program. The program reads and analyzes a set of data in a text file, then displays the information in a filtered way. Any improvements/ revision would be immensely appreciated.

PseudoCode:

Module main ()
//Set up format for text reading.
Declare String content
//Open and read file
Open “Data.txt”
Set contents = opened “Data.txt”

//Declare an array to store and split data

Declare string rows[] = contents split to lines
Declare string table [][] = length of rows[]

//Set loop to iterate through all values

For i is 0 less than length of rows
Table[] increments by 1
End for
//Declare arras to select and parse info
[]districts integer = integer [length of rows]
[] ages integer = integer [length of rows]
For i is 0, i is less than length of rows
i increments by 1
Set i of districts to integer parsing with position [i] in table[]
Set i of ages to integer parsing with position [i] in table[]
End for
// Analyze and pass data
Foreach integer in district
Display “district {0} has {1} resident”, district, countofdistrict.
Display “ Agegroup1 has {0} residents”, count of ages (restriction)
Display “Agegroup2 has {0} residents”, count of ages (restriction)
Display “ Agegroup3 has {0} residents”, count of ages (restriction)
Display “ Agegroup4 has {0} residents”, count of ages (restriction)
Display “ Agegroup5 has {0} residents”, count of ages (restriction)
End foreach

Program:

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

    namespace Project_2_practice
    {
        class Program
        {
           static void Main(string[] args)
            {

                //Parsing data into memory
                string content;
                using (StreamReader reader = new StreamReader(File.Open("Data.txt", FileMode.Open)))
                {
                    content = reader.ReadToEnd();
                }
                string[] rows = content.Split('\n'); //Each row is on a new line
                string[][] table = new string[rows.Length][];
                for (int i = 0; i < rows.Length; table[i] = rows[i].Split(','), i++) ;

         //selecting information

                int[] districts = new int[rows.Length];
                int[] ages = new int[rows.Length];

                for (int i = 0; i < rows.Length; i++)
                {
                    districts[i] = int.Parse(table[i][3]);
                    ages[i] = int.Parse(table[i][0]);
                }
                    //Analyzing selected information
                    foreach (int district in districts.Distinct().OrderBy(x => x))
                    Console.WriteLine("District {0} has {1} resident(s)", district, districts.Count(x => x == district));
                    Console.WriteLine("Ages 0-18 : {0} resident(s)", ages.Count(x => x < 18));
                    Console.WriteLine("Ages 18-30 : {0} resident(s)", ages.Count(x => x >= 18 && x <= 30));
                    Console.WriteLine("Ages 31-45 : {0} resident(s)", ages.Count(x => x >= 31 && x <= 45));
                    Console.WriteLine("Ages 46-64 : {0} resident(s)", ages.Count(x => x >= 46 && x <= 64));
                    Console.WriteLine("Ages >=65 : {0} resident(s)", ages.Count(x => x >= 65));
            }
        }
    }

When it comes to pseudo-code there are different standards depending on your organization, target group, etc, but here is my input:

The comments in your pseudo-code are redundant.

17 and 18 should be merged.

Variable declaration (such as 3. Declare String content) are not needed.

Not sure if every Console.WriteLine() is needed. This could be replaced with something like "Display data regarding Agegroup"


Factory style pattern in method I have a method in a class, and its sole purpose is to prepare another class for use:

private function createHttpPost($endPoint) {
    $httpPost = new HttpPost($this->baseUri . '/' . $endPoint);
    $httpPost->setContentType('application/json');
    $httpPost->setAccept('application/json');

    if($this->auth) {
        $this->authenticate();
        $httpPost->addCookies(array($this->sessionName => $this->sessionId));
        $httpPost->addHeaders(array('X-CSRF-Token' => $this->token));
    }

    return $httpPost;
}

I am familiar with the factory design pattern, but I'm not sure it is a good idea to apply that kind of thinking to a class method. Should I take this code out and put it in a new class called JsonHttpPostFactory, or something like that?

Is this a bad idea?  If so, why?

In itself, there's nothing wrong with this code. There really isn't. But there are some things you need to consider:

The method is private, so it's an internal kind of factory, why not use a lazy-loading getter?
The method is private, that makes this method a dead-end in a scenario where, for example, the content-type needs to be changed
If you have access to the HttpPost class, why not add a method setEndPoint, so you don't need to create a new instance time and time again

On the whole, I'd start with changing the access modifier to protected, so that in a child class, you can do something like:

protected function createHttpPost($endPoint, array $options = null)
{
    if ($options === null)//default, use parent version
        return parent::createHttpPost($endPoint);
    $httpPost = new HttpPost($this->baseUri.'/'.$endPoint);
    $httpPost->setContentType($options['contentType'])
        ->setAccept($options['accept']);
    //and so on...
}

If you can alter the HttpPost class, I'd also encourage you to add methods that reset the instance to a pristine state (a state that allows another request). Then you can lazy-load this instance as a property, and reuse it:

protected function createHttpPost($endPoint, array $options)
{
    $httpPost = $this->getHttpPost();//see below
    $httpPost->setContentType($options['contentType'])
        ->setAccept($options['accept'])
        ->setEndPoint($this->baseUri.'/'.$endPoint);
    //and so on
}
/**
 * Lazy-loader for HttpPost instance
 * @return HttpPost
 */
protected function getHttpPost()
{
    if ($this->httpPost === null)
        $this->httpPost = new HttpPost;
    else
        $this->httpPost->resetInstance();
    return $this->httpPost;
}

This way, the constructor will only be called once, and only when the instance of HttpPost is required. Then, you can re-use the instance for each request.
If the instance of HttpPost is passed around, or a request is not made instantaneously, then there could be a risk of this request object losing its state, before a request is made. If this is possible, then of course, you don't use a lazy-loader like the one above, but just use a getter function, or keep doing what you do (ie: create an instance ad-hoc).


Two strings to one containing distinct letters I've been doing lately CodeWars and here's my working code for this challenge.

The challenge :

  Take 2 strings s1 and s2 including only letters from a to z. Return a new sorted string, the longest possible, containing distinct letters, - each taken only once - coming from s1 or s2.

My code :

public static String longest(String s1, String s2) {
    StringBuilder sb = new StringBuilder();
    sb.append(s1).append(s2);

    String s = sb.toString();
    String result = "";

    for (int i = 0; i < s.length(); i++) {
        if (!result.contains(String.valueOf(s.charAt(i)))) {
            result += s.charAt(i);
        }
    }
    char[] ch = result.toCharArray();
    Arrays.sort(ch);

    return String.valueOf(ch);

}

Is there a way to skip all the transformations to arrays and then back to a String? I found it quite useful to use Arrays.sort because I didnt have to do another loop to sort chars in a String (dont think there is a StringBuilder method to sort chars as String is immutable, right?) Saw other solutions and many of them use streams but I havent learned about them yet so not trying to use them. Also I think there is no point in assigning String s becasue I can use sb.length() if a for loop and avoid another variable, right?

First of all, you can work directly with chars instead of creating one char Strings.

Second of all, looking at the challenge description

  Take 2 strings s1 and s2 including only letters from ato z. Return a new sorted string, the longest possible, containing distinct letters, - each taken only once - coming from s1 or s2.

you disregard an important part - there is a small number of possible unique characters in the input Strings. You can use that information to your advantage and eliminate the need to explicitly sort the output String.

For example, you can use an int array of length 26 to indicate for each letter of the abc whether it is present in one of the input Strings.

You can populate the array by iterating once over each of the input Strings.
Then you can iterate once over that array and add to the output String the characters corresponding with indices of the array for which the value > 0.

This would reduce your time complexity from O(nlogn) to O(n).

public static String longest(String s1, String s2) {
    int[] chars = new int[26];
    for (char c : s1.toCharArray()) {
        chars[c-'a']=1;
    }
    for (char c : s2.toCharArray()) {
        chars[c-'a']=1;
    }
    StringBuilder result = new StringBuilder(26);
    for (int i = 0; i < chars.length; i++) {
        if (chars[i] > 0)
            result.append ((char)(i+'a'));
    }
    return result.toString();
}


Improve file-path finding method I am using a recursive algorithm to find all of the file-paths in a given directory: it returns a dictionary like this: {'Tkinter.py': 'C:\Python27\Lib\lib-tk\Tkinter.py', ...}.

I am using this in a script to open modules by solely given the name. Currently, the whole process (for everything in sys.path) takes about 9 seconds. To avoid doing this every time, I have it save to a .pkl file and then just load this in my module-opener program.

The original recursive method took too long and sometimes gave me a MemoryError, so what I did was create a helper method to iterate through the subfolders (using os.listdir), and then call the recursive method.

Here is my code:

import os, os.path
def getDirs(path):
    sub = os.listdir(path)
    paths = {}
    for p in sub:
        print p
        pDir = '{}\{}'.format(path, p)
        if os.path.isdir(pDir):
            paths.update(getAllDirs(pDir, paths))
        else:
            paths[p] = pDir
    return paths

def getAllDirs(mainPath, paths = {}):
    subPaths = os.listdir(mainPath)
    for path in subPaths:
        pathDir = '{}\{}'.format(mainPath, path)
        if os.path.isdir(pathDir):
            paths.update(getAllDirs(pathDir, paths))
        else:
                paths[path] = pathDir
    return paths

Is there any way to make this faster? Thanks!

import os, os.path
def getDirs(path):

Python convention is to use lowercase_with_underscores for function names

    sub = os.listdir(path)

Don't needlessly abbreviate, and at least have it be a plural name.

    paths = {}
    for p in sub:

You don't need to store things in a temporary variable to loop over them

        print p

Do you really want this function printing?

        pDir = '{}\{}'.format(path, p)

Use os.path.join to join paths. That'll make sure it works regardless of your platform.

        if os.path.isdir(pDir):
            paths.update(getAllDirs(pDir, paths))

You shouldn't both pass it and update it afterwords. That's redundant.

        else:
            paths[p] = pDir
    return paths

def getAllDirs(mainPath, paths = {}):

Don't use mutable objects as default values, they have unexpected behavior.

    subPaths = os.listdir(mainPath)
    for path in subPaths:
        pathDir = '{}\{}'.format(mainPath, path)
        if os.path.isdir(pathDir):
            paths.update(getAllDirs(pathDir, paths))
        else:
            paths[path] = pathDir
    return paths

This whole section is repeated from the previous function. You should combine them.

Take a look at the os.walk function. It does most of the work you're doing here and you could use to simplify your code.


find matches between attributes from two different Objects I'm new in Java 8, and I'm trying to convert and optimize my method from java 6 to java 8. I want it to be less verbose, with fewer lines of code.
This method gets an object (userRequest) and tries to find from a list of other object type (User) the one with more matches between their attributes(for example, if the user requested has brown eyes, each user from the list who has brown eyes has one more match). If it results that there is more than one User, so the one with less age is chosen.
In this example it's only used a list of 3 users and only 3 or 4 attributes, but in the future it's intended to be more, so I'd like to optimize it for multiple users and attributes.
@Getter
@Setter
@AllArgsConstructor
@ToString
public class UserRequest{
    private String name;
    private String surname;
    private String company;
    private String eyesColor;
}

@Getter
@Setter
@AllArgsConstructor
@ToString
public class User{
    private Long id;
    private String name;
    private String surname;
    private String eyesColor;
    private int age;
}

@Service
public class UserServiceImpl implements UserService{

    @Autowired
    private UserRepository userRepository;

    @Autowired
    private UserFilters userFilters;

    public int getMostMatchingUserId(UserRequest userRequest){

        List<User> userList = userRepository.getUsers();

        List<User> mostMatchingUserList = userFilters.getMostMatchingUserList(userList, userRequest);

        return mostMatchingUserList.stream().min(Comparator.comparing(User::getAge)).orElse(mostMatchingUserList.get(0)).getId();
    }
}

@UtilityClass
public class UserFilters{

    public List<User> getMostMatchingUserList (List<User> userList, UserRequest userRequest) {

    List<User> newUserList = new ArrayList<>();
    int matchesInitialCount = 0;

    for(User user: userList){
        int userMatchesCount = countMatches(user, userRequest);
        if(userMatchesCount > matchesInitialCount){
            newUserList.clear();
            newUserList.add(user);
            matchesInitialCount = userMatchesCount;
        } else if(userMatchesCount == matchesInitialCount){
            newUserList.add(user);
        }
    }
    return newUserList;
    }

    private int countMatches (User user, UserRequest userRequest) {
        int matchesCounted = 0;

        if (Objects.nonNull(user.getName()) && user.getName().equals(userRequest.getName())) {
            matchesCounted++;
        }
        if (Objects.nonNull(user.getSurname()) && user.getSurname().equals(userRequest.getSurname())) {
            matchesCounted++;
        }
        if (Objects.nonNull(user.getAge()) && user.getAge() == userRequest.getAge() {
            matchesCounted++;
        }
        return matchesCounted;
    }
}

Imagine you get a new userRequest ("John", "Jackson", "aena", "brown") to check which one of the next userList has the most matches:
User1 (111, "John", "Wine", "blue", 37);
User2 (222, "Maria", "Jackson", "brown", 25);
User3 (333, "Michel", "Jackson", "brown", 55);
The result is: User1 has one match and User2 & User3 have two matches each. The next validation checks who is the youngest one from User2 & User3: so User2 will be the chosen one.

First off, a bug (or probably just a copy/paste error): countMatches is comparing age instead of eye color.
Then, any reason you are limited to Java 8? At the very least you should be using the LTS release 11, or better the current release 15.
Also, you have some minor formatting issues that don't confirm with standard Java conventions. Mostly it's wrong spacing: there should be spaces before { and between keywords (e.g. if) and (, but no spaces between method names and (.
Lombok
The annotations on the data classes could be most likely simplified to @Data @AllArgsConstructor. And you should even consider using @Value instead for immutable objects when ever possible, which can help avoiding bugs.
Spring DI
Generally autowiring private properties has been discouraged. You should prefer constructor injection. With Lombok's @RequiredArgsConstructor it becomes quite easy:
@Service
RequiredArgsConstructor
public class UserServiceImpl implements UserService{

    private final UserRepository userRepository;

    private final UserFilters userFilters;

    //....
}

Since you are injecting UserFilters it shouldn't be static i.e. @UtilityClass.
For a matter of fact I don't see why UserFilters is a separate class at all. Those methods don't look like they should be used outside of UserService, so they should belong as (private) methods in UserService.
Method UserServiceImpl.getMostMatchingUserId
The name isn't quite right and doesn't give enough information in my opinion. It's not a "getter", so it should be called find... and nt get..., the meaning "Most matching" needs to be defined in a comment and the word "youngest" needs to be added.
Also it seems quite limiting to have the method return just the user ID.
One problem in here is the use of orElse on the result of min(). min() only returns an empty Optional, if the stream is empty, so in that case .get(0) will throw an exception.
A final though here, is that normally one would expect that the repository (or usually the database behind it) should filter the list of results. If the database would do the filtering, then this would scale better with more users.
Method UserFilters.getMostMatchingUserList
Also not a "getter". filter... would be the most appropriate prefix here.
One could consider using a stream here, however in this specific case I don't think it would make the task much simpler/better to read. The only change I'd make here would be choose some better variable names: newUserList should be called filteredUserList and matchesInitialCount should be currentMatchesCount.
Method UserFilters.countMatches
This implementation is fine. Personally I'd leave it like this, and just add/change more property checks when needed, unless the list of properties needs to changed quickly or easily, for example, via configuration. It that case you need to work with reflection to find the properties.


Simple product inventory project The goal:

  Create an application which manages an inventory of products. Create a product class which has a price, id, and quantity on hand. Then create an inventory class which keeps track of various products and can sum up the inventory value.

I figured out how to get a list of dictionaries, and then add up the price. My output isn't pretty, but before I continue, I would simply like general critique.

class Product:
    def __init__(self, i, p, q):
        self.identification = i
        self.price = p
        self.quantity = q
        self.item_dictionary = {'ID: ': self.identification, 'Price: ': self.price, 'Quantity: ': self.quantity}

class Inventory:
    def __init__(self):
        self.identification, self.price_list, self.quantity_list = [], [], []
        self.products = [
            product_1.item_dictionary,
            product_2.item_dictionary
        ]  # Make list of item dictionaries

    for i in self.products:
        self.identification.append(i['ID: '])
        self.price_list.append(i['Price: '])
        self.quantity_list.append(i['Quantity: '])
        # Make unique lists of ID, Price, and Quantity

    print(sum(self.price_list))

product_1 = Product(32, 23, 2)
product_2 = Product(23, 65, 3)

inventory_1 = Inventory()

Single letter names are not readable in general. i, p and q, for example, would be easier to read as identification, price, and quantity. This is also a common pattern, where parameters to be stored as fields are named the same as the fields.
The last three lines should be in a main function, called if __name__ == '__main__'.
item_dictionary would be better expressed either by making Product a dict subtype or making an as_dict method.
product_1 and product_2 should be passed as either a products array argument or a *products star argument to Inventory().
Why unpack the products in Inventory?
The code directly inside Inventory (for loop and print()) should be in a method. I've never even seen Python code with something other than method definitions at the top level.
Product quantity is a property of an inventory, not a product.

I would probably implement this as follows (untested):

class Product:
    def __init__(self, identification, price):
        self.identification = identification
        self.price = price

class Inventory:
    def __init__(self, products):
        self.products = products

    @property
    def value():
        return sum(product.price for product in self.products)

def main():
    product_1 = Product(32, 23)
    product_2 = Product(23, 65)
    inventory = Inventory(2 * [product_1] + 3 * [product_2])
    print(inventory.value)

if __name__ == '__main__':
    main()


Lookups in list in C I read about lookups in lists and I think that is what I did. I would like to compare it to a skip list, a binary search or a B+ tree.

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

typedef struct node {
    int node_id;
    int data;
    struct node *left;
    struct node *right;
} node;

node *newNode(int data, int node_id) {
    node *new_node = (node *) malloc(sizeof(node));
    new_node->data = data;
    new_node->node_id = node_id;
    new_node->right = new_node->left = NULL;
    return new_node;
}

node *insert_node(node *root, int data, int node_id) {
    if (root == NULL)
        return newNode(data, node_id);
    else {
        node *cur;
        if (node_id < root->node_id) {
            cur = insert_node(root->left, data, node_id);
            root->left = cur;
        } else if (node_id > root->node_id) {
            cur = insert_node(root->right, data, node_id);
            root->right = cur;
        }
    }
    return root;
}

node *find_node_data(node *root, int node_id) {
    if (root == NULL)
        return NULL;
    else if (root->node_id == node_id)
        return root;
    else if (root->node_id > node_id)
        return find_node_data(root->left, node_id);
    else
        return find_node_data(root->right, node_id);
}

void print(node *np) {
    if (np) {
        print(np->left);
        printf("(%d, %d)", np->node_id, np->data);
        print(np->right);
    }
}

int main() {
    int T = 1000; //test case 1000 nodes
    int data, node_id;
    //printf("Input number of nodes:");
    //scanf("%d", &T);
    node *root = NULL;
    srand (time (NULL));
    while (T-- > 0) {
        //printf("Input data. %d:", T);
        //scanf("%d %d", &data, &node_id);
        int r = rand() % 1000;
        int r2 = rand() % 1000;
        root = insert_node(root, r, r2);
    }
    print(root);
    printf("\n");
    printf("Find data at node:", T);
    scanf("%d", &T);
    printf("node data %d", find_node_data(root, T)->data);
    return 0;
}

This is a binary search tree.
Do not cast the return value of malloc. It serves no purpose, and may mask a serious bug (in case stdlib.h is not included).
Lookup is usually implemented iteratively:

while (root) {
    if (root->id == id) {
        break;
    }
    root = (root->id < id)? root->right: root->left;
}
return root;

insert_node may use less indentation. Early return makes else unnecessary:

node *insert_node(node *root, int data, int node_id) {
    if (root == NULL)
        return newNode(data, node_id);
    node *cur;
    if (node_id < root->node_id) {
        cur = insert_node(root->left, data, node_id);
        root->left = cur;
    } else if (node_id > root->node_id) {
        cur = insert_node(root->right, data, node_id);
        root->right = cur;
    }
    return root;
}

You may want to inform the caller that the insertion failed due to id already present.


Calculate the Euler-Mascheroni constant without the math module The below code was written to generate γ, for educational purposes.

Single threaded, no functional zeroes required, no binary splitting(which can all be used to compute competitively like y-cruncher, that version in works).
Uses the Arithmetic Geometric Mean to calculate large logarithms quickly.
Uses decimal module for precision management.

I've computed 3000 digits in a few hours with it, and 200 in about a minute. I am happy calculating.

import decimal

D = decimal.Decimal

def agm(a, b):      #Arithmetic Geometric Mean
    a, b = D(a),D(b)
    for x in range(prec):
        a, b = (a + b) / 2, (a * b).sqrt()
    return a

def pi_agm():       #Pi via AGM and lemniscate
    a, b, t, p, pi, k = 1, D(2).sqrt()/2, 1/D(2), 2, 0, 0
    while 1:
        an    = (a + b) / 2
        b     = (a * b).sqrt()
        t    -= p * (a - an)**2
        a, p  = an, 2**(k+2)
        piold = pi
        pi    = (a + b) * (a + b) / (2*t)
        k    += 1
        if pi == piold:
            break
    return pi

def factorial(x):       #factorial fast loop
    x = int(x)
    factorial = D(1)
    for i in range(1, x+1):
        factorial *= i
    return factorial

def lntwo():        #Fast converging Ln 2
    logsum, logold, n = D(0), D(0), 0
    while 1:
        logold = logsum
        logsum += D(1/((D(961**n))*((2*n)+1)))
        n += 1
        if logsum == logold:
            logsum1 = (D(14)/D(31))*logsum
            break
    logsum, logold, n = D(0), D(0), 0
    while 1:
        logold = logsum
        logsum += D(1/((D(25921**n))*((2*n)+1)))
        n += 1
        if logsum == logold:
            logsum2 = (D(6)/D(161))*logsum
            break
    logsum, logold, n = D(0), D(0), 0
    while 1:
        logold = logsum
        logsum += D(1/((D(2401**n))*((2*n)+1)))
        n += 1
        if logsum == logold:
            logsum3 = (D(10)/D(49))*logsum
            break
    ln2 = logsum1 + logsum2 + logsum3
    return ln2

def lnagm(x):   #Natural log via AGM,
    try:
        if int(x) == 1:
            return 0
        if int(x) == 2:
            return lntwo()
    except:
        pass
    m = prec*2
    ln2 = lntwo()
    decimal.getcontext().prec = m
    pi = D(pi_agm())
    twoprec = D(2**(2-D(m)))/D(x)
    den = agm(1, twoprec)*2
    diff = m*ln2
    result = (D(pi/den) - D(diff))
    logr = D(str(result)[:m//2])
    decimal.getcontext().prec = prec
    return logr

def gamma():   #Compute Gamma from Digamma Expansion
    print('Computing Gamma!')
    k = D(prec/2)
    print('Calculating Logarithms...')
    lnk = lnagm(k)
    logsum = D(0)
    upper = int((12*k)+2)
    print('Summing...')
    for r in range(1, upper):
        logsum += D((D(-1)**D(r-1))*D(k**D(r+1)))/D(factorial(r-1)*D(r+1))
        if r%1000==0:
            print(str((D(r)/D(upper))*100)[:5], '% ; Sum 1 of 2')
    logsum1 = D(0)
    print('...')
    for r in range(1, upper):
        logsum1 += D((D(-1)**D(r-1))*(k**D(r+1)))/D(factorial(r-1)*D(D(r+1)**2))
        if r%1000==0:
            print(str((D(r)/D(upper))*100)[:5], '% ; Sum 2 of 2')
    twofac = D(2)**(-k)
    gammac = str(D(1)-(lnk*logsum)+logsum1+twofac)
    return D(gammac[:int(prec//6.66)])

#Calling Gamma
prec = int(input('Precision for Gamma: '))*8
decimal.getcontext().prec = prec
gam = gamma()
print(gam)

Computation of logsum and logsum1 in gamma() are suboptimal. You do costly operations of raising to power, and recompute factorial on each iteration (the latter invokes the quadratic time complexity BTW). Notice that in the \$\sum \dfrac{(-1)^{r-1} k^{r+1}}{(r+1)(r-1)!}\$ a consecutive term can be expressed via the previous one, as\$T_{r+1} = -k\dfrac{r+1}{(r+2)r} T_n\$. Instead of computing each term from scratch, use this recurrence, and enjoy a significant performance boost.

Converting the summation into a Horner schema would likely improve the accuracy, that is to achieve the desired number of digits you'd need less number of terms.
Tree loops in lntwo cry to become a function.


Obj-C wrapper for OpenLDAP This section of code reside in a utility class (LDAPUtility). It's purpose is to subscribe or unsubscribe a user from an LDAP group. My main questions are if I am using the LDAP library correctly and if I am handling the memory management correctly.

+ (BOOL)addUser:(const char *)userDN toGroup:(const char *)groupDN {
    return [LDAPUtility performOperation:LDAP_MOD_ADD withUser:userDN andGroup:groupDN];
}

+ (BOOL)removeUser:(const char *)userDN fromGroup:(const char *)groupDN {
    return [LDAPUtility performOperation:LDAP_MOD_DELETE withUser:userDN andGroup:groupDN];
}

+ (BOOL)performOperation:(int)op withUser:(const char *)userDN andGroup:(const char *)groupDN {
    LDAPMod **mods;
    LDAPMod mod;

    int err;

    mod.mod_op = op;
    mod.mod_type = "member";
    mod.mod_vals.modv_strvals = malloc(sizeof(char *));
    mod.mod_vals.modv_strvals[0] = (char *)userDN;
    mod.mod_vals.modv_strvals[1] = NULL;

    mods[0] = &mod;
    mods[1] = NULL;

    if ((err = ldap_modify_ext_s(ld, groupDN, mods, NULL, NULL)))
    {
        free(mod.mod_vals.modv_strvals);
        DDLogError(@"   ldap_modify_ext_s(): %s", ldap_err2string(err));
        return false;
    }

    free(mod.mod_vals.modv_strvals);
    return true;
}

And the layer of code above this (which is not what I am submitting for a review, but may help contextualize the code above):

+ (void)subscribeToGroup:(Group *)group completion:(void (^)(BOOL success, NSError *error))completion {
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul);
    dispatch_async(queue, ^{
        NSError *error = nil;
        BOOL success = false;

        if (![Authenticator currentUser]) {
            // Go away, you're drunk.
            error = [NSError errorWithDomain:@"Not Authenticated" code:401 userInfo:@{NSLocalizedDescriptionKey:@"User has not authenticated."}];
        } else if (!group) {
            error = [NSError errorWithDomain:@"Missing Parameter" code:400 userInfo:@{NSLocalizedDescriptionKey:@"The group must be specified."}];
        } else {
            const char *userDN = [[Authenticator currentUser].distinguishedName UTF8String];
            const char *groupDN = [group.distinguishedName UTF8String];
            if (![LDAPUtility addUser:userDN toGroup:groupDN]) {
                error = [NSError errorWithDomain:@"LDAP Error" code:500 userInfo:@{NSLocalizedDescriptionKey:@"The LDAP server update was not successful."}];
            } else {
                success = YES;
            }
        }

        dispatch_sync(dispatch_get_main_queue(), ^{
            completion(success, error);
        });
    });
}

Answerer's note: I'm moving commentary about NSString versus C-style strings out of this original answer and into their own answer.

The strings we use in creating the error objects, as well as the integers we use for the code in these objects should be defined, named constants (and probably an enum for the integers) all put together in one common place.  This way, if we ever want to reuse any of these, that's simple.  If we ever want to rename, reword, renumber, etc., they're all located in one easy to find location no matter how messy the code in this particular section gets.

The word "and" should be used to indicate that a method performs two distinct actions (and typically should just be two separate methods anyway).  We shouldn't use the word "and" to chain our variables.

Why not make an enumeration for our operation constants?  This will make our Objective-C code slightly more clear, and if someone uses this in a Swift project, they'll really like that they get errors if trying to send an invalid argument for this.

Our enum might look like this:

typedef NS_ENUM(NSInteger, LDAPOperation) {
    LDAPOperationAddUser = LDAP_MOD_ADD,
    LDAPOperationDeleteUser = LDAP_MOD_DELETE
};

And one final note about this code, given that it's Objective-C, we should use YES and NO rather than true or false.  There's no technical difference, but the Objective-C style is to use the YES/NO defines.


Decreasing the runtime of this non-abundant sums solution I have written a solution to problem 23 of Project Euler, and after doing some testing and editing, I've managed to get the run time down from 1.7443 seconds to 1.56616 seconds. The problem is

  Find the sum of all the positive integers which cannot be written as the sum of two abundant numbers.

What are some other ways that I could improve this program? I'm pasting it entirely, and it should be noted that I'm compiling with g++ -o3.

#include <chrono>
#include <cmath>
#include <iostream>
#include <ctime>
#include <ratio>

int main()
{
//Value given in the question
const int LIMIT = 28124;
//Used to measure the runtime
std::chrono::high_resolution_clock::time_point t1 = std::chrono::high_resolution_clock::now();

//Sieve of Erasthotenes to remove a significant number of iterations in the next step
int rootMax = (int)sqrt((double)LIMIT);

bool set[LIMIT+1] = {0};

for(int i = 2; i <= rootMax; ++i)
{
    if(!set[i])
    {
        for(int j = (i*i); j <= LIMIT; j += i)
        {
            set[j] = true;
        }
    }
}

bool isSumOfAb[LIMIT] = {0};

int abundant[6965];
int count = 0;
for(int i = 12; i < LIMIT; ++i) //This can start at 12, as we know that 12 is the 1st abundant number
{
    if(!set[i]) // Prime numbers won't be abundant, so next number
    {
        continue;
    }

    int sum = 0;
    for(int j = 1; j < i; ++j)
    {
        if(i % j == 0)
        {
            sum += j;
        }
    }

    if(sum > i)
    {
        abundant[count] = i;
        ++count;
    }
}

//Lots of work to do here.
//Establish all of the sums and compare
int a, c;
for(int i = 0; i < 6965; ++i)
{
    a = abundant[i];

    for(int j = i; j < 6965; ++j)
    {
        c = a + abundant[j];
        //Big speed increase when made like this as opposed to `if(c <= LIMIT) isSumOfAb[c] = true;`
        if(c > LIMIT)
        {
            break;
        }else
        {
            isSumOfAb[c] = true;
        }
    }
}

int sum = 0;
for(int i = 24; i < LIMIT; ++i) //can start at 24, as that is the 1st sum of 2 abundant numbers
{
    if(!isSumOfAb[i])
    {
        sum += i;
    }
}

std::cout << "Sum: " <<  sum;
std::chrono::high_resolution_clock::time_point t2 = std::chrono::high_resolution_clock::now();
std::chrono::duration<double> time_span = std::chrono::duration_cast<std::chrono::duration<double>>(t2 - t1);
std::cout << "\nIt took me " << time_span.count() << " seconds.";
}

After some tests (adding more time-points), we can find that the double for-loop used to find all abundant numbers is the slowest. Now we can think about speeding it up, by shortening the inner loop. We start with sum=1 and try every number we find can divide i upto sqrt(i). We immediatelly know that the product will of course divide the number as well, but need to take special care for numbers of the form i=j*j.

//  stop is the biggest integer such that stop*stop <= i
//  the reasoning behind using it is similar to the reasoning for using it in first phase
//  when marking all primes, but we need to take care for div=i/j when i%j == 0
    int sum = 1, stop = sqrt(i);
    for(int j = 2;;)
    {
        int div = i/j;         // divison and modulo will probably be optimized
        int mod = i%j;         // as one instruction (we could use some divmod built-in)

        if(mod == 0)           // when this condition is met we know that:
        {                      // i = j*div
            if(j == div)
            {                  // but can be i = j*j when j=div
                sum += j;      // we need to sum it only once in such case (e.g. 16=4*4)
                break;         // and can exit the loop, because j=stop here
            }
            sum += j+div;      // this is the tricky part when we sum both j and div
        }                      // e.g. for 15: 1 + 3+5 (j=3, div=5, stop=3 here)
        if(++j > stop)
            break;             // moving the condition here is another optimization
    }                          // ...and could be rewritten as do-while loop

The original code needed more than 8.8s on my old PC, but the current version needs only 0.11s. Full code:

#include <chrono>
#include <iostream>
#include <algorithm>

using the_clock = std::chrono::high_resolution_clock;
using time_point = the_clock::time_point;
using time_span = std::chrono::duration<double>;

// all integers greater than 28123 can be written as the sum of two abundant numbers
static const int LIMIT = 28124;
// smallest abundant number is 12
static const int minAbundant = 12;

// all prime numbers will have false in this array
static bool not_prime[LIMIT+1]; // zero initialized
static const int maxPrimeRoot = (int)sqrt(LIMIT);
static inline bool is_prime(int x) {
    return !not_prime[x];
}

// all abundant numbers
static const int abundantSz = 6965; // we know that, see final output
static int abundant[abundantSz];

// all numbers that are sum of abundant numbers will be marked true here
static bool isSumOfAb[LIMIT];

int main()
{
//  1. Mark all primes:
    time_point t1 = the_clock::now();

    for(int i = 2; i <= maxPrimeRoot; i++)
    {
        if(is_prime(i))
        {
            for(int j = (i*i); j <= LIMIT; j += i)
                not_prime[j] = true;
        }
    }

//  2. Find all abundant numbers
    time_point t2 = the_clock::now();

    abundant[0] = minAbundant;
    int count = 1;
    for(int i = minAbundant+1; i < LIMIT; ++i)
    {
        if(is_prime(i)) // Prime numbers won't be abundant, so next number
            continue;

    //  BIG CHANGE HERE
    //  we sum both the number we find can divide i
    //  and the product + stop after sqrt(i)
    //  we need to take special care for numbers of the form i*i (e.g. 16=4*4)
        int sum = 1, stop = sqrt(i);
        for(int j = 2;;)
        {
            int div = i/j;
            int mod = i%j;
            if(mod == 0) {
                if(j == div) {
                    sum += j;
                    break; }
                sum += j+div;
            }
            if(++j > stop)
                break;
        }

        if(sum > i)
        {
            abundant[count] = i;
            ++count;
        }
    }

//  3. Mark are numbers that are sum of abundant numbers
    time_point t3 = the_clock::now();

    for(int i = 0; i < count; ++i)
    {
        int a = abundant[i];

        for(int j = i; j < count; j++)
        {
            int c = a + abundant[j];
            if(c > LIMIT)
                break;
            isSumOfAb[c] = true;
        }
    }

//  4. Sum them all
    time_point t4 = the_clock::now();

    int sum = 0;
    for(int i = 24; i < LIMIT; ++i) //can start at 24, as that is the 1st sum of 2 abundant numbers
    {
        if(!isSumOfAb[i])
            sum += i;
    }

//  Output
    time_point t5 = the_clock::now();
    using std::cout;
    cout << "Sum: " <<  sum;
    cout << "\nIt took me " << time_span(t5-t1).count() << " seconds.";
    cout << "\nTimes: " << time_span(t2 - t1).count()
        << ", " << time_span(t3 - t2).count()
        << ", " << time_span(t4 - t3).count()
        << ", " << time_span(t5 - t4).count();
    cout << "\nAbundant count: " << count; // just a check

    if(sum != 4179595)
        cout << "\nERROR - bad sum\n";
}

You can see I have moved few variables outside main() and added few using to reduce the code a bit. But I am really not the one to talk about code-styling :D


A method to get a document's path I have a model called Document, in this I have a method that I use throughout my website to get the full path to the specific document:

public function getDocument($documentName = true)
{
    $documentName = $documentName ? $this->name : null;

    return storage_path('app/' . $this->getPath() . $documentName);
}

public function getPath()
{
    return "{$this->stream->token}/{$this->unique_id}/";
}

I use it like this (example):

$documentPath = $this->document->getDocument();

However, as you can see, the document path is made up of:

$this->stream->token/$this->unique_id/

Where $this->stream->token is calling a relationship in my Document model.

public function stream()
{
    return $this->belongsTo(Stream::class);
}

Now the method works - I can use the getDocument() to get the full path to the actual file (or just the folder, if I pass in false).

However, this code is called from multiple places at different times - and because the folder structure is dependent on the stream_token, it makes a query to get the associated stream on each request.

Solution(s)?

I have tried adding a $with = ['stream'] property to my Document model, so  the associated stream is always loaded when fetching the model - but this I don't believe is a good option either, since then all the other queries I have for the document model, that don't care about the stream property make an extra query.

What else can I do to improve the method and keep the # of queries to the database to a minimum?

I'm sorry, this is not an answer to your question, but it is a partial review of your code.

It seems you've got a serious problem with choosing your names. They don't actually tell me what the purpose of the methods is, or the content of the variables.

Your method getDocument() can do two things: Return the path or the folder of the document. The name doesn't tell me that. It should be something like getDocumentPathOrFolder(). The argument $documentName doesn't contain a document name. It's a boolean. The only way to find out, what really is contained in this argument, is to read the code in the method. It seems to decide whether a path or folder should be returned. A better name would have been $addNameToFolder. Still not ideal.

To make matters worse you have a method called getPath() which, you've guessed it, doesn't return a path. By now I no idea what is actually returned by this method.

If I'm allowed to completely rewrite your code, with a better structure and better names, it would look something like this:

class Document
{

    public function getDocumentName()
    {
        return $this->documentName;
    }

    public function getDocumentFolder()
    {
        return storage_path("app/{$this->stream->token}/{$this->unique_id}/");
    }

    public function getDocumentPath()
    {
        return $this->getDocumentFolder() . $this->getDocumentName();
    }

}

I assume that storage_path() is part of Laravel, and cannot be changed. The long argument to storage_path() should probably be split into its seperate parts, especially if you also use it elsewhere, but please, spent some time to choose good names.

References:

Ten tips to help you choose good names

Writing Good Method & Variable Names

The importance of naming in programming


Find a n x m binary matrix with exactly a 1's in all rows and b 1's in all columns [Base on https://codeforces.com/contest/1360/problem/G]

So my logic is to process each column at a time and find b rows such that the total 1s in them is less than a and if there are many candidates I take the rows with least 1s.

import           Data.List
import           Data.Array
import           Control.Monad
-- import           Debug.Trace

-- debug = flip trace

main :: IO ()
main = do
    t <- read <$> getLine -- testcases
    replicateM_ t $ do
        n : m : a : b : _ <- (map read . words <$> getLine) :: IO [Int]
        mapM putStrLn $ case solve n m a b of
            Just mat ->
                "YES"
                    : [ concatMap show [ mat ! (i, j) | j <- [1 .. m] ]
                      | i <- [1 .. n]
                      ]
            Nothing -> ["NO"]

-- solve :: Int -> Int -> Int -> Int -> Maybe (Array (Int, Int) Int)
solve n m a b = go
    (array (1, n) . zip [1 .. n] $ repeat 0) -- rowcnts
    (array ((1, 1), (n, m)) [ ((i, j), 0) | i <- [1 .. n], j <- [1 .. m] ]) -- matrix
    1 -- column
  where
    go rowcnts mat c
        | length idxs /= b = Nothing -- not enough rows
        | c == m = if all (== a) (elems rowcnts') then Just mat' else Nothing -- last column to process
        | otherwise = go rowcnts' mat' (c + 1) -- recurse for next column
      where
        idxs = take b . filter (\i -> (rowcnts ! i) < a) $ sortOn
            (rowcnts !)
            [1 .. n] -- candidate rows
        rowcnts' = accum (+) rowcnts . zip idxs $ repeat 1 --`debug` show (elems rowcnts)
        mat'     = mat // [ ((idx, c), 1) | idx <- idxs ]

I wish to ask how can I write better code which might be concise and cleaner.

You don't need to collect the rows in a matrix because you only touch each row once, and you don't need Data.Array because you only ever process a whole row. The check at the end makes the checks in the middle functionally superfluous. mapAccumL captures the explicit recursion.

main :: IO ()
main = do
  t <- readLn
  replicateM_ t $ do
    [n,m,a,b] <- map read . words <$> getLine
    mapM putStrLn $ case solve n m a of
      (colcnts, rows) | all (==b) colcnts -> "YES" : map (concatMap show) rows
      _ -> ["NO"]

solve :: Int -> Int -> Int -> ([Int], [[Int]])
solve n m a = mapAccumL go (replicate m 0) [1..n] where
  go colcnts c = (zipWith (+) row colcnts, row) where row
    = map fst $ sortOn snd $ zip (replicate a 1 ++ repeat 0)
    $ map fst $ sortOn snd $ zip [1..] colcnts

Note that your entire approach doesn't obviously find a solution if there is one. Can you prove that?

Edit: I thought you process each row at a time. Easy to mix up since the approaches are symmetric :). I choose the row-at-a-time approach, since they can be directly printed.


Playing different audio files based on video keyframe The general idea of this function is to load a different sound file depending on current time position of the video. I have the following information about audio files:

var samples: [
['00:00:05', 'audio/1.mp3'],
['00:01:00', 'audio/2.mp3']
]

The first element contains the end time for specific audio sample, and the second is the sample. I think we don't need the start time, because the following condition should return a good result:

if( video.currentTime <= endTime ) { ... }

Anyway, here is the code of function I'm using to check which audio file to play (it's executed when the video is paused):

var APP.playAudio = function() {

    var pAudio = samples; // the array above

            for(var i = 0; i < pAudio.length; i++ ) {

                if( video.currentTime < hms_to_s(pAudio[i][0]) ) {
                    pCurrentAudio = pAudio[i][1];
                    break;
                }

            }
}

I'm looking for more tricky and clever way to achieve this. As you see, now we iterate over the pAudio array each time the video is paused.

You want to play a certain audio at a certain time? Well, you're in luck. I recently did this in a demo. It wasn't playing audio with video though. It was triggering animations at certain points of the audio, but it should be the same idea.

<audio> and <video> elements are HTMLMediaElements and have media events.

If you want to play your mapped audio in real time, there is the timeupdate event which is triggered when the current time of the element has changed.
If you want to play it during pause, then there is the paused event.

You can use these events to check the currentTime of your element, which in this case is your video. currentTime is in seconds. If you want to stick to your hh:mm:ss format, you'd need your own converter.

Instead of urls, you should have preloaded audio elements there so that they will play instantly when we need them.

Anyways, here's a sample structure with seconds and audio elements

var samples = [
  [5,HTMLAudioElement],
  [10,HTMLAudioElement],
  [15,HTMLAudioElement],
  ...
];

Then, hook to the timeupdate event of the video, check the current time, and compare with the indicated item in the array. You can use a counter to indicate your current position.

var i = 0
//get first audio as current audio and play
var currentAudio = samples[i][1].play();

video.addEventListener('timeupdate',function(){

  //if video time is greater than current audio end time
  if(this.currentTime > samples[i][0]){
    if(currentAudio) currentAudio.pause(); //pause existing audio (maybe do a "seek to zero" to reset)
    currentAudio = samples[++i][1];        //reference next audio as current
    currentAudio.play();                   //play the current audio
  }
});


A simple mastermind clone I made a simple Mastermind clone and I'd just like some tips on what I could do better/different solutions for what I have already coded. If you're wondering what mastermind is, there are, for the original, 6 different colors and 4 different holes. I decided to make this since I had made mastermind in other things so I thought it would be a good starter project in C#.

static void Main(string[] args)
    {
        Random GenRandom = new Random();
        int t = 0, r, c1 = GenRandom.Next(1, 6), c2 = GenRandom.Next(1, 6), c3 = GenRandom.Next(1, 6), c4 = GenRandom.Next(1, 6);
        bool w = false;
        string Input, Code = Convert.ToString(c1); Code += c2; Code += c3; Code += c4;
        while (t != 8) {
            t++;
        Unepic:
            Console.Clear();
            Console.WriteLine("You have {0} turn(s) left.",9-t);
            Console.WriteLine("Guess the code E.g 1561: ");
            Input = Console.ReadLine();
            if (Input.Length != 4) goto Unepic; // Checks if input is 4 characters long
            try { Convert.ToInt16(Input); Convert.ToString(Input); } catch (FormatException) { goto Unepic; } // Checks if input is only numbers
            if (Input == Code) { w = true; goto End; }; // Checks if you've won
            if (Input.Contains("0") || Input.Contains("7") || Input.Contains("8") || Input.Contains("9")) { goto Unepic; } // Checks if it has any digits that are 0 or above 7
            r = -1;
            while (r != 3)
            {
                r++;
                if (Input[r] == Code[r]) Console.Write(1); else Console.Write(0); // Checks if a digit of the input is equal to a digit of the code
            }
            Console.WriteLine();
            Console.Write("Press any key to continue.");
            Console.ReadKey(true);
        }
        End:;
        Console.Clear();
        if (w == true) { Console.WriteLine("You won! The code you guessed was {0}.", Code); } else { Console.WriteLine("You lost! The code you couldnt guess was {0}.",Code); };
        Console.ReadKey(true);

This is sophisticated for a first attempt, but today would be a great day to break yourself of bad habits.  Your code is full of them.

Start with:  format your code using standard formatting conventions.  We understand code more easily if it is vertical, and your code is horizontal.

Let's go through this line by line.

static void Main(string[] args)
{

Programs that do everything in Main are programs that cannot be easily refactored or re-used.  Main should create an object that represents the program and then execute it.

    Random GenRandom = new Random();

Follow C# naming conventions. Locals get camelCase names.  This should be

    Random random = new Random();

    int t = 0, r, c1 = random.Next(1, 6), c2 = random.Next(1, 6), c3 = random.Next(1, 6), c4 = random.Next(1, 6);

Though this style is legal, it's considered poor style.  Declare and initialize variables one per line.  Give your variables meaningful names.  You will not die younger if you spend time typing turn instead of t.  This should be:

    int turn = 0;
    int digit;
    int cell1 = random.Next(1, 6);
    int cell2 = random.Next(1, 6);
    int cell3 = random.Next(1, 6);
    int cell4 = random.Next(1, 6);

Moving on:

    bool w = false;

Same deal:

    bool hasWon = false;

Moving on:

    string Input, Code = Convert.ToString(c1);
    Code += c2;
    Code += c3;
    Code += c4;

Again, one per line.

We now see that we have the same data in two places: the locals c1, c2, c3, c4 which are never used again and Code.  It is time for our first refactoring.  What we want is:

string code = GenerateCode();

Let's implement that:

static Random random = new Random();
static string GenerateCode()
{
  return random.Next(1, 6).ToString() +
    random.Next(1, 6).ToString()
    random.Next(1, 6).ToString()
    random.Next(1, 6).ToString();
}

And we remove random, c1, and so on, from our method.

All right, where were we? We now have:

    string input;
    string code = GenerateCode();

Moving on:

    while (t != 8) {

First, bad form in the disco brace style.  Second, eight?  Where did eight come from? Rewrite it:

const int maximumTurns = 8;
while (turn != maximumTurns)
{

Moving on:

  t++;

I find increment operators distasteful.

  turn += 1;

Much nicer.

    Unepic:

OH THE PAIN.  Do not write unstructured code in C#; it is 2018, not 1972.

        Console.Clear();

The first line I don't object to! :-)  But as we'll see, it should be in a method of its own.

        Console.WriteLine("You have {0} turn(s) left.",9-t);

Better would be:

        Console.WriteLine($"You have {maximumTurns + 1 - turn} turn(s) left.");

Because now if you change maximumTurns to 7, you only have to change the code in one place, not many.

Better still, make a helper function:

static string Plural(int x, string s)
{
  return x == 1 ? s : s + "s";
}

and then

        int remainingTurns = maximumTurns - 1 - turn;
        Console.WriteLine($"You have {remainingTurns} {Plural(remainingTurns, "turn")} left.");

And then move that into a helper.

Moving on:

        Console.WriteLine("Guess the code E.g 1561: ");

Eschew Latinisms. Should be:

        Console.WriteLine("Guess the code (for example, 1561): ");

Now we come into a section that demands refactoring:

        input = Console.ReadLine();

Fine so far...

        if (input.Length != 4) goto Unepic; // Checks if input is 4 characters long

Do not write comments that repeat what is plainly evident in the code. And do not use spaghetti logic.

        try { Convert.ToInt16(Input); Convert.ToString(Input); } catch (FormatException) { goto Unepic; } // Checks if input is only numbers

Do not re-invent the wheel; TryParse already exists:

    int number; // In C# 7 you can use a discard instead.
    bool valid = int.TryParse(input, out number);

How should we structure this whole thing better? What we want is:

    if (!ValidInput(input)) goto Unepic;

So write that:

static bool ValidateInput(string input)
{
  int number;
  return input.Length == 4 && int.TryParse(input, out number);
}

Moving on:

        if (Input == Code) { w = true; goto End; }; // Checks if you've won

There's a feral semi at the end there. Remove it.

Again, when you are writing comments that are obvious, ask yourself "how could I rewrite the code so that the comment was unnecessary?"  Like this:

        if (input == code)
        {
          hasWon = true;
          goto End;
        }

We'll address the goto later.

        if (Input.Contains("0") || Input.Contains("7") || Input.Contains("8") || Input.Contains("9")) { goto Unepic; } // Checks if it has any digits that are 0 or above 7

Why is this check here? It should be in ValidateInput.  Put it there!

We then have a little loop:

        r = -1;
        while (r != 3)
        {
            r++;
            if (Input[r] == Code[r]) Console.Write(1); else Console.Write(0); // Checks if a digit of the input is equal to a digit of the code
        }

Again, put it in a method.

static void WriteHints(string input, string code)
{

The normal way we would write this loop is:

  for (int i = 0; i < input.Length; i += 1)
    Console.Write(input[i] == code[i] ? 1 : 0;
}

Finishing up the game loop:

        Console.WriteLine();
        Console.Write("Press any key to continue.");
        Console.ReadKey(true);

Why make the user press a key here?  But again, it could be put into a helper method.

    }
    End:;
    Console.Clear();

Again, I'm fine with this but we'll come back to the goto later.

    if (w == true) { Console.WriteLine("You won! The code you guessed was {0}.", Code); } else { Console.WriteLine("You lost! The code you couldnt guess was {0}.",Code); };

Writing if (w == true) is a sure sign of newbie code.  w is already either true or false; you don't have to say "if it is true that w is true" and you don't have to say "if w is true" -- just say "if w".

And again, this could go into a helper.

Now, let's look at your main loop as I have refactored it, all together

static void Main(string[] args)
{
    int turn = 0;
    bool hasWon = false;
    string code = GenerateCode();
    string input;
    const int maximumTurns = 8;
    while (turn != maximumTurns)
    {
      turn += 1;
    Unepic:
      PromptUserForInput(maximumTurns, turn);
      input = Console.ReadLine();
      if (!ValidInput(input)) goto Unepic;
      if (input == code)
      {
        hasWon = true;
        goto End;
      }
      WriteHints(input, code);
      PromptUserToPressKey();
    }
    End:;
    PrintWinOrLoseMessage(hasWon, code);
}

This is already one million times easier to read but we are not done.

The first thing we notice is that we do some work to ensure that turn is not incremented if the user did a bad input. That's great! But the code does not read like that intent.  Let's rewrite it so it does.

    int turn = 1; // not zero!
    while (turn <= maximumTurns) // not !=
    {
    Unepic:
      PromptUserForInput(maximumTurns, turn);
      input = Console.ReadLine();
      if (!ValidInput(input)) goto Unepic;
      if (input == code)
      {
        hasWon = true;
        goto End;
      }
      WriteHints(input, code);
      PromptUserToPressKey();
      turn += 1;
    }

Now turn is only ever incremented after the user has put in a good guess, and the code doesn't need to initialize turn to the wrong value so that it can be incremented later.

Now we notice that the goto Unepic and goto End are continue and break:

    while (turn <= maximumTurns)
    {
      PromptUserForInput(maximumTurns, turn);
      input = Console.ReadLine();
      if (!ValidInput(input))
        continue;
      if (input == code)
      {
        hasWon = true;
        break;
      }
      WriteHints(input, code);
      PromptUserToPressKey();
      turn += 1;
    }

Look at how much easier to understand my version is compared to yours.  Anyone, even a non-coder, could look at this thing and just read it like English.  "while the turn is less than or equal to the maximum turns, prompt the user for input, then read the line from the console, then validate the input..."

That is what you must strive for in your code.  Always be asking yourself how could I make this easier to understand? How could I make this read more like a description of my intentions?

Could we do better?  Sure.  We actually have two loops written as one.  There's the loop that gets valid user input, and there's the loop that runs the game.  Make that explicit:

    while (turn <= maximumTurns)
    {
      do
      {
        PromptUserForInput(maximumTurns, turn);
        input = Console.ReadLine();
      }
      while(!ValidInput(input));
      if (input == code)
      {
        hasWon = true;
        break;
      }
      WriteHints(input, code);
      PromptUserToPressKey();
      turn += 1;
    }

And now we see another opportunity for helper methods. Move the inner loop to a helper!  Move the winning check to a helper:

    while (turn <= maximumTurns)
    {
      input = ObtainValidatedInput(maximumTurns, turn);
      hasWon = CheckForWin(input, code);
      if (hasWon)
        break;
      WriteHints(input, code);
      PromptUserToPressKey();
      turn += 1;
    }

Were I writing your code from scratch, I would make it considerably more abstract than even the version I've given you here.  My main would be:

static void Main()
{
  var game = new Game();
  game.Start();
  while (!game.IsOver)
    game.ExecuteTurn();
  game.End();
}

Try writing your code starting from this template.  Notice how clear and logical it forces your code to be when you start from a position of the code reading like an abstract description of the workflow.  Make every method like this, where you can read the logical workflow right out of a half dozen lines of code.  Get in good habits while you are still a beginner.