Find the most combinations of two numbers that equal third or get close to it My task is to find how many different "tasks" can be done if I have two types of tasks that take specific time and time to complete all of them. I have to find the most tasks that can be done so that the gap between the time and sum of tasks is minimal. For example consider this input:

3 5 55

And output(2x5 + 15x3):

17

Or:

Input: 886 340 6177, Output: 10

Also:

Input: 3 5 54, Output: 18

My code is this:

 import java.util.Scanner;
 public class Protsessor {
    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        int m = in.nextInt(); //Time of first task
        int n = in.nextInt(); //Time of second task
        int t = in.nextInt(); //Time available

        int tmpgap;
        int gap = t;
        int tasks = 0;
        int t1  = t;

        if( m <= 0|| n <= 0) { //If input is 0 or less
            System.out.println(0);
            System.exit(0);
        }
        if(m > n) { //I rearrange my input as it doesn't work when m is bigger then n
            int tmp = m;
            m = n;
            n = tmp;
        }
        for (int i = 0; i < (t/m+1); i++) {
            int tmptasks = 0;
            t1 -= i*m;
            tmptasks += i;
            for (int j = 0; j < (t/n+1) && t1 >= n; j++) {
                t1 -= n;
                tmptasks++;
            }
            tmpgap = t1;
            if(tmpgap <= gap){
                gap = tmpgap;
                tasks = tmptasks;
            }
            t1 = t;

        }
        System.out.println(tasks);
    }
}

As my code is quite messy, how can I optimize/improve it using dynamic programming?

NOTE: This was a task in competetive programming lesson, I already submitted this code(passed).

Have you simply tried to split code into smaller functions?

int t = in.nextInt(); //Time available

How about simply naming it timeAvailable instead of t ?

if(m > n) { //I rearrange my input as it doesn't work when m is bigger then n
    int tmp = m;
    m = n;
    n = tmp;
}

Common pattern is something like:

int calculate(int m, int n, int t) {
    if (m > n) {
        return calculate(n, m, t);
    }
    ...
}

Try to declare variables in smallest possible scope - int tmpgap; should be inside the for-loop. The inner for-loop might be replaced by division + Math.floor().


Is there a better way to make sure a variable is initialized when using blocks? I have a method to provide an NSManagedObjectContext using UIManagedDocument. I need to ensure that the context did initialize before I return, so I added an infinite while loop at the end of the method. But I think that was very stupid and is not acceptable.

- (NSManagedObjectContext *)context {
    NSURL *url = [[[NSFileManager defaultManager] URLsForDirectory:NSDocumentDirectory inDomains:NSUserDomainMask] lastObject];
    url = [url URLByAppendingPathComponent:@"Test"];
    UIManagedDocument *document = [[UIManagedDocument alloc] initWithFileURL:url];

    if (![[NSFileManager defaultManager] fileExistsAtPath:[url path]]) {
        [document saveToURL:url forSaveOperation:UIDocumentSaveForCreating completionHandler:^(BOOL success) {
            if (success) {
                self.managedObjectContext = document.managedObjectContext;
            }
            else {
                NSLog(@"[%@ %@] FATAL Error CANNOT create DB", NSStringFromClass([self class]), NSStringFromSelector(_cmd));
                exit(1);
            }
        }];
    }
    else if (document.documentState == UIDocumentStateClosed) {
        [document openWithCompletionHandler:^(BOOL success) {
            if (success) {
                self.managedObjectContext = document.managedObjectContext;
            }
            else {
                NSLog(@"[%@ %@] FATAL Error CANNOT open DB", NSStringFromClass([self class]), NSStringFromSelector(_cmd));
                exit(1);
            }
        }];
    }
    else {
        self.managedObjectContext = document.managedObjectContext;
    }

    while (!self.managedObjectContext) {
        // Is there any way better?
        NSDate *futureTime = [NSDate dateWithTimeIntervalSinceNow:1];
        [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:futureTime];
    }

    return self.managedObjectContext;
}

Any suggestion will be appreciated.

Create and run operation using

NSOperation *op = [NSBlockOperation ... your block...];
[[NSOperationQueue new] addOperation: op];

and instead of using a runloop, wait for the result:

[op waitUntilFinished];


Generating a MySQL query I'm trying to create a function to return a DataTable with the results of a MySQL query where the program doesn't know the names of the queried columns until run-time.

How can I improve the readability of this function? Is concatenating strings like this an appropriate way to generate MySQL queries?

public static DataTable Entries(string db, string user, string dtStart, string dtEnd = "yyyy-MM-dd HH:mm:ss", List<string[]> column = null)
    {
            string mysqlTimestamp = "yyyy-MM-dd HH:mm:ss";
            string cmd = string.Format("SELECT * FROM `{0}`.`{1}_table` WHERE ", db, user);

            if (dtEnd != mysqlTimestamp)
            {
                cmd += "`Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd";
            }
            else {
                cmd += "`Timestamp` = @dtStart";
            }

            if (column != null)
            {
                cmd += " AND";

                string colQuery = " `{0}` = @{0}";
                for (int i = 0; i < column.Count(); i++)
                {
                    cmd += string.Format(colQuery, column[i]);
                    if (i != column.Count() - 1) { cmd += " AND"; }
                }
            }

            var data = new DataSet();
            using (var MySql = Query.Connection(Session.Current.Username, Session.Current.Database))
            {
                MySql.Open();
                using (var MySqlCmd = new MySqlCommand(cmd, MySql))
                {
                    MySqlCmd.Parameters.AddWithValue("@dtstart", dtStart);
                    MySqlCmd.Parameters.AddWithValue("@dtEnd", dtEnd);
                    if (column != null)
                    {
                        foreach (string[] col in column)
                        {
                            MySqlCmd.Parameters.AddWithValue("@" + col[0], col[1]);
                        }
                    }

                    using (var Adapter = new MySqlDataAdapter(MySqlCmd))
                    {
                        Adapter.Fill(data);
                    }
                }
            }
            return data.Tables[0];
        }
    }

Results in the string:

SELECT * FROM `db`.`someuser_table` WHERE `Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd AND `Column1` = @Column1 AND `Column3` = @Column3

or

SELECT * FROM `db`.`someuser_table` WHERE `Timestamp` >= @dtStart

and anything in between.

To improve readability you should use proper naming conversions.
From the given code what i can understand that the input parameter user is for constructing the table name, that means all your table looks like someName_table(i'm not sure), if so user is not a good name for that variable, let me rename them as queryOnTable.
Another thing i have noticed is that Timestamp will be a common column for all your tables.(me too assume so or else we have to consider that too).
Here in your case you are specifying the database name as well as the
current user of the database in the connection using var MySql =
Query.Connection(Session.Current.Username, Session.Current.Database)
so you need not be specified them in the executable query. Or else
you can use the input parameters db and user(if the above point is false) can be used to create the connection(if connection string change dynamically). This may help you to reduce the complex formatting of the select statement.
Here the given snippet will always select the whole data ( select *
) from the given table, This will not be a good choice, always
collect the data which you want. I i prefer you to use another input
array to the function that specifies the required fields.
This article says that .Parameters.Add() is better choice that .Parameters.AddWithValue() So we can prefer that here by changing the last parameter List<string[]> column to something like List<myQueryParameter>.

where myQueryParameter is defined as:

public class myQueryParameter
    {
        public MySqlDbType  paramType { get; set; }
        public string paramValue { get; set; }
    }

I suggest you to include an additional parameter called additionalClause which can be used for specifying clauses like Group by, order by etc.
Now the query building; Many article available in the various sites stated that StringBuilder is the best option for concatenating the string when the concatenations are more than 1000, where as String.Join() is even more efficient than StringBuilder if concatenations are less than 1000. + and String.Concat() are less efficient than these two.

Now consider the modified method:

public static DataTable GetDataTable(string queryOnTable,
                                    string dtStart,
                                    string dtEnd = "yyyy-MM-dd HH:mm:ss",
                                    List<string> reqColumns = null,
                                    List<myQueryParameter> conditionalParams = null),
                                    string additionalClause="")
    {
        if (reqColumns == null) { reqColumns = new List<string>() { "*" }; } // select all columns if not specified

        string mysqlTimestamp = "yyyy-MM-dd HH:mm:ss";

        StringBuilder sqlQueryBuilder = new StringBuilder();
        sqlQueryBuilder.Append("SELECT ");
        sqlQueryBuilder.Append(String.Join(",", reqColumns));
        sqlQueryBuilder.Append(String.Format(" FROM {0} WHERE ", queryOnTable));

        if (dtEnd != mysqlTimestamp)
        {
            sqlQueryBuilder.Append("`Timestamp` >= @dtStart AND `Timestamp` <= @dtEnd");
        }
        else
        {
            sqlQueryBuilder.Append("`Timestamp` = @dtStart");
        }

        if (conditionalParams != null)
        {
            foreach (myQueryParameter param in conditionalParams)
            {
                sqlQueryBuilder.Append(String.Format(" AND `{0}` = @{0}", param.param));
            }
        }

        if (additionalClause != "")
            sqlQueryBuilder.Append(additionalClause);

        var data = new DataSet();
        using (var MySql = new MySqlConnection("Query.Connection(Session.Current.Username, Session.Current.Database)"))
        {
            MySql.Open();
            using (var MySqlCmd = new MySqlCommand(sqlQueryBuilder.ToString(), MySql))
            {
                MySqlCmd.Parameters.AddWithValue("@dtstart", dtStart);
                MySqlCmd.Parameters.AddWithValue("@dtEnd", dtEnd);
                if (conditionalParams != null)
                {
                    foreach (myQueryParameter condition in conditionalParams)
                    {
                        MySqlCmd.Parameters.Add(String.Format("@{0}",condition.param),condition.paramType).Value = condition.paramValue;
                    }
                }
                using (var Adapter = new MySqlDataAdapter(MySqlCmd))
                {
                    Adapter.Fill(data);
                }
            }
        }
        return data.Tables[0];
    }

Before going to the usage example let me define a table; Let user_data is the table having columns as follows:

Column Name  | DataType
user_id          VarChar
user_name        VarChar
int_field        Int32
text_field1       Text
text_field2       Text
text_field3       Text
Timestamp         DateTime

We can call the GetDataTable() method as like the following:

// Creating input parameters
List<string> reqcolumns = new List<string>() { "user_name", "int_field", "text_field1", "text_field2","text_field3" };
List<myQueryParameter> parameters = new List<myQueryParameter>();
parameters.Add(new myQueryParameter() { param = "user_id", paramType = MySqlDbType.VarChar, paramValue = "USR001" });
parameters.Add(new myQueryParameter() { param = "int_field", paramType = MySqlDbType.Int32, paramValue = "12" });
parameters.Add(new myQueryParameter() { param = "user_name", paramType = MySqlDbType.VarChar, paramValue = "un-luckey" });

//Calling the method
var dt = GetDataTable("user_data",
                      "2014-02-15 00:00:00",
                      "yyyy-MM-dd HH:mm:ss",
                      reqcolumns, parameters,
                      " Order by Timestamp");

Which will creates the query as like the following:

  SELECT user_name,int_field,text_field1,text_field2,text_field3 FROM
  user_data WHERE Timestamp = @dtStart AND user_id = @user_id AND
  int_field = @int_field AND user_name = @user_name Order by Timestamp


Ruby if block refactor from new synthax needed I have this, I guess, new Ruby syntax in my method but Rubocop are warning me that the second last line is too long. Could you please help me to refactor this method?

def show
  identification_document = IdentificationDocument.find(params[:id])
  authorize identification_document
  return unless identification_document

  #this line below is to too long
  document = params[:size] == 'resized' ? identification_document.id_document_resized : identification_document.id_document
  send_data(document.file.read, filename: identification_document.file_name)
end

Guidelines Rubocop

The guidelines favor if/case over multi-line ternary operator when a line is too long.

  Maximum Line Length   Limit lines to 80 characters.
  No Multi-line Ternary   Avoid multi-line ?: (the ternary operator); use if/unless instead.
  Use if/case Returns   Leverage the fact that if and case are expressions which return a result.

Refactored Code

def show
  identification_document = IdentificationDocument.find(params[:id])
  authorize identification_document
  return unless identification_document

  document =
    if params[:size] == 'resized'
       identification_document.id_document_resized
    else
       identification_document.id_document
    end

  send_data(document.file.read, filename: identification_document.file_name)
end


Bubble sort in Python I am quite new in Python and I am starting my journey with sorting algorithms, PEP8 and the Zen of Python. I would like to get some feedback about my BubbleSort algorithm which I wrote.
Also, is there a possibility to change the break statement to something else which will stop the loop?

def bubble_sort(structure):
    """
        Bubble sort.

        Description
        ----------
        Performance cases:
        Worst      : O(n^2)
        Average    : O(n^2)
        Best case  : O(n)

        Parameters
        ----------
        structure : Mutable structure with comparable objects.

        Returns
        -------
        structure : return sorted structure.

        Examples
        ----------
        >>> bubble_sort([7,1,2,6,4,2,3])
        [1, 2, 2, 3, 4, 6, 7]

        >>> bubble_sort(['a', 'c', 'b'])
        ['a', 'b', 'c']

    """

    length = len(structure)

    while True:
        changed = False
        for i in range(length - 1):
            if structure[i] > structure[i + 1]:
                structure[i], structure[i + 1] = structure[i + 1], structure[i]
                changed = True
        if not changed:
            break
    return structure

Your code looks nice and the documentation is high quality.

Maybe a few things can be improved anyway.

API

It could be a good idea to get inspiration from the Python API functions related to sorting, we have:

sorted returning a new list from an iterable
list.sort sorts a list in place, returns None

In your case, as structure is sorted in place, I am not sure it makes sense to return it.

Parameter structure

This is mostly personal opinion but instead of structure, I'd find container to be a better name.

Also, if you want to be precise in the documentation, you might want to say that it is mutable but also that it implements __len__, __getitem__ and __setitem__. These objects are also called (mutable) sequences.

The code

Not much to say about the code itself.

Instead of having if not changed: break, you could write a: while changed loop which is very slightly more concise.

Also, you'll find an optimisation suggestion on Wikipedia:

  The bubble sort algorithm can be easily optimized by observing that the n-th pass finds the n-th largest element and puts it into its final place. So, the inner loop can avoid looking at the last n − 1 items when running for the n-th time

Before getting into such a change, I highly recommend writing unit tests.


Using Python to remove blank lines and comments in Assembly code I made this little script to help a friend. He studies computer engineering and is doing a project where he has to make assembly files with comments and some formatting for presentation, but, as he is making a compiler. He explained to me that he needs all their assembly codes without spaces, blank lines or comments and that the other people from his group couldn't find a small and fast solution yet to convert that.

I came up with a simple solution to it using Python:

The script gets the filename or path
Opens it and creates an output file (can be changed to another input for the user to type the output file name)
Then it iterates all the lines from the file and for each line it does the following:
Split the line when it has a comment that is identified by //
Then it gets the first element of the split line which is everything that comes before any comment
Now it replaces every space " " with "". This will get rid of all blank spaces.
If the line had a script the end of the line will be at the second element of the list, this means that the first part will not have a "\n" at the end, do we test if the list has more than one element and if it has we add a "\n" at the end.
I then used rstrip() function to check if the line is not empty, and, if it isn't we write it on the output file.

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

# Made by Iago Sousa.
# fciagsousa@gmail.com
# https://github.com/iagsousa/python/blob/master/scripts/removerComentariosAssembly.py

f = raw_input("Insert file name or path: ")
finput = open(f, "r")
foutput = open("saida.txt", "w")
for line in finput:
    l = line.split("//")
    l[0] = l[0].replace(" ", "")
    if len(l) > 1:
        l[0] += "\n"
    if l[0].rstrip():
        foutput.write(l[0])

Input example:

Output example:

Are there any steps that I could've taken to improve my code? Is is optimal for what's intended? Those are my only questions.

I have a few recommendations for your code.

First I noticed that you do not close neither the input nor the output file. This can lead to issues if you or someone else wants to access the files after your script was run. Adding a simple ''finput.close()'' and analogous for the foutput would basically "fix" the issue. However it is considered best practice to use a context manager for accessing resources like files. A context manager is able to open the file for you and close it if you leave the context no matter if you leave it by intent or because of an exception. Using a context manager could look like the following:

with open(input_path) as finput:
    for line in finput:
        print(line)

Another point is how you strip off line endings and what you store in the variables. You say you want to get rid of all comments but nonetheless you are storing them in a variable as well as I would use the strip command before the check if you need to add a new line char. The strip will also remove new line chars and therefore convert empty lines to empty strings one can easily test for. All in all I would write it the following way:

new_line = line.split("//")[0].replace(" ","").rstrip()
if new_line:
    foutput.write(new_line + "\n")

I hope that helps even though I did not add links to the mentioned methods and best practices.


Speed up search/replace in VBA Word I have a word document with 450 pages (and growing) in witch I make a search and replace (linking referencies) after inserting new text. Unfortunately this procedure is very slow.

Have you any suggestions to speed up this search/replace-function? Eg. by defining the range for the funtion to only new added text since last saving the doc (no idea if that would be possible)?

Sub AddLinks_OG()

Dim Rng As Range, SearchString As String, Id As String, PathDatenServer As String, PathArchiv As String, LinkDatenServer As String, LinkArchiv As String

Selection.Find.ClearFormatting
Selection.Find.Replacement.ClearFormatting

Set Rng = ActiveDocument.Range

SearchString = "([A-Z]{2}[0-9]{6})"
PathDatenServer = "\\Srvdat02\OGZ\Geschäft\S3\"
PathArchiv = "\\Srvarc01\OGZ\Geschäft\S3\"

    With Rng.Find
    .MatchWildcards = True
        Do While .Execute(findText:=SearchString, Forward:=False) = True
            Id = Rng.Text
            LinkDatenServer = PathDatenServer & Id & "-O\U.doc"
            LinkArchiv = PathArchiv & Id & "-O\U.doc"

            ' If file exists in local filesystem link to that
            If Dir(LinkDatenServer) <> "" Then
                ActiveDocument.Hyperlinks.Add Anchor:=Rng, _
                    Address:=LinkDatenServer, _
                    SubAddress:="", TextToDisplay:=Rng.Text
                Rng.Collapse wdCollapseStart

            ElseIf Dir(LinkArchiv) <> "" Then
                ' If file exists in archive link to archive
                ActiveDocument.Hyperlinks.Add Anchor:=Rng, _
                    Address:=LinkArchiv, _
                    SubAddress:="", TextToDisplay:=Rng.Text
                Rng.Collapse wdCollapseStart
            End If

        Loop
    End With

End Sub

One thing that is undoubtedly slowing you down is that you are reprocessing found text that is already hyperlinked. You should check that Rng.Text isn't already hyperlinked before you do all that work.

Do While .Execute(findText:=SearchString, Forward:=False) = True
    If Rng.Hyperlinks.Count = 0 Then
        'do your processing here
    End If
Loop

Although that will still perform the search on all of your previous text even if it skips processing them.

If you know that your unlinked text will always and only be at the end of the document, you could use a Boolean flag to stop searching once you hit found text that is already hyperlinked. Something like this:

Dim newText As Boolean
newText = True

With Rng.Find
    .MatchWildcards = True
    Do While newText
        If .Execute(findText:=SearchString, Forward:=False) Then
            newText = (Rng.Hyperlinks.Count = 0)
        Else
            newText = False
        End If

        If newText Then
            Id = Rng.Text
            'add your hyperlinks here
        End If
    Loop
End With

That should speed things up even more.


A cartesian product of tuples in C++17 I would like to write a function that computes a cartesian product of two tuples in C++17 (the tuples can be of type std::tuple or std::pair, or any other type that can be used with std::apply). The tuples can be of different types and sizes, and the resulting std::tuple has to contain all std::pairs of elements, where the first element of a pair is from the first tuple, and the second element is from the second tuple. The function must be suitable for use in a constexpr context.

My approach is to use std::apply to effectively convert a tuple to a parameter pack, and then to recursively apply a generic lambda to it to build the result. Because a lambda cannot be explicitly recursive, I pass a reference to it to itself as an additional parameter self.

#include <functional>
#include <tuple>
#include <utility>

template<class X, class Y>
constexpr auto cartesian_product(const X& tx, const Y& ty) {
    if constexpr (std::tuple_size_v<X> == 0) return std::make_tuple();
    else return std::apply([&](const auto& ... ys) {
        return std::apply([&](const auto& ... xs) {
            const auto recursive = [&](
                const auto& self,
                const auto& arg, const auto& ... args)
            {
                if constexpr (sizeof...(args) == 0)
                    return std::make_tuple(std::make_pair(arg, ys)...);
                else return std::tuple_cat(
                        std::make_tuple(std::make_pair(arg, ys)...),
                        self(self, args...));
            };
            return recursive(recursive, xs...);
        }, tx);
    }, ty);
}

// Test
constexpr auto x = std::make_tuple('a', 2);
constexpr auto y = std::make_tuple(true, std::optional<long>{});
constexpr auto result = cartesian_product(x, y);

static_assert(result == std::make_tuple(
    std::make_pair('a', true),
    std::make_pair('a', std::optional<long>{}),
    std::make_pair(2,   true),
    std::make_pair(2,   std::optional<long>{})));

I would appreciate any comments or suggestions how to simplify or improve this code.

Update: A generalization to an arbitrary number of factors in the cartesian product (the components of the result are now tuples rather than pairs):

#include <functional>
#include <tuple>
#include <utility>

constexpr std::tuple<std::tuple<>> cartesian_product() {
    return {{}};
}

template<class X, class ... Y>
constexpr auto cartesian_product(const X& tx, const Y& ... ty) {
    if constexpr (std::tuple_size_v<X> == 0) return std::make_tuple();
    else return std::apply([&](const auto & ... tuples) {
        return std::apply([&](const auto & ... xxs) {
            const auto recursive = [&](
                const auto& self,
                const auto& x, const auto& ... xs)
            {
                    auto tuple = std::make_tuple(
                        std::tuple_cat(std::make_tuple(x), tuples)...);
                    if constexpr (sizeof...(xs) == 0) return tuple;
                    else return std::tuple_cat(std::move(tuple), self(self, xs...));
            };
            return recursive(recursive, xxs...);
        }, tx);
    }, cartesian_product(ty...));
}

constexpr auto x = std::make_tuple('a', 2);
constexpr auto y = std::make_tuple(true, std::optional<long>{});
constexpr auto z = std::make_tuple(1L);
constexpr auto result = cartesian_product(x, y, z);
static_assert(result == std::make_tuple(
    std::make_tuple('a', true, 1L),
    std::make_tuple('a', std::optional<long>{}, 1L),
    std::make_tuple(2,   true, 1L),
    std::make_tuple(2,   std::optional<long>{}, 1L)));

I do not like std::make_tuple(x) part, but do not see a better way to prepend an element to a tuple.

You actually hint at the primary concern with your note about std::make_tuple(), std::tuple_cat() and gradually building up the result.
The point is that you have much copying with a plethora of temporaries going on.

Also, quite a bit of recursion.

Use index_sequences, the power of template parameter pack expansion, and forwarding using std::forward_as_tuple() and you can flatten it, as well as only copy directly from the source to the destination without intermediate steps.

While you are at it, ponder noexcept.

#include <utility>
#include <tuple>

namespace detail {
    template <class T>
    constexpr std::size_t tuple_size_v = std::tuple_size_v<std::decay_t<T>>;

    template <std::size_t I, class T, std::size_t... N>
    static constexpr auto index_helper(std::index_sequence<N...>) noexcept {
        return (1 * ... * (I < N ? tuple_size_v<std::tuple_element_t<N, T>> : 1));
    }
    template <std::size_t N, std::size_t I, class T>
    static constexpr auto index() noexcept {
        return N
            / index_helper<I, T>(std::make_index_sequence<tuple_size_v<T>>())
            % tuple_size_v<std::tuple_element_t<I, T>>;
    }

    template <std::size_t N, class T, std::size_t... I>
    static constexpr auto cartesian_product(T t, std::index_sequence<I...>) noexcept {
        return std::forward_as_tuple(std::get<index<N, I, T>()>(std::get<I>(t))...);
    }
    template <class T, std::size_t... N>
    static constexpr auto cartesian_product(T t, std::index_sequence<N...>) noexcept {
        return std::make_tuple(cartesian_product<N>(t, std::make_index_sequence<tuple_size_v<T>>())...);
    }

    template <class T>
    auto tuple_no_ref(T t)
    { return std::apply([](auto&&... x){ return std::make_tuple(x...); }, t); }
    template <class T>
    auto tuple2_no_ref(T t)
    { return std::apply([](auto&&... x){ return std::make_tuple(tuple_no_ref(x)...); }, t); }
}

template <class... T>
constexpr auto cartesian_product_ref(T&&... t) noexcept {
    constexpr auto N = sizeof...(T) ? (1 * ... * detail::tuple_size_v<T>) : 0;
    return detail::cartesian_product(std::forward_as_tuple(t...), std::make_index_sequence<N>());
}

template <class... T>
constexpr auto cartesian_product(T&&... t)
noexcept(noexcept(decltype(detail::tuple2_no_ref(cartesian_product_ref(t...)))(cartesian_product_ref(t...))))
{
    auto r = cartesian_product_ref(t...);
    using R = decltype(detail::tuple2_no_ref(r));
    return R(r);
}


Simple imperative doubly linked list I've started learning C++ and still making my first program. This is a continuation of my last post.

My original aim was to make a linked list at a professional level. But have now changed it to a doubly linked list, to simplify the code. It implements a couple of public functions that should increase the usability of the list significantly.

I also tried to implement an internal iterator. But didn't see a benefit to making a bidirectional iterator, for internal use, to replace getNode.

Whilst I think I improved all problems mentioned in my last post. I mostly focused on implementing the sentinel. I don't think it's a sentinel, as it's doing more than just being my end condition. And so I'd love some feedback on my implementation. But any and all critiques are wanted.

#include <iostream>

template <class T>
class LinkedList {
private:
    class Node {
    public:
        Node* next;
        Node* prev;
        T data;
        // For the sentinal
        Node(): data('\0'), prev(this), next(this) {}
        Node(T data, Node* before)
                : data(data) {
            prev = before;
            next = before->next;
            prev->next = this;
            next->prev = this;
        }
        ~Node() {
            prev->next = next;
            next->prev = prev;
        }
        Node(Node const&)            = delete;
        Node& operator=(Node const&) = delete;
    };
    Node sentinal;
    Node* getNode(int index) {
        if (index >=  0) {
            int i = 0;
            for (Node* node = sentinal.next; node != &sentinal; node = node->next) {
                if (i == index) {
                    return node;
                }
                i++;
            }
        } else {
            int i = -1;
            for (Node* node = sentinal.prev; node != &sentinal; node = node->prev) {
                if (i == index) {
                    return node;
                }
                i--;
            }
        }
        throw std::out_of_range("List doesn't contain that item.");
    }
public:
    LinkedList() {}
    ~LinkedList() {
        clear();
    }
    LinkedList(LinkedList const&)            = delete;
    LinkedList& operator=(LinkedList const&) = delete;

    void clear() {
        while (sentinal.next != &sentinal) {
            delete sentinal.next;
        }
    }

    int length() {
        int length = 0;
        for (Node* node = sentinal.next; node != &sentinal; node = node->next) {
            length += 1;
        }
        return length;
    }

    void append(T data) {
        new Node(data, sentinal.prev);
    }

    T get(int index) {
        return getNode(index)->data;
    }

    void insert(T data, int index) {
        new Node(data, getNode(index)->prev);
    }

    T pop(int index=-1) {
        Node* node = getNode(index);
        T data = node->data;
        delete node;
        return data;
    }
};

int main() {
    LinkedList<char> list;
    list.append('h');
    list.append('l');
    list.append('l');
    list.append('l');
    list.append('o');
    list.insert('e', 1);
    std::cout << list.pop(2) << list.get(-1) << '\n';
    for (int i=0; i < list.length(); i++) {
        std::cout << list.get(i);
    }
    std::cout << '\n';
    list.clear();
    std::cout << list.length() << '\n';
}

getNode does not replace an iterator. It always starts at the beginning of the list. It means that scanning over the list has a quadratic time complexity.
A Node default constructor is dubious. It assumes that T can be constructed from a NUL character. It seems like an unnecessary restriction. Assuming that T itself has a default constructor, and using it is more logical.
length() method is expected to have a constant time complexity. A linear one is at best surprising. I recommend to make a member, to be incremented at each insertion, and decremented on each removal.


National Insurance Number Validation I'm a beginner to PHP and I've written a function that validates a U.K National Insurance Number. The function works by checking whether the given number against the format below;

  The National Insurance Number ($NINO) is made up of 3 components in the
  format AAnnnnnnA. The 3 components are the prefix, numeric body and
  suffix.

  Prefix

  The first two alpha characters, validated as follows:

  • The prefix must not be BG, GB, NK, KN, TN, NT or ZZ

  • The prefix must not contain D F I Q U or V

  • The second character of the NINo prefix must not be O

  Numeric body

  The 8 numeric characters of the NINO, validated as follows:

  • Must be numeric in the range 000000 – 999999

  Suffix

  The final alpha character; validated as follows:

  • Must be in the range A – D

<?php

function validateNI($NINO) {
    $NILength = strlen($NINO);

    if (!($NILength < 10 | $NILength > 10)) {
        $NIParts = str_split($NINO, 2);

        if(!(preg_match("/[^!@#$%^&*]*(BG|GB|NK|TN|NT|ZZ|D|F|I|Q|U|V)[^!@#$%^&*]*/i", $NIParts[0]))) {

            $NIPartsSplit = str_split($NIParts[0]);

            if(preg_match("/[^!@#$%^&*]*(O)[^!@#$%^&*]*/i", $NIPartsSplit[1])) {
                echo "Invalid National Insurance Number.";
            } else {
                $NIPartsNum = $NIParts[1].$NIParts[2].$NIParts[3];

                if (is_numeric($NIPartsNum)) {

                    if(preg_match("/[^!@#$%^&*]*(A|B|C|D)[^!@#$%^&*]*/i", $NIParts[4])) {
                        echo "Valid National Insurance Number.";
                    } else {
                        echo "Invalid National Insurance Number.";
                    }
                } else {
                    echo "Invalid National Insurance Number.";
                }
            }

        } else {
            echo "Invalid National Insurance Number.";
        }

    } else {
        echo "Invalid National Insurance Number.";
    }
}

validateNI("AA1220091A");

?>

I'm looking for advice on how I can reduce the overall lines of code (including the use of regex) as well as increasing the efficiency and simplicity of my code.

I am happy for you, that you are asking for help with refactoring this scary function.
First of all, you must not write more than 2 indentations with if statements, if you want to be a great developer.

Secondly, when you write a program, keep in your mind that every method or function must do one simple task.

Well, what is the responsibility of your function?

check insurance number Length.
check prefix.
check the second string of prefix.
check suffix.
check the numeric body number.

Perfect, why must we create a class instead of keeping your method?

There is one principle, it is KISS(Keep it simple stupid),
the main idea behind this principle is to keep your methods or classes as simple and stupid as possible.

For that, we should create new methods for our responsibilities above. For example to check the first responsibility which is "check insurance number length" we will create a method called isNotValidInsuranceNumberLength to do that for us etc..

Since we have many methods, we should encapsulate all of them within a class, that will be its responsibility to take care of the national insurance number validation.

And this touches on the SRP(single responsibility principle) because we must ensure that our class will have one and only one reason to change(validation of the national insurance number).

I was changing the imbricated if statements by small methods that describe what they do exactly by its name.

  I advise you, that when you write code think like you are writing a
  story.

  another advice: read Uncle Bob clean code's book.

my refactorization is below :

<?php

class  NationalInsuranceNumberValidator
{
    $insuranceNumber = null;

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

    public function validate()
    {
        if($this->isNotValidInsuranceNumberLength())
        {
            echo "Invalid National Insurance Number. isNotValidInsurenceNumberLength";
        }
        elseif($this->prefixIsNotValid())
        {
            echo "Invalid National Insurance Number. prefixIsNotValid";
        }
        elseif($this->secondPartOfPefixIsNotValid())
        {
            echo "Invalid National Insurance Number. secondPartOfPefixIsNotValid";
        }
        elseif($this->suffixIsNotValid())
        {
            echo "Invalid National Insurance Number. suffixIsNotValid";
        }
        elseif($this->isNotNumericBody())
        {
            echo "Invalid National Insurance Number. isNotValidNumericBody";
        }
        else
        {
            echo "Valid National Insurance Number.";
        }
    }

    private function isNotValidInsuranceNumberLength()
    {
        $insuranceNumberLength = strlen($this->insuranceNumber);

        return $insuranceNumberLength < 10 || $insuranceNumberLength > 10;
    }

    private function prefixIsNotValid()
    {
        return preg_match("/[^!@#$%^&*]*(BG|GB|NK|TN|NT|ZZ|D|F|I|Q|U|V)[^!@#$%^&*]*/i", $this->getPrefix()) == 1 ? true : false;
    }

    private function secondPartOfPefixIsNotValid()
    {
        $prefix = $this->getPrefix();

        return preg_match("/[^!@#$%^&*]*(O)[^!@#$%^&*]*/i", $prefix[1]) == 1 ? true : false;
    }

    private function suffixIsNotValid()
    {
        return preg_match("/[^!@#$%^&*]*(A|B|C|D)[^!@#$%^&*]*/i", $this->getSuffix()) == 1 ? false : true;
    }

    private function isNotNumericBody()
    {
        return !is_numeric($this->getNumericBody());
    }

    private function getPrefix()
    {
        return substr($this->insuranceNumber, 0, 2);
    }

    private function getNumericBody()
    {
        return substr($this->insuranceNumber, 2, 7);
    }

    private function getSuffix()
    {
        return substr($this->insuranceNumber, 9, 1);
    }
}

$nationalInsuranceNumber = new NationalInsuranceNumberValidator('AA1220091C');

$nationalInsuranceNumber->validate();


How to optimize my telegram bot written in Python? I have written a fairly simple bot that provides information or additional choice based on the user's query. It runs well and does the job, however, I was wondering how can I optimize my code and use the telebot better?
Currently the main action happens in the query handler and something tells me that I am missing possible efficiency gains (when I will attempt to build more choices for example). Below is the code, simplified for demonstration purposes:
import telebot
from telebot import types

token = bottoken

bot = telebot.TeleBot(token)

@bot.message_handler(commands=['start'])
def welcome(message):

    # here i give the main choice that is always visible to the user

    markup = types.ReplyKeyboardMarkup(resize_keyboard=False)
    item1 = types.KeyboardButton("Button1")
    item2 = types.KeyboardButton("Button2")

    markup.add(item1, item2)

    bot.send_message(message.chat.id, "Welcome message".format(message.from_user, bot.get_me()),
        parse_mode='html', reply_markup=markup)

@bot.message_handler(content_types=['text'])
def main_choice(message):

    # here i develop each of the main choices and redirect to the callback handler

    if message.text == 'Button1':

        # Forward to callback handler

            markup = types.InlineKeyboardMarkup()
            item1 = types.InlineKeyboardButton("Yes", callback_data = 'demo_yes')
            item2 = types.InlineKeyboardButton("No", callback_data = 'demo_no')

            markup.add(item1, item2)

            bot.send_message(message.chat.id, 'Some text', reply_markup=markup)

    elif message.text == 'Button2':

        # some logic

    else :
        bot.send_message(message.chat.id, 'Sorry, i dont understand. You can use /help to check how i work')

@bot.callback_query_handler(func=lambda call: True)
def tempo(call):

    # here a handle the queries launched both by main buttons and the inline keyboard

    if call.data == "demo_yes":

        keyboard = types.InlineKeyboardMarkup()
        item1 = types.InlineKeyboardButton("Text1", callback_data = 'call1')
        item2 = types.InlineKeyboardButton("Text2", callback_data = 'call2')
        keyboard.add(item1,item2)

        bot.send_message(call.message.chat.id, 'message', reply_markup = keyboard)

    elif call.data == "demo_no":

        kkeyboard = types.InlineKeyboardMarkup()
        item1 = types.InlineKeyboardButton("Text3", callback_data = 'call3')
        item2 = types.InlineKeyboardButton("Text4", callback_data = 'call4')
        keyboard.add(item1,item2)

        bot.send_message(call.message.chat.id, 'message', reply_markup = keyboard)

    if call.data == "call1":

        keyboard = types.InlineKeyboardMarkup()
        item1 = types.InlineKeyboardButton("Text5", callback_data = 'another_call1')
        item2 = types.InlineKeyboardButton("Text6", callback_data = 'another_call2')
        keyboard.add(item1,item2)

        bot.send_message(call.message.chat.id, 'message', reply_markup = keyboard)

    elif call.data == "call2":

        kkeyboard = types.InlineKeyboardMarkup()
        item1 = types.InlineKeyboardButton("Text7", callback_data = 'another_call3')
        item2 = types.InlineKeyboardButton("Text8", callback_data = 'another_call4')
        keyboard.add(item1,item2)

        bot.send_message(call.message.chat.id, 'message', reply_markup = keyboard)

From my point of view you can use a visitor pattern for this with a simple dictionary, specially for your operations, here is a small example:
Insead of this
@bot.callback_query_handler(func=lambda call: True)
def tempo(call):

    # here a handle the queries launched both by main buttons and the inline keyboard

    if call.data == "demo_yes":

        keyboard = types.InlineKeyboardMarkup()
        ......

Use the following
tempo_operations = {"demo_yes" : demo_yes_function,
                    "demo_no" : demo_no_function,
                    ....}

@bot.callback_query_handler(func=lambda call: True)
def tempo(call):

    if call.data in tempo_operations:
        tempo_operations[call.data]()
    else:
        print("Operation not supported")

Hope you can get the idea :)


Longest Common Substring solution for a string and a list Recently I have discovered one thing, which makes me feel frustrated. The Perl is lack of syntax sugar for strings, to do the same things, those are applicable to lists. The implementation I posted here would require to be rewritten to work for lists.

Thus, I had to re-write the code to Python to work for list as efficient as it does for strings:

def _contains(subseq, inseq):
    return any(inseq[pos:pos + len(subseq)] == subseq for pos in range(0, len(inseq) - len(subseq) + 1))

def lcss(s1, s2, threshold=3):

    all_of = []

    def is_inner_seq(i, j):
        return any(k < i < j <= l or k <= i < j < l for k, l in all_of)

    for i in range(0, len(s1)):
        for j in range(i + threshold, len(s1) + 1):

            if _contains(subseq=s1[i:j], inseq=s2):
                all_of.append((i, j))  # add a tuple to work OK when s1, s2 are lists

    only_outer_seq = filter(lambda tup: not is_inner_seq(tup[0], tup[1]),
                            all_of)

    return [s1[i:j] for i, j in only_outer_seq]

Here I implemented one minor improvement - filtering out inner substrings from the resultset, which aims to display more laconic result.

To not to frighten anyone with long sheets of code, here I am attaching a link to units tests, I used to tune up this solution: https://gist.github.com/anonymous/aa7540cc7db2ca446e2b

Your comments are welcomed! Especially on function '_contains', which does not look like a simple way of doing a simple task. I have started studying the Python just a few weeks ago.

No docstrings! What do your functions do and how do I call them?
Your lcss function has a name that suggests it might return the longest common subsequence. But in fact it returns all common subsequences of length 3 or more (except for ones that appear inside other common subsequences in s1). Is this really what you want the function to return? If so, you probably want to rethink the name.
Python's in operator already implements your _contains operation for strings:

>>> [w.strip() for w in open('/usr/share/dict/words') if 'abc' in w]
['crabcatcher', 'dabchick']

It's also much faster.
I guess you didn't use in because you want your subsequence operation to work on all kinds of sequence (not just strings). But in that case you should prefer to use one of the Boyer–Moore search algorithms, for example Boyer–Moore–Horspool, so that the complexity is more like \$O(n)\$ (on average on random inputs) than \$O(mn)\$.

See this answer for a Boyer–Moore–Horspool implementation in Python.
Your algorithm has complexity \$O(m^3n)\$ (where \$m\$ and \$n\$ are the lengths of the two sequences). But the longest common subsequence problem for two sequences has a well known \$O(mn)\$ algorithm, described in detail in the Wikipedia article.


Updating Google Sheets table efficiently Program description:

A program accepts a list (uTable). The list consists of at least 4 different lists (rows), each with 7 elements (cells). Check whether the value in [7] is zero and assign a new random variable to it on the basis of what [3] is: 1, 2, 3, 4 or 5. A chance should be used to calculate the chance of the assigning operation. A cell should be updated using sheet_store.update_cell(i, k, value). A check for admin permissions should be included ( if adminId in admins:).

My solution:
 if adminId in admins:
        uTable = sheet_store.get_all_values()
        for i in range(len(uTable)):
            row = uTable[i]
            if (row[7] == '0'):
                chance = random.uniform(0, 1)
                if (row[3] == '5'):
                    if chance <= 0.05:
                        print('5 EXECUTED!')
                        sheet_store.update_cell(i+1, 8, 1)
                elif (row[3] == '4'):
                    random_1 = randint(1, 3)
                    if chance <= 0.1:
                        print('4 EXECUTED!')
                        sheet_store.update_cell(i+1, 8, random_1)
                elif (row[3] == '3'):
                    random_2 = randint(1, 5)
                    if chance <= 0.4:
                        print('3 EXECUTED!')
                        sheet_store.update_cell(i+1, 8, random_2)
                elif (row[3] == '2'):
                    random_3 = randint(1, 10)
                    if chance <= 0.6:
                        print('2 EXECUTED!')
                        sheet_store.update_cell(i+1, 8, random_3)
                elif (row[3] == '1'):
                    random_4 = randint(1, 20)
                    if chance <= 1:
                        print('1 EXECUTED!')
                        sheet_store.update_cell(i+1, 8, random_4)
        return True
    else:
        return False

Input: (TABLE IS TRANSFERRED AS LIST) (Row and column with index 0 are not included)
0    1              2   3       4          5           6          7
1 артифак           0   5   незвестен   a_1.jpg 07/08/20/15/30    0
2 Съедобный предмет 1   3   бафф        a_2.jpg                   0
3 описание          2   4   неизвестен  a_3.jpg время обновления  1
4 Отличный компан   3   2   компаньон                             7

Runtime:
1 EXECUTED!
2 EXECUTED!

success
0.5s

Is there any way to improve the code and make the program run faster? Thank you in advance.

These are my suggestions regarding performances:
Generate random numbers only if needed
The new value for the column 7 can be generated after you are sure that needs to be updated. From:
elif (row[3] == '4'):
    random_1 = randint(1, 3)
    if chance <= 0.1:
        print('4 EXECUTED!')
        sheet_store.update_cell(i+1, 8, random_1)

To:
elif (row[3] == '4'):
    if chance <= 0.1:
        print('4 EXECUTED!')
        sheet_store.update_cell(i+1, 8, randint(1, 3))

In this way, the new random value is generated only if chance<=0.1. Same for the other cases.
API call cost
I assume that you are using gspread. The doc says:

Under the hood, gspread uses Google Sheets API v4. Most of the time
when you call a gspread method to fetch or update a sheet gspread
produces one HTTP API call.

Given this statement, your code sends 1 API call with get_all_values() and 1 APIs call for each row to update with update_cell(). In the worst case, there are 5 API calls.
One suggestion is to generate the list of all new values for the column 7 and send a single update call with update(), so that the total number of API calls will be reduced to 2.


HackerRank, Sherlock and Array Here is the code I have written for a HackerRank challenge which takes multiple arrays, and attempts to determine if there exists an element in an array such that the sum of the elements on its left is equal to the sum of the elements on its right. If there are no elements to the left/right, then the sum is considered to be zero.

Detailed problem statement:

Input Format

The first line contains \$T\$, the number of test cases. For each test case, the first line contains N , the number of elements in the array \$A\$ . The second line for each test case contains N space-separated integers, denoting the array \$A\$.

Output Format

For each test case print YES if there exists an element in the array, such that the sum of the elements on its left is equal to the sum of the elements on its right; otherwise print NO.

It works fine, but it's definitely not an optimal solution, as it fails two test cases (time outs occur). Could anybody provide any insights on optimising it further?

T = int(input())
N = []
A = []

for i in range(T):
    #N.append(int(input()))
    N = int(input())
    arr = input().strip().split(' ')
    arr = list(map(int, arr))
    A.append(arr)
#print(A)
for i in A:
    count=0
    for j in range(len(i)):
        preSum = sum(i[0:j])
        postSum = sum(i[j+1:len(i)])
        if(preSum == postSum):
            count+=1
    if(count>0):
        print("YES")
    else:
        print("NO")

The flaw in your algorithm is that you're summing for each item in each array. That means you get \$O(n^2)\$ for time complexity.

Sum once, then add to the left side whilst removing from the right side. This will give a time complexity of \$O(2n)\$ (once to sum, another to move each element over from the left to the right side).

Additionally, once you have found 1 case in which both sides match, you can stop looking for that testcase.


Count all unique subsets with Sum S Team 7 faces a horrible foe. He can only be defeated with a special
  quadruple combination attack of strength (\$1 <=S <= 10^9\$). Naruto,
  Sasuke, Sakura and Kakashi must attack simultaneously to perform the
  combo. Each of them can choose from \$N\$ (\$1 <= N <=1000\$) attacks,
  having strengths \$si\$ each (\$0 <= i < N\$, \$1 <= si <=10^9\$). The strengths
  of individual attacks add up to form the strength of the combo.

  Is there a valid combination that they can use? Note that the same attacks are available to all of them.

  You are required to write a function which takes input as follows:

  An integer \$N\$ as number of attacks, an integer vector s[] as the
  strengths of \$N\$ attacks and an integer S as the required strength of
  the combo. Set the output variable to the number of distinct valid
  combos.

  Two combinations are different if they differ in strength of at least one attack used.

  Input: 1 {1} 4
  Output: 1  ===> {1,1,1,1}

  Input: 2 {1,2} 5
  Output: 1 ===> {1,1,1,2}

#include "stdafx.h"
#include<stdio.h>
#include<string.h>
#include<iostream>
#include<vector>
#include<map>
#include<set>
#include<algorithm>

const int noOfPalyers = 4;

int validCombo(int input1,int input2[],int input3)
{
    std::cout<< "-----------------------------------------------------------------------------"<<"\n";
    int count = 0;
    std::vector<int> vec;
    int size =input1*noOfPalyers;
    for(int i = 0; i < input1; i++)
    {
        for(int j = 0; j < noOfPalyers;j++)
        {
            vec.push_back(input2[i]);
        }
    }

    std::vector< std::vector< std::pair<int, int> > > vecHash;

    for(int i =0; i < (size*size); i++)
    {
        vecHash.push_back(std::vector< std::pair<int, int> > ());
    }

    for(int i =0; i < size; i++)
    {
        for(int j =1; j < size; j++)

        {
            int key = vec[i] + vec[j];
            if(vec[i]<= vec[j])
                vecHash[key].push_back(std::make_pair(vec[i], vec[j]));
            else
                vecHash[key].push_back(std::make_pair(vec[j], vec[i]));

        }
    }

    std::set< std::vector<int> > solution;
    for(int i = 0; i < input3; i++)
    {

        if(vecHash[input3 - i].size() > 0)
        {
            for(int k = 0; k < vecHash[i].size() ; k++)
            {
                std::pair<int, int> fPair = vecHash[i][k];

                for(int m =0; m < vecHash[input3 - i].size(); m++)
                {
                    std::pair<int, int> sPair = vecHash[input3 - i][m];
                    std::vector<int> vec;
                    vec.push_back(fPair.first);
                    vec.push_back(fPair.second);
                    vec.push_back(sPair.first);
                    vec.push_back(sPair.second);
                    std::sort(vec.begin(), vec.end());
                    solution.insert(vec);
                }

            }
        }
    }

    std::set< std::vector< int> >::iterator it;
    for (it = solution.begin(); it != solution.end(); it++)
    {
        std::cout<< "\n";
        for(int i = 0; i < (*it).size(); i++)
            std::cout << (*it)[i] << " , ";
        std::cout<< "\n";
    }
    count = solution.size();

    std::cout << "solution size = "  << solutionSet.size();
    std::cout<< "\n";
    std::cout<< "\n";

    std::cout<< "-----------------------------------------------------------------------------"<<"\n";

    return count;

}

int  main()
{
    int i = 3;
    int arr[] = {1,2,3};
    int j = 7;
    int arr1[] ={1};
    std::cout <<"o/p == "  << validCombo(i, arr, j)<< "\n";
    std::cout <<"o/p == "  << validCombo(1, arr1, 4);

    getchar();
    return 0;
}

A lot of your vector initialization can be handled by vector's constructor. Not only is that much shorter and less error prone, it's also more efficient since it lets the vector avoid resizing over and over again.

As an example:

std::vector< std::vector< std::pair<int, int> > > vecHash;

for(int i =0; i < (size*size); i++)
{
    vecHash.push_back(std::vector< std::pair<int, int> > ());
}

This can be:

std::vector< std::vector< std::pair<int, int> > > vecHash(size*size);

if(vecHash[input3 - i].size() > 0)

This should be if(!vecHash[input3 - i].empty()) as empty() reads clearer.

Your logic and your IO should be handled separately. You should add a function for printing solutions so your validCombo isn't doing quite so much. That will mean returning the solutions instead of the number of solutions, but that makes more sense anyway.

For small little programs like this it doesn't really matter, but for anything non-trivial, you want each function to only do one thing. That allows you to control and modify things in a much easier way. (Even though we're not talking about OOP, this is essentially the essence of the SRP.

count = solution.size();
std::cout << "solution size = "  << solutionSet.size();

// ...

return count;

Either use count consistently, or don't bother assigning it.

Also, declare variables as close to use as possible. It's confusing to try to figure out where in the world count came from when it's declared a few dozen lines up.


Getting lists of values from a CSV I have a function that takes a column title, and a response.body from a urllib GET (I already know the body contains text/csv), and iterates through the data to build a list of values to be returned. My question to the gurus here: have I written this in the cleanest, most efficient way possible? Can you suggest any improvements?

def _get_values_from_csv(self, column_title, response_body):
    """retrieves specified values found in the csv body returned from GET
    @requires: csv
    @param column_title: the name of the column for which we'll build a list of return values.
    @param response_body: the raw GET output, which should contain the csv data
    @return: list of elements from the column specified.
    @note: the return values have duplicates removed. This could pose a problem, if you are looking for duplicates.
    I'm not sure how to deal with that issue."""
    dicts = [row for row in csv.DictReader(response_body.split("\r\n"))]
    results = {}
    for dic in dicts:
        for k, v in dic.iteritems():
            try:
                results[k] = results[k] + [v] #adds elements as list+list
            except: #first time through the iteritems loop.
                results[k] = [v]

    #one potential problem with this technique: handling duplicate rows
    #not sure what to do about it.
    return_list = list(set(results[column_title]))
    return_list.sort()
    return return_list

Here's a shorter function that does the same thing. It doesn't create lists for the columns you're not interested in.

def _get_values_from_csv(self, column_title, response_body):
    dicts = csv.DictReader(response_body.split("\r\n"))
    return sorted(set(d[column_title] for d in dicts))


Image filter for gradient detection This filter is basically gradient detection (the variable gradImg) like an edge detection (Sobel or Prewitt filters) and also the direction of that gradient whether vertical or horizontal (dirImg).

I'm trying to optimize it to run faster and I found that this part of the code slow. Would you have any suggestions please?  Here's the code you can copy to http://quick-bench.com/.

#include <math.h>
static void Old_Version(benchmark::State& state) {
  // Code inside this loop is measured repeatedly
  int rows = 1000, cols = 1000;
  unsigned char *smoothImg = new unsigned char[cols*rows];
  short *gradImg = new short[cols*rows];
  unsigned char* dirImg = new unsigned char[cols*rows];
  for (auto _ : state) {
    for (int i = 1; i<rows - 1; i++) {
           for (int j = 1; j<cols - 1; j++) {
               int com1 = smoothImg[(i + 1)*cols + j + 1] - smoothImg[(i - 1)*cols + j - 1];
               int com2 = smoothImg[(i - 1)*cols + j + 1] - smoothImg[(i + 1)*cols + j - 1];

               int gx=abs(com1+com2+(smoothImg[i*cols + j + 1] - smoothImg[i*cols + j - 1]));
               int gy=abs(com1-com2+(smoothImg[(i + 1)*cols + j] - smoothImg[(i - 1)*cols + j]));

               int sum = (int)sqrt((double)gx*gx + gy*gy);

               int index = i*cols + j;

               gradImg[index] = sum;
               if (sum >= 20) {
                   if (gx >= gy) dirImg[index] = 1;//1 vertical
                   else      dirImg[index] = 2;//2 Horizontal
               }
           }
       }
  }
}
// Register the function as a benchmark
BENCHMARK(Old_Version);

My compilation command, using GCC (Ubuntu 8.1.0-5ubuntu1~16.04) 8.1.0:

g++ -std=c++1z -fomit-frame-pointer -O4 -ffast-math -mmmx -msse -msse2 -msse3 -DNDEBUG -Wall improve_code.cpp -o improve_code -fopenmp

For SIMD optimization, I tried doing it with SSE2, as the compilation flags imply SSSE3 is disabled, and SSE3 is not useful here.

It's mostly a transliteration of the scalar code, but there are a few points of interest:

Except the squaring of gx and gy which is done in 32bit, and taking the square root which is done in floating point, most arithmetic is done in 16bit. That shouldn't change the results as everything fits well within the margins.
16bit is more than 8bit, so basically there is a choice between doing some half-wide loads and stores, or "doubling" the code. Often "doubling" the code is a little faster because it gets more arithmetic done with the same number of loads and stores, but that didn't pan out this time. So I used the less-common _mm_loadl_epi64 and the corresponding store. Note that these notionally take a pointer to __m128i, but they actually just load and store a qword (with no alignment restriction).
_mm_madd_epi16 is used for the gx*gx + gy*gy expression, which requires interleaving gx and gy, but is still more convenient than the alternatives.
There is no good way to conditionally not write a byte somewhere in the middle of a qword store, so for gradients less than 20 I write a zero, which is definitely different than the scalar code (but maybe safer anyway)
SSE2 has no single instruction to take an absolute value, but SSSE3 does. I didn't use it, but I factored it out so it can easily be updated if SSSE3 support can be assumed someday.
I paid no particular attention to alignment. The qword loads and stores don't care that much about alignment inherently, but on Core2 it is nevertheless Quite Bad to cross a cache line that way. If Core2 is a serious target, perhaps that aspect can be improved.
No testing was done other than for performance.

The results on quick-bench are OK but not amazing, about an 4x improvement (the final code there is labeled Gradient2). But maybe that's all we can hope for, given that there are square roots to be done. Of course you can still add threading on top of this which should (mostly) stack multiplicatively.

#include <x86intrin.h>

__m128i abs_epi16(__m128i x)
{
    __m128i m = _mm_srai_epi16(x, 15);
    return _mm_xor_si128(_mm_add_epi16(x, m), m);
}

void Gradient(unsigned char *smoothImg, short *gradImg, unsigned char* dirImg, size_t rows, size_t cols) {
    for (size_t i = 1; i + 1 < rows; i++) {
        size_t j = 1;
        // do blocks of 8 pixels at the time until near the edge
        for (; j + 8 < cols; j += 8) {
            __m128i zero = _mm_setzero_si128();
            // com1
            __m128i img1 = _mm_loadl_epi64((__m128i*)&smoothImg[(i + 1)*cols + j + 1]);
            __m128i img2 = _mm_loadl_epi64((__m128i*)&smoothImg[(i - 1)*cols + j - 1]);
            __m128i img1A = _mm_unpacklo_epi8(img1, zero);
            __m128i img2A = _mm_unpacklo_epi8(img2, zero);
            __m128i com1 = _mm_sub_epi16(img1A, img2A);
            // com2
            __m128i img3 = _mm_loadl_epi64((__m128i*)&smoothImg[(i - 1)*cols + j + 1]);
            __m128i img4 = _mm_loadl_epi64((__m128i*)&smoothImg[(i + 1)*cols + j - 1]);
            __m128i img3A = _mm_unpacklo_epi8(img3, zero);
            __m128i img4A = _mm_unpacklo_epi8(img4, zero);
            __m128i com2 = _mm_sub_epi16(img3A, img4A);
            // gx
            __m128i img5 = _mm_loadl_epi64((__m128i*)&smoothImg[i*cols + j + 1]);
            __m128i img6 = _mm_loadl_epi64((__m128i*)&smoothImg[i*cols + j - 1]);
            __m128i img5A = _mm_unpacklo_epi8(img5, zero);
            __m128i img6A = _mm_unpacklo_epi8(img6, zero);
            __m128i gx = _mm_add_epi16(_mm_add_epi16(com1, com2), _mm_sub_epi16(img5A, img6A));
            // gy
            __m128i img7 = _mm_loadl_epi64((__m128i*)&smoothImg[(i + 1)*cols + j]);
            __m128i img8 = _mm_loadl_epi64((__m128i*)&smoothImg[(i - 1)*cols + j]);
            __m128i img7A = _mm_unpacklo_epi8(img7, zero);
            __m128i img8A = _mm_unpacklo_epi8(img8, zero);
            __m128i gy = _mm_add_epi16(_mm_sub_epi16(com1, com2), _mm_sub_epi16(img7A, img8A));
            // sum
            // gx and gy are interleaved, multiplied by themselves and
            // horizontally added in pairs, creating gx*gx+gy*gy as a dword
            // 32bits is required here to avoid overflow, but also convenient for the next step
            __m128i gxgyL = _mm_unpacklo_epi16(gx, gy);
            __m128i gxgyH = _mm_unpackhi_epi16(gx, gy);
            __m128i lensqL = _mm_madd_epi16(gxgyL, gxgyL);
            __m128i lensqH = _mm_madd_epi16(gxgyH, gxgyH);
            __m128i lenL = _mm_cvttps_epi32(_mm_sqrt_ps(_mm_cvtepi32_ps(lensqL)));
            __m128i lenH = _mm_cvttps_epi32(_mm_sqrt_ps(_mm_cvtepi32_ps(lensqH)));
            __m128i sum = _mm_packs_epi32(lenL, lenH);

            // store gradient lengths
            size_t index = i*cols + j;
            _mm_storeu_si128((__m128i*)&gradImg[index], sum);

            // classify H/V/low
            __m128i thresholdLow = _mm_set1_epi16(19);
            __m128i markerV = _mm_set1_epi8(1);
            __m128i isSignificant = _mm_cmpgt_epi16(sum, thresholdLow);
            __m128i isHorizontal = _mm_cmplt_epi16(abs_epi16(gx), abs_epi16(gy));
            // if not horizontal, then 1 - 0 = 1
            // if horizontal,  then 1 - (-1) = 2
            // if not significant, then make it zero
            __m128i classifier = _mm_and_si128(_mm_sub_epi16(markerV, isHorizontal), isSignificant);
            _mm_storel_epi64((__m128i*)&dirImg[index], _mm_packs_epi16(classifier, classifier));
        }
        for (; j + 1 < cols; j++) {
            int com1 = smoothImg[(i + 1)*cols + j + 1] - smoothImg[(i - 1)*cols + j - 1];
            int com2 = smoothImg[(i - 1)*cols + j + 1] - smoothImg[(i + 1)*cols + j - 1];

            int gx=abs(com1+com2+(smoothImg[i*cols + j + 1] - smoothImg[i*cols + j - 1]));
            int gy=abs(com1-com2+(smoothImg[(i + 1)*cols + j] - smoothImg[(i - 1)*cols + j]));

            int sum = (int)sqrt((double)gx*gx + gy*gy);

            size_t index = i*cols + j;

            gradImg[index] = sum;
            if (sum >= 20) {
                if (gx >= gy) dirImg[index] = 1; //1 vertical
                else      dirImg[index] = 2; //2 Horizontal
            }
        }
    }
}


Checking if an index is out of bounds For a simple project, I represent a two-dimensional grid as a one-dimensional array, with both row and column indices between 1 and gridHeight. When retrieving a cell from the grid, I first need to check if both row and column IDs are within allowed bounds.

public void checkIfAllowed(int row_or_column) {
    if ((row_or_column < 1) || (row_or_column > gridHeight)) {
        throw new IndexOutOfBoundsException("Row or column ID is not acceptable.");
    }
}

public boolean getCellById(int row, int column) {
    checkIfAllowed(row);
    checkIfAllowed(column);
    return grid[row * gridHeight + column];
}

Can it possibly be improved?

There are two things about this code that I don't particularly like:

Somewhat repetitive code in getCellById.
It asks for permission instead of forgiveness: first checks everything, and then does the action. I would prefer a try .. catch construction (something like "try to return a value and then catch IndexOutOfBoundsException), but would it really work for Java? The array dimension is defined during run time, and initial declaration for my grid is just private boolean[] grid;.

Not sure if I've understood yout question correctly, but I assume, that your grid is a square grid (you test both, maximal high and width, as a lower than grid's height).

Referring to your first question:
There are many options. You can make a method which takes two parameters at a time:

public void checkIfAllowed(int row, int column){
      boolean areCorrect = checkIfRowAllowed(row) && checkIfColAllowed(column);
      if(!areCorrect){
           throw new IndexOutOfBoundsException("Row or column ID is not acceptable.");
      }
}

private boolean checkIfAllowed(int row_or_column) {
    return (row_or_column < 1 || row_or_column > gridHeight);
}

Now, a client is not forced to check each row and column in separate lines. Just an example:

public boolean getCellById(int row, int column) {
      try{
          checkIfAllowed(row, column);
          return grid[row * gridHeight + column];

      } catch (IndexOutOfBoundsException e) {
           // log an error, or do something else
      }
}

But this exception handling seems strage for me. I would simply throw it in getCellById(int row, int col) and handle when this method is invoked.


Function to show or hide validation error messages I'm a bit clumsy at writing JavaScript.  I have some nice code like this:

(function($){
    $.fn.forceNumeric = function(){
        return this.each(function(){
            $(this).keyup(function(){
                if (!/^[0-9]+$/.test($(this).val())){
                    $(this).val($(this).val().replace(/[^0-9]/g, ''));
                }
            });
        });
    };
})(jQuery);

I know what it does and how it works. I can call the function and I see why it works.

For the life of me, I cannot figure out how to use that same elegant style/approach for my Neanderthal function:

function showHideErrorBox(theFriendlyName, check){
     var topLevelDiv,
        numDivsAfter;

        if(check === 0){
        // REMOVE THE OLD ERROR MESSAGE IN THE ERROR BOX
            $('.form-validation-errors').find('div').each(function(){
                if( $(this).html().indexOf(theFriendlyName) != -1 ){
                    $(this).remove();
                }
            });
        }

        //  CHECK WHETHER THE ENTIRE MESSAGE BOX CAN BE REMOVED NOW
        topLevelDiv = document.getElementById('alert-form-errors');
        numDivsAfter = topLevelDiv.getElementsByTagName('div').length;

        if(check === 0){
            if(numDivsAfter <= 1){
                $('#alert-form-errors').hide();
            }

        }else if(check === 1){

            if(numDivsAfter >= 1){
                $('#alert-form-errors').show();
            }
        }
}

Can anyone help me out? Where do I start? I have Google'd for tutorials, but I have not been able to find something suitable for my needs.

You could start by minimizing your function calls. like so

ORIGINAL

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test($(this).val())){
                $(this).val($(this).val().replace(/[^0-9]/g, ''));
            }
        });
    });
};

OPTIMIZED

$.fn.forceNumeric = function(){
    return this.each(function(){
        $(this).keyup(function(){
            if (!/^[0-9]+$/.test(this.value)){
                this.value = this.value.replace(/[^0-9]/g, '');
            }
        });
    });
};

  Just in this little bit you have saved on 3 Function calls.

And on the larger part.

OPTIMIZED

function showHideErrorBox(theFriendlyName, check){
     var topLevelDiv,
         numDivsAfter;

        if(check === 0){
        // REMOVE THE OLD ERROR MESSAGE IN THE ERROR BOX
            $('.form-validation-errors').find('div').each(function() {
                var $this = $(this);
                if( $this.html().indexOf(theFriendlyName) !== -1 ){
                    $this.remove();
                }
            });
        }

        //  CHECK WHETHER THE ENTIRE MESSAGE BOX CAN BE REMOVED NOW
        topLevelDiv = $(document.getElementById('alert-form-errors'));
        numDivsAfter = topLevelDiv[0].getElementsByTagName('div').length;

        if(check === 0 && numDivsAfter <= 1) {
            return topLevelDiv.hide();
        }

        if(check === 1 && numDivsAfter >= 1) {
            return topLevelDiv.show();
        }
}

  You have also saved on 3 Function calls here too;

Just remember that $ is a function and it's better to call a function once than twice right?

if you ever have something like this

$(this).show();
$(this).val('hello');
$(this).hide();

It should be like this

var $this = $(this);
$this.show()
this.value = 'hello';
$this.hide();


Frog jumping algorithm I took a test on Codility for coding the minimum number of frog jumps to reach from position x to position y:

  A small frog wants to get to the other side of the road. The frog is
  currently located at position X and wants to get to a position greater
  than or equal to Y. The small frog always jumps a fixed distance, D.
  Count the minimal number of jumps that the small frog must perform to
  reach its target.

  Write a function: def solution(x, y, d) that, given
  three integers X, Y and D, returns the minimal number of jumps from
  position X to a position equal to or greater than Y.

  For example,
  given:
  X = 10
             Y = 85
             D = 30

  the function should return 3,
  because the frog will be positioned as follows:

  after the first jump,
  at position 10 + 30 = 40

  after the second jump, at position 10 + 30 +
  30 = 70

  after the third jump, at position 10 + 30 + 30 + 30 = 100
  Assume that:

  X, Y and D are integers within the range
  [1..1,000,000,000];

  X ≤ Y.

  Complexity: expected worst-case time
  complexity is O(1);

  expected worst-case space complexity is O(1).

Here is my solution:

def solution(x, y, d)
    position = x
    positions = []
    until position >= y
        position += d
        positions << position
    end
    return positions.length

end

This solution works, but got a performance score of 0% and a correctness score of 100%.

My solution works on smaller data sets, but always times out on larger data sets.

It's a math problem more than a programming problem.

The distance to cover is \$y - x\$, so we divide that with our "speed", d, and round up to get the number of jumps. No loops or anything, just arithmetic.

The only trick is that if d is an integer, our division will be imprecise, since \$\frac{85-10}{30} = 2.5\$ but the decimal gets dropped by everything being treated as integers.

So we can either make d a float using to_f, or we can use fdiv to force a more precise, floating point division.

Using the latter, you get:

def solution(x, y, d)
  (y - x).fdiv(d).ceil
end

The return can be skipped, since this is Ruby.

Using to_f you could do:

def solution(x, y, d)
  jumps = (y - x) / d.to_f
  jumps.ceil
end

Finally, as Caridorc points out, renaming the arguments could make this even clearer

def solution(start, goal, jump_distance)
  (goal - start).fdiv(jump_distance).ceil
end

The above is the more correct solution (again, it's a math problem), but if we pretend that we do need a loop, you don't need the array for anything. Instead you could just do:

def solution(x, y, d)
  jumps = 0
  until x >= y
    x += d
    jumps += 1
  end
  jumps
end

Again, not the right solution for the problem, but a better one than using an array.


Program for printing all numbers that user inputs till 42 in Clojure As the name suggests it is a very simple program to print numbers to screen till user inputs 42.

(map
  println
  (take-while
    #(not= % "42")
    (repeatedly #(.readLine *in*))))

I have started learning Clojure so please share whether there is some easier way to read lines or anything else that can be improved.

I see a few things worth pointing out:

Lazy sequences and side effects

First and foremost, you should be careful about mixing side effects (printing using println) and lazy sequences (such as map, repeatedly, and many more). It may not work the way you expect. Here's an example of side effects just disappearing:

(let [lazy-effects (map println [1 2 3])]
  1)

This doesn't print anything as lazy-effects is never evaluated. There's no need to traverse the lazy sequence, and hence no need to evaluate println on each of the elements. To be sure, your code doesn't suffer from this, as you're evaluating map right in the REPL (or where ever else you're running it), but as soon as you put it inside def or any number of other hidden dangers, it'll no longer get evaluated. Just something to keep in mind.

Using doseq and friends for forcing side effects

With the above in mind, your code would work equally well, but not carry the hidden risk of postponing side effects inside a lazy sequence, if it made use of doseq or some of the other do-like constructs in Clojure. doseq could be used as follows:

(doseq [input (take-while #(not= % "42") (repeatedly read-line))]
  (println input))

If you think this feels slightly less "Clojure-like" in nature, you're right! Side effects generally mix poorly with many of Clojure's core constructs, and special care often needs to be taken. doseq, doall and a few other are provided to help in the case where you really want to force the evaluation of something.

Using read-line from Clojure core

As you probably noticed I managed to sneak in a reference to read-line above, which you might as well be using for this scenario. There is no inherent benefit except the removed need for Java interop.

Otherwise I'd say you're on the right track. The concept of generating an infinite lazy sequence using repeatedly, limiting its length using take-while, and transforming it using map is spot on. It just so happens that this works more painlessly with pure functions that do not cause side effects.


Named Semaphore with async calls I have multiple distinct processes that need to access external resources that are rate limited.  The processes are all async in nature and run in different applications.  In times past I would just use SemaphoreSlim this design doesn't allow for that.

I've found several samples that seem to be half complete, or cut and paste into their code.  This was derived from an existing post, but heavily modified to encompass the additional methods and to honor the cancellation token.

Any feedback would be appreciated.

public sealed class SemaphoreAsync : IDisposable
{
  Semaphore _semaphore;

  private SemaphoreAsync(Semaphore sem) => _semaphore = sem;
  public SemaphoreAsync(int initialCount, int maximumCount) => _semaphore = new Semaphore(initialCount, maximumCount);
  public SemaphoreAsync(int initialCount, int maximumCount, string name) => _semaphore = new Semaphore(initialCount, maximumCount, name);
  public SemaphoreAsync(int initialCount, int maximumCount, string name, out bool createdNew, SemaphoreSecurity semaphoreSecurity) => _semaphore = new Semaphore(initialCount, maximumCount, name, out createdNew, semaphoreSecurity);

  public static SemaphoreAsync OpenExisting(string name)
  {
    return new SemaphoreAsync(Semaphore.OpenExisting(name));
  }

  public static SemaphoreAsync OpenExisting(string name, SemaphoreRights rights)
  {
    return new SemaphoreAsync(Semaphore.OpenExisting(name, rights));
  }

  public static bool TryOpenExisting(string name, out SemaphoreAsync result)
  {
    if (Semaphore.TryOpenExisting(name, out Semaphore semaphore))
    {
      result = new SemaphoreAsync(semaphore);
      return true;
    }
    result = null;
    return false;
  }

  public static bool TryOpenExisting(string name, SemaphoreRights rights, out SemaphoreAsync result)
  {
    if (Semaphore.TryOpenExisting(name, rights, out Semaphore semaphore))
    {
      result = new SemaphoreAsync(semaphore);
      return true;
    }
    result = null;
    return false;
  }

  public async Task<bool> WaitOne(TimeSpan timeout, CancellationToken ct)
  {
    DateTime start = DateTime.UtcNow;
    while (!_semaphore.WaitOne(0))
    {
      ct.ThrowIfCancellationRequested();
      if (DateTime.UtcNow < start.Add(timeout))
        return false;
      await Task.Delay(100, ct);
    }
    return true;
  }

  public async Task<bool> WaitOne(int millisecondsTimeout, CancellationToken ct)
  {
    DateTime start = DateTime.UtcNow;
    while (!_semaphore.WaitOne(0))
    {
      ct.ThrowIfCancellationRequested();
      if (millisecondsTimeout > 0)
      {
        if (DateTime.UtcNow < start.AddMilliseconds(millisecondsTimeout))
          return false;
      }
      await Task.Delay(100, ct);
    }
    return true;
  }

  public async Task<bool> WaitOne(CancellationToken ct)
  {
    while (!_semaphore.WaitOne(0))
    {
      ct.ThrowIfCancellationRequested();
      await Task.Delay(100, ct);
    }
    return true;
  }

  public SemaphoreSecurity GetAccessControl()
  {
    return _semaphore.GetAccessControl();
  }

  public int Release()
  {
    return _semaphore.Release();
  }

  public int Release(int releaseCount)
  {
    return _semaphore.Release(releaseCount);
  }

  public void SetAccessControl(SemaphoreSecurity semaphoreSecurity)
  {
    _semaphore.SetAccessControl(semaphoreSecurity);
  }

  #region IDisposable Support
  private bool disposedValue = false; // To detect redundant calls
  void Dispose(bool disposing)
  {
    if (!disposedValue)
    {
      if (disposing)
      {
        // TODO: dispose managed state (managed objects).
        if (_semaphore != null)
        {
          _semaphore.Dispose();
          _semaphore = null;
        }
      }
      disposedValue = true;
    }
  }

  // This code added to correctly implement the disposable pattern.
  public void Dispose()
  {
    // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
    Dispose(true);
  }
  #endregion

}

You could use ThreadPool.RegisterWaitForSingleObject to register a callback when a the WaitHandle (Semaphore extends WaitHandle) is signaled.

Together with a TaskCompletionSource you could completely remove all your wait loops.

Example:

private async Task Run()
{
    var semaphore = new Semaphore(0, 1);
    await AwaitWaitHandle(semaphore, CancellationToken.None, TimeSpan.FromMilliseconds(-1));
}

private Task AwaitWaitHandle(WaitHandle handle, CancellationToken cancellationToken, TimeSpan timeout)
{
    var taskCompletionSource = new TaskCompletionSource<bool>();

    var reg = ThreadPool.RegisterWaitForSingleObject(handle,
        (state, timedOut) =>
        {
            // Handle timeout
            if (timedOut)
                taskCompletionSource.TrySetCanceled();

            taskCompletionSource.TrySetResult(true);
        }, null, timeout, true);

    // Handle cancellation
    cancellationToken.Register(() =>
    {
        reg.Unregister(handle);
        taskCompletionSource.TrySetCanceled();
    });

    return taskCompletionSource.Task;
}

You could use AwaitWaitHandle in your SemaphoreAsync implementation to await the Semaphore.


The best way for inserting multiple objects into array I have a transformer helper function. It reduces over the array and transform key/value pairs. At the end of the loop there is the key 'EXAMPLE1' exists and I should insert two objects after the first and the second array(result) items. Right now, it works as expected, but I have concerns - is it a good approach for using splice inside reduce in my case?

 const commonData = Object.entries(settings).reduce((result, [currentKey, currentValue]) => {
        if (currentKey === SETTINGS_FIELDS.EXAMPLE1) {
            const type = {
                key: myKey,
                value: myValue,
            }
            const code = {
                key: myKey,
                value: myValue,
            }

            const insertedData = result.splice(2, 0, type, code)

            return [...result, ...insertedData]
        }

        return result.concat({ key: transformToUpperCase(currentKey), value: currentValue })
    }, [])

    return [...commonData]
}
```

Looks like you were trying to follow the best practices on not mutating data but this is an overkill:

when processing data locally in a function there's no need to clone the intermediate data
no need to clone the array in return, it's already a temporary cloned array
reduce is the wrong choice here - at least if you keep its needless duplication of the resultant array in each step - the task is a simple transformation of each element so map() seems a better choice + an additional splice on the special key
insertedData is always an empty array so no need for [...result, ...insertedData]
for consistency, if you use splice and concat then go all the way and use concat instead of array spread as well or switch to array spread instead of concat.

Simplified code using map():

function foo() {

  const commonData = Object.entries(settings).map(([key, value]) => ({
    key: transformToUpperCase(key),
    value,
  }));

  if (Object.hasOwnProperty.call(settings, SETTINGS_FIELDS.EXAMPLE1)) {
    commonData.splice(2, 0, {
      key: myKey,
      value: myValue,
    }, {
      key: myKey,
      value: myValue,
    });
  }

  return commonData;
}

If you don't override prototype of settings or objects in general the key check can be simplified further: if (SETTINGS_FIELDS.EXAMPLE1 in settings)


Secure database connection in PHP I have this code I use to connect to the database and get a thing from it secure, How can I make more secure??

class db {
    // The database connection
    protected static $connection;

    /**
     * Connect to the database
     *
     * @return bool false on failure / mysqli MySQLi object instance on success
     */
    protected function connect() {
        // Try and connect to the database
        if(!isset(self::$connection)) {

            // Load configuration there are defined in config.php
            require_once('app/config/config.php');
            self::$connection = new mysqli(DB_HOST, DB_USERNAME, DB_PASSWORD, DB_DBNAME);
        }

        // If connection was not successful, handle the error
        if(self::$connection === false) {
            // Handle error - notify administrator, log to a file, show an error screen, etc.
            return false;
        }
        return self::$connection;
    }

    /*
        Query the database

        @param $query The query string
        @return mixed The result of the mysqli::query() function
    */
    private function query($query) {
        // Connect to the database
        $connection = $this->connect();

        // Query the database
        $result = $connection->query($query);
        return $result;
    }

    /*
        Fetch rows from the database (SELECT query)

        @param $query The query string
        @return bool False on failure / array Database rows on success
    */
    public function select($query) {
        $result = $this->query($query);

        if($result === false) {
            return false;
        }

        $rows = array();
        while ($row = $result->fetch_assoc()) {
            $rows[] = $row;
        }
        return $rows;
    }
}

When I want infomation from the DB
Example:

$db = new db();
$result = $db->select("SELECT * FROM users WHERE name = John");

You're asking the wrong question. The right question is "What should I use instead of query?" The answer is that if you want to reinvent the wheel, you should use prepared statements, and if you don't then you should use an ORM which uses prepared statements under the hood. Prepared statements allow you to pass in parameters and ensure that the escaping is done correctly. E.g. you would use "SELECT * FROM foo WHERE bar = ?" passing parameter "baz" rather than "SELECT * FROM foo WHERE bar = " . mysqli_real_escape_string("baz").


Making a database entity superclass I am using JPA for my database work and I need to create a database entity superclass which all of my table entities will extend.

Every database entity will have a primary key named id (which in some cases is an int and in some a String) and also a lastUpdate field which stores the time at which the database row was last updated (set from within the code).

DatabaseEntity superclass:

import java.util.Date;
import javax.persistence.Basic;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.Temporal;
import javax.persistence.TemporalType;

@Entity
public class DatabaseEntity <T> {
    @Id
    @Basic(optional = false)
    @Column(name = "id")
    private T _id;

    @Basic(optional = false)
    @Temporal(TemporalType.TIMESTAMP)
    @Column(name = "last_update")
    private Date _lastUpdate;

    public T getId() {
        return _id;
    }

    public void setId(T id) {
        _id = id;
    }

    public Date getLastUpdate() {
        return _lastUpdate;
    }

    public void setLastUpdate() {
        _lastUpdate = new Date();
    }
}

Anything that I am doing wrong or that I should add in the superclass?

Looks fine for me, but such a class should be abstract, because it does not make sense to be able create instances of this type.

public abstract class DatabaseEntity <T>{
//
}

Making it abstract will reflect the actual abstraction of this entity.

One more thing, try to avoid java.util.Date as much as you can, it is just awful. You can use Joda time instead, or the new java date api if you using Java 8.


Show list of states depending on user selection I have some data that needs to bind to my view depending on the user selection.

Example: changing the list of states in a dropdown depending on the selected country.

My understanding is, there are two ways to do this:

Have a global JS variable, say,
var model, which has a property, "states". This contains all the relevant data of states.

So, I bind the data from the "states" property of the model variable depending on the selection of country.

Code:

In my view:

<select name="state" ></select>
<select name="country" >
   <option value="US" >USA</option>
   <option selected="selected" value="CN" >CHINA</option>
   <option value="IN" >INDIA</option>
</select>
<script type="text/javascript">
    var model = @Html.Raw(Json.Encode(Model)); //Model is my C# object that I am converting into a JS object
</script>

In my JS:

$(document).ready(function () {
    stateSelection(model.StateList)
})

function stateSelection(stateList) {
        var jStates = jQuery('select[name="state"]');
        var selectedCountry = jQuery('select[name="country"] option:selected').val();
        var provinceExists = false;
        jStates
                .children()
                .remove()
                .end()
        if (stateList.length > 0) {
            jQuery.each(stateList, function (key, val) {
                if (selectedCountry === val.ID) {
                    jStates
                        .append('<option value="' + val.Value + '"  class="' + selectedCountry + '" >' + val.Text + '</option>');
                }
            });
        }
    }

Make an ajax call whenever the country dropdown selection is made, then pass the country name/id as parameter and get a response object that contains the list of states that I want to display, and bind them to my view.
In my JS:

$.ajax({
    type: "POST",
    url: Home/GetStates,
    data: $('select[name="country"] option:selected').val(),
    error: function (xhr, status, error) {
    },
    success: successFunction
});

function successFunction(data) {
    stateSelection(data) //data contains the list of states
}

My question basically is, is it bad practice to have a rather large JS object (list of states) as a global variable - or does it make sense to get only the data I need from my server, only when I need it.

I'm trying to understand which one would be the better approach and under what conditions. Or is there a better alternative?

Update:

I tried both and found that loading the data into my DOM was the better way. Not only was the user experience seamless but it helped me avoid writing additional server side code and database calls. Loading the states into my browser did not have any noticeable impact on the front end performance if anything we thought it was better user experience - no loaders - no ajax calls. Thanks to everyone who answered below.

Regarding your question about downloading all options at once versus fetching the state options dynamically using ajax, the answer depends on just how big the state list is, but probably the right answer is to download them all at once.

File Size Considerations

Consider that jquery, which you are already including, is about 30KB minified and compressed.  My guess would be that your state list is small compared with that, so you shouldn't worry about downloading the whole thing at once.  If that assumption is wrong, then ajax might be a reasonable choice.

UX Considerations

Think about the user experience when the entire state list is downloaded.  Making a new a country selection will instantly repopulate the states -- a seamless experience.  If you have to use ajax, there will be a pause between the user's country selection and when the states become available.  This is awkward, and ideally, if you do this, you'd display an ajax spinner (or some other indication that you are waiting for a network response).  This will make your code more complex, and can be difficult to get exactly right.  For example, you'd need to make sure you handle errors gracefully, and show appropriate messages to the user.  If you don't use ajax, you simply avoid all these issues.

So my advice would be to avoid ajax unless you really need it -- eg, your state lists are huge and you know you'll be supporting users with slow connections.


Clean Code / OOP on Monty Hall Simulation Implementation can you review the this code for the Monty Hall simulation, mainly on the aspects of OOP and CleanCode

public static void main(String[] args) {
    final int iterations = 10000;
    System.out.println("chance with    change = " + getChanceForPlay(iterations, true));
    System.out.println("chance without change = " + getChanceForPlay(iterations, false));
}

private static double getChanceForPlay(int iterations, boolean doChange) {
    double win = 0;
    for (int i = 0; i < iterations; i++) {
        List<Price> doors = createDoors();
        int selectedDoor = random.nextInt(3); //the player selects one of three doors

        //let's remove one goat (but not the selected door)
        for (int currentDoor = 0; currentDoor < 3; currentDoor++) {
            if (currentDoor != selectedDoor && doors.get(currentDoor) == Price.GOAT) {
                doors.remove(currentDoor);

                //re-Index, if required
                selectedDoor = currentDoor < selectedDoor? selectedDoor-1:selectedDoor;
                break;
            }
        }

        //player changes door
        if (doChange) {
            selectedDoor = selectedDoor == 0 ? 1 : 0;
        }

        if (doors.get(selectedDoor) == Price.CAR) {
            win++;
        }
    }
    return win / iterations;
}

private static List<Price> createDoors() {
    List<Price> doors = new ArrayList<>();
    doors.add(Price.GOAT);
    doors.add(Price.GOAT);
    doors.add(Price.CAR);
    Collections.shuffle(doors);
    return doors;
}

private enum Price {
    GOAT, CAR
}

I know many other people made the same thing. It's more of an academical question...

some thoughs upon it done by myself:

should i break up the code more? maybe one method for removing one door by the showmaster?
usage of integer for doors - i think that's not so fancy...
overusage of brackets
too much comments (tell don't ask)
too complicated (brain overload)

Edit:

there exists an follow up question on how to do it better

.. review .. mainly on the aspects of OOP and CleanCode

To Some of Your Thoughts

  usage of integer for doors - i think that's not so fancy...

Additional to a Door class we could add classes for Player, ShowMaster, and MontyHall too.

  should I break up the code more? maybe one method for removing one door by the showmaster?

This job is be done when we split up the code with classes into different logical units.

  too much comments (tell don't ask)

Comments are add all not bad but when you group your code in a method you all ready see semi-independent logic. In Robert C. Martins words: “Don’t Use a Comment When You Can Use a Function or a Variable”

Review

Bad news first.. I see no oop at all in your code.. An indicator for non-oop code is the heavy use off static and no own data-types. With other words: You write a huge procedural algorithm.

Find Objects

My first step would be to wrap this method into a class called MontyHall and give it a public method caluclateChance. I would avoid the get-prefix because it is not a getter.

After that, we can search for cohesion and abstractions. Your comments helps us very well!

The comment //the player selects one of three doors shows us that we could create a Player. After that the ShowMaster wants //.. remove one goat (but not the selected door). Than our //player changes door if he wants to.

We should think again about a Door. Currently, the doors are represented as a list of integers. But in the domain of Monty Hill, a door is something that hides a price and has a number.

class Door {
    private int number;
    private Price price;

    // all-args constructor..

    // hashcode and equals-method..
}

Code Smell

Magic Number

The magic number 3 appears two times in your code:

random.nextInt(3)
if (..., currentDoor < 3; ..)

The 3 represents the number of doors and can be replaced by int NUMBER_OF_DOORS = doors.size(), or you can hide it in an object, which would be the preferred way.

Flag Argument

The method getChanceForPlay takes as second argument a Boolean value, which creates uses if (doChange) { /*...*/} to create a branch in your method. The downside of this flag is that we need to test each branch when writing a unit test (but ok.. in this we have only on possible branch).

This flag has a logical cohesion to a Player. Let's say a Player is an abstract data type and there are two possible types of players: StraightPlayer and ChangePlayer. With polymorphism we can pass both players and call player.chooseDoor. While the StraightPlayer chooses the same door as before and ChangePlayer changes his/her selection.

Code

// will follow