Sort Array By Parity The task is taken from leetcode

  Given an array A of non-negative integers, return an array consisting
  of all the even elements of A, followed by all the odd elements of A.

  You may return any answer array that satisfies this condition.

  Example 1:

  Input: [3,1,2,4]

  Output: [2,4,3,1]

  The outputs [4,2,3,1], [2,4,1,3], and [4,2,1,3] would also be
  accepted.

  Note:

  1 <= A.length <= 5000

  0 <= A[i] <= 5000

My functional solution 1

const sortArrayByParity = A => A.reduce((acc, x) => {
        if (x % 2 === 0) { acc.unshift(x); }
        else { acc.push(x); }
        return acc;
    }, []);

My functional solution 2

const sortArrayByParity = A => A.reduce((acc, x) => x % 2 === 0 ? [x, ...acc] : [...acc, x]);

My imperative solution

function sortArrayByParity2(A) {
  const odd = [];
  const even = [];
  for (const a of A) {
    if (a % 2 === 0) {
      even.push(a);
    } else {
      odd.push(a);
    }
  }
  return [...even, ...odd];
};

Bug

Your second functional solution does not run. You forgot to add the second argument to A.reduce. I will assume you wanted an array as the last argument.

Why functional sucks

This example clearly illustrates the problem with some functional solutions that involve data manipulation. The requirement of no side effects forces the solution to copy the whole dataset even when you manipulate only a single item. This is particularly noticeable in your second functional solution. See performance results below.

Code and Style

Some minor points...

function () {};  the trailing semicolon is not needed.
Swap the conditions.

You have if(a % 2 === 0) { /*even*/ } else { /*odd*/ } ...

can be if(a % 2) { /*odd*/ } else { /*even*/ }
Compact code. Try to avoid sprawling code. It may not matter for small segments of code, but source code can very long and reading code there spans pages is not as easy as having it all in one view.
Before a newline it is either } or ;. There are two exceptions. The } that closes an object literal should have a closing ; eg const a = {};. And multi line statements and expressions.

Know the language.

You do a lot of code examples, many of them are rather trivial. Of late many of your posts contain bugs or incomplete code (may be a sign of boredom? or a lack of challenge (engagement)) . I do not believe in the classical closed book assessment examination, it does not reflect the real world. However a good memory of the field makes you a more productive coder.

There are many subtle tricks in JavaScript that can catch you out if unaware. Testing your knowledge improves your understanding of the language making you a better coder.

This is a example JavaScript Web Development Quiz picked at random from a web search "javascript quiz"

It is good practice to do one of these every now and then.

I did not get 100%

Example A

Compacting the function.

function sortByParity(arr) {
    const odd = [], even = [];
    for (const val of arr) {
        if (val % 2) { odd.push(val) }
        else { even.push(val) }
    }
    return [...even, ...odd];
}

Performance

The second functional example was so slow I had to push the other best time down to the timer resolution cutoff 0.2ms or it would have taken forever to complete the test.

The functions as tested

function sortByParity_I1(A) {
    const odd = [], even = [];
    for (const a of A) {
        if (a % 2 === 0) { even.push(a) }
        else { odd.push(a) }
    }
    return [...even, ...odd];
}
const sortByParity_F2 = A => A.reduce((acc, x) => x % 2 === 0 ? [x, ...acc] : [...acc, x], []);
const sortByParity_F1 = A => A.reduce((acc, x) => {
        if (x % 2 === 0) { acc.unshift(x) }
        else { acc.push(x) }
        return acc;
    }, []);

Benchmarks

Mean time per call to the function in 1/1,000,000 second. OPS is operations per second. % is relative performance of best.

For array of 1000 random integers

sortByParity_I1:    20.709µs OPS  48,287 100%
sortByParity_F1:   133.933µs OPS   7,466  15%
sortByParity_F2: 3,514.830µs OPS     284   1%

For array of 100 random integers

sortByParity_I1:     2.049µs OPS 488,148 100%
sortByParity_F1:    10.005µs OPS  99,947  20%
sortByParity_F2:    46.679µs OPS  21,422   4%

Note that the results for the functional solution do not have a linear relationship with the size of the array.

Improving performance

I will drop the  slow functional and add an improved imperative function that pre allocates the result array. This avoids the overhead of allocation when you grow the arrays. As there is one array the overhead of joining the two arrays is avoided as well. This almost doubles the performance.

function sortByParity_I2(A) {
    const res = new Array(A.length);
    var top = res.length - 1, bot = 0;
    for (const a of A) { res[a % 2 ? top-- : bot ++] = a }
    return res;
}

10000 items
sortByParity_I2:   119.851µs OPS     8,343 100%
sortByParity_I1:   223.468µs OPS     4,474  54%
sortByParity_F1: 5,092.816µs OPS       196   2%

1000 items
sortByParity_I2:    13.094µs OPS    76,372 100%
sortByParity_I1:    23.731µs OPS    42,138  55%
sortByParity_F1:   123.381µs OPS     8,104  11%

100 items
sortByParity_I2:     0.900µs OPS 1,110,691 100%
sortByParity_I1:     2.245µs OPS   445,398  40%
sortByParity_F1:     9.520µs OPS   105,039   9%

Test on Win10 Chrome 73 CPU i7 1.5Ghz


Validating Support Tickets Being given the following code I wonder how can I make it better, without using variable valid, yet check ALL the cases and if at least one of them matches set the returned value to False

  def validate_ticketer_issue(self, ticketer_issue):
      """
      Validate ticket system issue for common rules
      :param ticketer_issue: (str) ticket system issue key
      :return: (bool) True if issue is successfuly validated
      """
      valid = True
      issues_not_allowed = ('Pool', 'Document', 'Declaration', 'Restriction', 'Pre-Check')
      issues_not_allowed_inprod = ('Analysis', 'Test')

      try:
          issue = self.service.issue(ticketer_issue)
          if len(issue.fields.versions) == 0:  # Affects Version/s
              valid = False
              print('{}: Affects Version/s field is empty'.format(ticketer_issue))

          if issue.fields.issuetype.name in issues_not_allowed:
              valid = False
              print('{}: According to working model commit is not'
                    ' allowed to the ticket system issue types'
                    '{}'.format(issue['key'], issues_not_allowed))

          if issue.fields.issuetype.name in issues_not_allowed_inprod:
              valid = False
              print('{}: According to working model commit'
                    ' to the ticket system {} issue types'
                    ' is not allowed in production code'.format(
                        issues_not_allowed_inprod, issue['key']))

          if issue.fields.issuetype.name == 'Fault':
              if not self.validate_fault_issue_summary(issue):
                  valid = False

      except ticketer.exceptions.TicketerError:
          valid = False
          print('{}: issue does NOT exist in the ticket system!'.format(ticketer_issue))

      if valid:
          print('{}: OK'.format(ticketer_issue))

      return valid

The code you show is explicit about what it does, easy to read and understand. I suppose you find the usage of the valid flag error-prone in case the logic gets deeper nested, or any sort of refactoring that might forget about that flag?

When you encounter an error, you are actually doing two things : setting the valid flag to False and printing an error. One simplification would be to make a list of error messages (no valid variable), and push error messages on it. You don't print anymore, and the list of errors becomes the valid flag.

def validation_function(item):
    errors = []

    try:
        if not test1:
            errors.append("Error on test 1 for item")
        if not test2:
            errors.append("Error on test 2 for item")
    except SomeException as e:
            errors.append("Exception on whatever for item")

    # change the return type to list of errors
    # reporting errors is responsability of caller.
    return errors

You could raise an exception from inside the validation_function but I see it as a low level validation routine which collects errors and returns them. The caller is responsible for handling (raise, print) the returned errors if any.

Also, your validate_ticketer_issue is actually harmonizing errors : it leads its own set of tests but also catches exceptions raised by functions it calls and translates that to a boolean + error messages. The above example preserves that behaviour which is all right as long as you are very specific about the exceptions you catch.

Finally you could see the function as a bunch of predicates with specific error messages run one after the other.

# List of predicates and messages. Predicates must have a
# harmonized api. Note: I'm using lambdas here but regular
# functions will work too.
PREDICATES = [
    (lambda item: some_logic_here, "Error on test 1 for item"),
    (lambda item: some_other_logic_here, "Error on test 2 for item")
]

def validation_function(item):
    errors = []

    try:
        for predicate, msg in PREDICATES:
            if not predicate(item):
                # here you have to harmonize you messages
                # so that they can be formatted using the
                # same argument.
                errors.append(msg.format(item=item))
    except SomeException as e:
            errors.append("Exception on whatever for item")

    # change the return type to list of errors
    # reporting errors is responsability of caller.
    return errors

This example might seem more scalable but for a small amount of predicates, it is probably overkill as understanding the code is not obvious, logic lives in other places, and some work has to be done to harmonize predicates and their messages.

These are the few ideas I have to improve the code so that it doesn't rely on the valid flag. The second step makes it perhaps more scalable at the expense of readability.


Writing 1.5 million rows in a file I am generating SQL statements (of MySQL format) and writing into a file to create 10,000 tables. Each table contains 5 columns with 1.5 million rows of data in each column. I generated these data by using regex and faker module. So, I have a loop of 10,000 (one for each create table statement) and within this loop, I have another loop of 1.5 million (to insert rows of data into each table created).

The code is running properly but is extremely slow. Is there any way to make this run considerably faster?

import rstr
import random
from faker import Faker
import time

fake = Faker()

name = ['Name', 'Person_Name', 'Customer_Name', 'Employee_Name', 'Contact_Name', 'CustName', 'EmpName']
ssn = ['SSN', 'Social_Security_Number', 'National_Identification_Number', 'NID', 'US_SSN', 'Social_Number', 'Social_Security_No', 'Social_Security_Num', 'Customer_Social_Security Number', 'Employee_Social_Security_Number']
address = ['Address', 'Customer_Location_Details', 'Employee_Address_Details', 'Address_Det.', 'Add_Line_1', 'Add_Line_2', 'Address_Line_1', 'Address_Line_2']
dob = ['DOB', 'Date_Of_Birth', 'Birth_Date', 'V_VLD_BRTH_DT', 'DAY_OF_BRTH', 'DT_OF_BIRTH']
phone = ['Phone_Number', 'Contact_Number', 'Office_Phone', 'Residential_phone', 'Contact_Phone']

ssn1='(?!000|.+0{4})(?:\d{9})'
ssn2='(?!000|.+0{4})(\d{3}-\d{2}-\d{4})'
sList=[ssn1,ssn2]

db1='^(0[1-9]|[12][0-9]|3[01])[/](0[1-9]|1[012])[/](19)\d\d$'
db2='^(19)\d\d$[.](0[1-9]|1[012])[.](0[1-9]|[12][0-9]|3[01])'
db3='^(19)\d\d$[-](0[1-9]|1[012])[-](0[1-9]|[12][0-9]|3[01])'
db4='^(0[1-9]|[12][0-9]|3[01])[-](0[1-9]|1[012])[-](19)\d\d$'
db5='^(0[1-9]|[12][0-9]|3[01])[.](0[1-9]|1[012])[.]\d\d$'
dbList=[db1,db2, db3, db4, db5]

p1='1[2-9][0-8][0-9][2-9][0-9]{2}[0-9]{4}'
p2='[2-9][0-8][0-9][2-9][0-9]{2}[0-9]{4}'
p3='001[2-9][0-8][0-9][2-9][0-9]{2}[0-9]{4}'
p4='[(][\d]{3}[)][ ]?[\d]{3}-[\d]{4}'
p5='\+1[2-9][0-8][0-9][2-9][0-9]{2}[0-9]{4}'
p6='(\d{3}\d{3}\d{4})'
p7='[2-9][0-8][0-9][-][2-9][0-9]{2}[-][0-9]{4}'
pList=[p1, p2, p3, p4, p5, p6, p7]

tables=10001
rows=1500001

with open("D:\Output.sql", "a") as text_file:
    count = 0

    for i in range(1,tables):
        a=[]
        b=[]
        j=str(i)
        x=f'Tab_{i}'

        #a.append(random.choice(name)+'_'+j)
        #a.append(random.choice(ssn)+'_'+j)
        #a.append(random.choice(address)+'_'+j)
        #a.append(random.choice(dob)+'_'+j)
        #a.append(random.choice(phone)+'_'+j)
        a.extend(f'{random.choice(n)}_{j}' for n in (name, ssn, address, dob, phone))

        text_file.write(f"create table Python_test.{x}({a[0]} VARCHAR(255),{a[1]} VARCHAR(255),{a[2]} VARCHAR(255),{a[3]} VARCHAR(255),{a[4]} VARCHAR(255));\n")

        start = time.time()
        for j in range(1,rows):
            text_file.write(f"insert into {x}({a[0]},{a[1]},{a[2]},{a[3]},{a[4]}) values ('{fake.name()}','{rstr.xeger(random.choice(sList))}','{fake.address()}','{rstr.xeger(random.choice(dbList))}','{rstr.xeger(random.choice(pList))}');\n")

        print(time.time() - start)
        text_file.write('\n')
        count+=1
        print(count)

Use open in a with

...so that you can be guaranteed of file closure regardless of any potential exceptions.

Use f-strings

So that something like this:

x='Tab_'+str(i)

turns into

x = f'Tab_{i}'

Computers are good at repetition

a.append(random.choice(name)+'_'+str(i))
a.append(random.choice(ssn)+'_'+str(i))
a.append(random.choice(address)+'_'+str(i))
a.append(random.choice(dob)+'_'+str(i))
a.append(random.choice(phone)+'_'+str(i))

can be

a.extend(f'{random.choice(n)}_{i}' for n in (name, ssn, address, dob, phone))

Injection attacks

Unless you're really confident about the source, safety and validity of your data, constructing a string and sending it off as executable SQL is enemy number one of database security. This is what prepared statements are for. It's unclear what flavour of SQL you need, so it's unclear which library you'll need for this.

Combined assignment/increment

count=count+1

can be

count += 1


Vertical & Horizontal Direction of a DataTable I wanted to have a DataTable with cells in direction that is top to bottom or bottom to top and left to right or right to left.

LR - LEFT to RIGHT
RL - RIGHT to LEFT
BT - BOTTOM to TOP
TB - TOP to BOTTOM

To do that, below is the method I created

    internal DataTable GetContainerLocationMap(int row, int column, string bayCode, string horizontalDirection, string verticalDirection)
    {
        var pageMap = new DataTable();

        if (horizontalDirection == "LR")
        {
            for (int i = 1; i <= column; i++)
            {
                pageMap.Columns.Add("C" + i);
            }
            if (verticalDirection == "BT")
            {
                for (int i = row; i > 0; --i)
                {
                    var _pageMapRow = pageMap.NewRow();
                    for (int j = 1; j <= column; j++)
                    {
                        _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
                    }
                    pageMap.Rows.Add(_pageMapRow);
                }
            }
            else if (verticalDirection == "TB")
            {
                for (int i = 1; i <= row; i++)
                {
                    var _pageMapRow = pageMap.NewRow();
                    for (int j = 1; j <= column; j++)
                    {
                        _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
                    }
                    pageMap.Rows.Add(_pageMapRow);
                }
            }
        }
        else if (horizontalDirection == "RL")
        {
            for (int i = column; i > 0; --i)
            {
                pageMap.Columns.Add("C" + i);
            }
            if (verticalDirection == "BT")
            {
                for (int i = row; i > 0; --i)
                {
                    var _pageMapRow = pageMap.NewRow();
                    for (int j = column; j > 0; --j)
                    {
                        _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
                    }
                    pageMap.Rows.Add(_pageMapRow);
                }
            }
            else if (verticalDirection == "TB")
            {
                for (int i = 1; i <= row; i++)
                {
                    var _pageMapRow = pageMap.NewRow();
                    for (int j = column; j > 0; --j)
                    {
                        _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
                    }
                    pageMap.Rows.Add(_pageMapRow);
                }
            }
        }

        return pageMap;
    }

If you call above method by these params

GetContainerLocationMap(2, 5, "A", "LR", "TB");

It would return DataTable like below:

A-1-1   A-2-1   A-3-1   A-4-1   A-5-1
A-1-2   A-2-2   A-3-2   A-4-2   A-5-2

I would like to know the code I have written is all okay or do I need to make it more simple?

Thanks in advance

I like that you have an actual method, instead of a bunch of code in Main. This makes is easier to reuse, segment changes, and test.

Passing in a string for horizontalDirection and verticalDirection is error prone. The caller could pass in the wrong value without realizing it. It also makes it harder to make changes with refactoring tools.

A better data type would be an enum. An example would be

enum HorizontalDirection
{
    LeftToRight,
    RightToLeft
}

and similar for the vertical direction.

In terms of making it simpler, the main thing that sticks out is you have essentially 4 nearly identical loops. Let's start by comparing the adding of rows. Between the LR and RL versions, the only difference is the direction the column j loop goes. The direction you go when setting all of the cell values doesn't matter in this case (because you are looking up the column with the same index you are setting in the value), so let's move the row adding loops outside the column adding loops.

Now our method looks like this:

internal DataTable GetContainerLocationMap(int row, int column, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
    var pageMap = new DataTable();

    if (horizontalDirection == HorizontalDirection.LeftToRight)
    {
        for (int i = 1; i <= column; i++)
        {
            pageMap.Columns.Add("C" + i);
        }
    }
    else if (horizontalDirection == HorizontalDirection.RightToLeft)
    {
        for (int i = column; i > 0; --i)
        {
            pageMap.Columns.Add("C" + i);
        }
    }

    if (verticalDirection == VerticalDirection.BottomToTop)
    {
        for (int i = row; i > 0; --i)
        {
            var _pageMapRow = pageMap.NewRow();
            for (int j = column; j > 0; --j)
            {
                _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
            }
            pageMap.Rows.Add(_pageMapRow);
        }
    }
    else if (verticalDirection == VerticalDirection.TopToBottom)
    {
        for (int i = 1; i <= row; i++)
        {
            var _pageMapRow = pageMap.NewRow();
            for (int j = column; j > 0; --j)
            {
                _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, i);
            }
            pageMap.Rows.Add(_pageMapRow);
        }
    }

    return pageMap;
}

Better, but there still seems to be some duplication. Let's try to condense the row adding loops.

Comapring BT against TB, the only difference is the start, end, and direction of the i loop. That means the inside of the loop is identical. Sound like a good place for another method?

internal DataTable GetContainerLocationMap(int row, int column, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
    var pageMap = new DataTable();

    if (horizontalDirection == HorizontalDirection.LeftToRight)
    {
        for (int i = 1; i <= column; i++)
        {
            pageMap.Columns.Add("C" + i);
        }
    }
    else if (horizontalDirection == HorizontalDirection.RightToLeft)
    {
        for (int i = column; i > 0; --i)
        {
            pageMap.Columns.Add("C" + i);
        }
    }

    if (verticalDirection == VerticalDirection.BottomToTop)
    {
        for (int i = row; i > 0; --i)
        {
            AddRowToContainerMap(i, column, bayCode, pageMap);
        }
    }
    else if (verticalDirection == VerticalDirection.TopToBottom)
    {
        for (int i = 1; i <= row; i++)
        {
            AddRowToContainerMap(i, column, bayCode, pageMap);
        }
    }

    return pageMap;
}

private static void AddRowToContainerMap(int currentRow, int column, string bayCode, DataTable pageMap)
{
    var _pageMapRow = pageMap.NewRow();
    for (int j = column; j > 0; --j)
    {
        _pageMapRow["C" + j] = string.Format("{0}-{1}-{2}", bayCode, j, currentRow);
    }
    pageMap.Rows.Add(_pageMapRow);
}

You could get really fancy and create variables for your loop index start, end, and increment conditions. I don't think it would help readability, but this is another option for condensing code:

int iStart;
int iEnd;
int iIncrement;

if (horizontalDirection == HorizontalDirection.LeftToRight)
{
    iStart = 1;
    iEnd = column + 1;
    iIncrement = 1;
}
else
{
    iStart = column;
    iEnd = 0;
    iIncrement = -1;
}

for (int i = iStart; i != iEnd; i += iIncrement)
{
    pageMap.Columns.Add("C" + i);
}

Comparing the readability of the two, I don't like this change.

At this point, you have two separate if blocks, each with their own for loop, and only one function inside. That's about as "simple" as I would try to make it.

Some style suggestions (the first two are the only ones I feel strongly about, the rest are very subjective):

Using an underscore in a variable name is something I expect for a member variable (field), and not a a local variable. I'd rename _pageMapRow to pageMapRow.
The variables row and column are somewhat ambiguous. At a first glance, I expected them to be an index. Turns out they are the total number of rows and columns to generate. Something like maxRow or totalRows would be clearer in my opinion.
Since we don't have any background to what your function is for, I don't know what bayCode means. If it is clear based on your context, leave the variable name as is. I presume it has to do with an identifier in a warehouse floor or similar.
C# 6 introduced string interpolation as an alternative to string.Format(). It's not necessarily more or less readable in this particular instance, but I like to share it as an option for other scenarios. If you had a longer string it may be useful to see the values as you read it. It also means you don't have to remember to change all of your indices as you add/remove parameters to string.Format.  You could set the cell values using this sytax:

pageMapRow["C" + j] = $"{bayCode}-{j}-{currentRow}";

internal is an unexpected access modifier. That means your method is available for any caller inside your assembly, but not outside. It certainly has its uses, but I tend to learn toward the broader public vs private.

Final look putting all of it together

public enum HorizontalDirection
{
    LeftToRight,
    RightToLeft
}

public enum VerticalDirection
{
    TopToBottom,
    BottomToTop
}

public DataTable GetContainerLocationMap(int totalRows, int totalColumns, string bayCode, HorizontalDirection horizontalDirection, VerticalDirection verticalDirection)
{
    var pageMap = new DataTable();

    if (horizontalDirection == HorizontalDirection.LeftToRight)
    {
        for (int i = 1; i <= totalColumns; i++)
        {
            pageMap.Columns.Add("C" + i);
        }
    }
    else if (horizontalDirection == HorizontalDirection.RightToLeft)
    {
        for (int i = totalColumns; i > 0; --i)
        {
            pageMap.Columns.Add("C" + i);
        }
    }

    if (verticalDirection == VerticalDirection.BottomToTop)
    {
        for (int i = totalRows; i > 0; --i)
        {
            AddRowToContainerMap(i, totalColumns, bayCode, pageMap);
        }
    }
    else if (verticalDirection == VerticalDirection.TopToBottom)
    {
        for (int i = 1; i <= totalRows; i++)
        {
            AddRowToContainerMap(i, totalColumns, bayCode, pageMap);
        }
    }

    return pageMap;
}

private static void AddRowToContainerMap(int currentRow, int column, string bayCode, DataTable pageMap)
{
    var pageMapRow = pageMap.NewRow();
    for (int j = column; j > 0; --j)
    {
        pageMapRow["C" + j] = $"{bayCode}-{j}-{currentRow}";
    }
    pageMap.Rows.Add(pageMapRow);
}


Find swapped nodes in BST A BST has two nodes swapped. Figure out which two nodes.
Looking for code-review, optimizations and best practices.

public class SwappedNodes {

    private TreeNode root;

    SwappedNodes(List<Integer> items) {
        create(items);
    }

    private void create (List<Integer> items) {
        if (items.isEmpty()) {
            throw new IllegalArgumentException("The item is empty");
        }
        root = new TreeNode(items.get(0));

        final Queue<TreeNode> queue = new LinkedList<TreeNode>();
        queue.add(root);

        final int half = items.size() / 2;

        for (int i = 0; i < half; i++) {
            if (items.get(i) != null) {
                final TreeNode current = queue.poll();
                int left = 2 * i + 1;
                int right = 2 * i + 2;

                if (items.get(left) != null) {
                    current.left = new TreeNode(items.get(left));
                    queue.add(current.left);
                }
                if (right < items.size() && items.get(right) != null) {
                    current.right = new TreeNode(items.get(right));
                    queue.add(current.right);
                }
            }
        }
    }

    private static class TreeNode {
        private TreeNode left;
        private int item;
        private TreeNode right;

        TreeNode (int item) {
            this.item = item;
        }
    }

    private static class PrevNode {
        private TreeNode prevNode;
        PrevNode (TreeNode prevNode) {
            this.prevNode = prevNode;
        }
    }

    /**
     * Returns the 2 nodes which if swapped results in binary search tree.
     */
    public int[] getSwappedNode( ) {
        final List<TreeNode> list = new ArrayList<>();
        computeSwappedNodes (root, new PrevNode(null), list);

        int[] result = {list.get(0).item, list.get(list.size() - 1).item};
        return result;
    }

    private void computeSwappedNodes (TreeNode node, PrevNode prevNode, List<TreeNode> list) {
        if (node == null) return;

        computeSwappedNodes (node.left, prevNode, list);
        if (prevNode.prevNode != null) {
            if (prevNode.prevNode.item > node.item) {
                list.add(prevNode.prevNode);
                list.add(node);
            }
        }

        prevNode.prevNode = node;

        computeSwappedNodes (node.right, prevNode, list);
    };
}

public class SwappedNodesTest {

    // both values in different branches
    @Test
    public void testDifferentBranches() {
        SwappedNodes sn1 = new SwappedNodes(Arrays.asList(10, 4, 25, 2, 20, 8, null));
        int[] a1 = {20, 8};
        assertArrayEquals(a1, sn1.getSwappedNode());
    }

    // parent child relationship
    @Test
    public void testParentChild() {
        SwappedNodes sn3 = new SwappedNodes(Arrays.asList(10, 2, 20, 5, 8, 15, 29));
        int[] a3 = {5, 2};
        assertArrayEquals(a3, sn3.getSwappedNode());
    }

    @Test
    public void testAncestor() {
        SwappedNodes sn4 = new SwappedNodes(Arrays.asList(8, 5, 20, 2, 10, 15, 29));
        int[] a4 = {10, 8};
        assertArrayEquals(a4, sn4.getSwappedNode());
    }

}

computeSwappedNodes (root, new PrevNode(null), list);

Unneeded space between function name and parentheses with arguments. You do this multiple times.

/**
 * Returns the 2 nodes which if swapped results in binary search tree.
 */
public int[] getSwappedNode( ) {

No, you return the items stored within. Not the nodes. You don't fulfill the assignment as stated. Additionally, you return multiple nodes, so name the method getSwappedNodes. Lastly, no need for a space in an empty set of parentheses.


How to refract a while loop with a necessary initial setting? I put the readable version into the gist file -> link

Please ignore the print and comment in the following code.

I often have to write this sort of logic: init the conditions, and deep into loop condition, get the variables in each iteration and break when the condition meets.

As you can see, I get the first line in a region and then loop until the region ends.

The exit condition is while True and (beginning_spaces - start_beginning_spaces).

Are there any better practices in my case?

 def get_exception_region(self, except_region, max_lines_in_exception=100):
        '''
        # get current line number
        row, _ = self.view.rowcol(except_region.a)
        # get next line's starting point
        next_row_starting = self.view.text_point(row + 1, 0)
        # get the whole next line
        next_row_region = self.view.full_line(next_row_starting)
        print(self.view.substr(next_row_region))
        '''

        # Init first setting
        try:

            row, _ = self.view.rowcol(except_region.a)
            next_line_text, next_line_region = self.get_next_line__text_and_region(row)
            prev_line_text, prev_line_region  = next_line_text, next_line_region
            beginning_spaces = self.count_the_beginning_spaces(next_line_text)
            start_row, start_beginning_spaces = row, beginning_spaces

            print "=================Start================="
            print row, beginning_spaces, next_line_text.strip()

            while True and (beginning_spaces - start_beginning_spaces) < max_lines_in_exception :
                row = row + 1
                next_line_text , next_line_region = self.get_next_line__text_and_region(row)
                beginning_spaces = self.count_the_beginning_spaces(next_line_text)
                print row, beginning_spaces, next_line_text.strip()
                if beginning_spaces > 1 and beginning_spaces != start_beginning_spaces:
                    print "=================Exit================="
                    break
                prev_line_text, prev_line_region  = next_line_text, next_line_region
            return row, prev_line_text, prev_line_region

            pass

        except Exception as e:
            exc_type, exc_obj, exc_tb = sys.exc_info()
            fname = os.path.split(exc_tb.tb_frame.f_code.co_filename)[1]
            print("[Error]\n", exc_type, fname, exc_tb.tb_lineno)

            raise e
            sys.exit(e)
            #EndOfExcept

I'm having trouble figuring out exactly what your code is doing. It's called get_exception_region but it appears to do a lot of printing. Is that just for debugging purposes? I'm going to assume that's the case, and thus that get_exception_region returns the row (line number?), text, and region of the line following the exception. Here are my suggestions, given that assumption:

Change the docstring to reflect what get_exception_region does, not how it does it. From another angle, describe why someone would choose to call get_exception_region, not what code they would have to write in its place. If there's any code in the docstring, it should show how to call the function, not reminders on functions it may call to implement itself. If you need the latter, comments are more appropriate for that.
If you have control over it, the name except_region.a doesn't convey much. Consider using a more descriptive name than a.
As mentioned by @ChrisWue, drop the True and from your while condition. At best it does nothing; at worst it distracts readers and slows the program.
Consider if rewriting your while loop as a for loop (for row in range(start_row, start_row + max_lines_in_exception): ...) would add or diminish clarity. This would avoid code with a slight smell (row = row + 1), but in this case I'm uncertain whether the replacement would be better.
Avoid catching Exception; if you must suppress most exceptions, it's typically better to catch BaseException (at least in python 3). That said, since this code quickly re-raises the exception, this advice is less crucial than usual.
Avoid accessing attributes like tb_frame, f_code, and co_filename in a function not specifically geared towards this. Instead for the usage in your exception handler, prefer using tools from traceback such as print_exc; in other cases inspect can be similarly useful.
Remove dead code. Having raise e followed by sys.exit(e) is misleading, since the raise will unconditionally prevent the exit from executing. Similarly the pass after return will never be reached.
If all you are really doing in this handler is printing out a partial trace, consider omitting the try/except handler entirely, and letting the caller be fully responsible.


Refactoring three property methods I am trying to refactor these three methods into one:

    public StatusReturn UpdateProperty(int id, int onlineReportId)
    {
        //Yes I realize I should not be using ref variables but the queryPermissions is an external dll.
        var webQueryInputProperty = new WebQueryInputProperty();
        //This line is different
        _queryPermissions.GetInputPropertyByKey(id, ref webQueryInputProperty);
        webQueryInputProperty.WQI_Key = onlineReportId;
        webQueryInputProperty.WQIP_Key = id;
        var webQueryInputProperties = new List<WebQueryInputProperty>{webQueryInputProperty};
        return _queryPermissions.UpdateWQInputProperties(webQueryInputProperties);
    }

    public StatusReturn AddProperty(int id, int onlineReportId)
    {
       var webQueryInputProperty = new WebQueryInputProperty();
        webQueryInputProperty.WQIP_Key = id;
        webQueryInputProperty.WQI_Key = onlineReportId;
        var webQueryInputProperties = new List<WebQueryInputProperty> { webQueryInputProperty };
        return _queryPermissions.UpdateWQInputProperties(webQueryInputProperties);
    }

    public StatusReturn DeleteProperty(int id, int onlineReportId)
    {
        var webQueryInputProperty = new WebQueryInputProperty();
        _QueryPermissions.GetInputPropertyByKey(id, ref webQueryInputProperty);
        webQueryInputProperty.WQI_Key = onlineReportId;
        webQueryInputProperty.WQIP_Key = id;
        var webQueryInputProperties = new List<WebQueryInputProperty> { webQueryInputProperty };
        //this line is different
        return _QueryPermissions.DeleteWebQueryInputProperties(webQueryInputProperties);
    }

I want to refactor the methods, remove the one line from AddProperty and change the return statement. I would rather not copy and paste.

I am trying to refactor these three methods into one

That would be, uh, de-factoring. I don't know of any merge methods refactoring - when people refactor code, they tend to extract functionality so as to have more methods (or classes and/or interfaces) that are also more simple, which means they do very little.
Taking 3 methods that do 3 different (albeit similar) things, and merging into one, would be a very bad idea. You have redundant code though, and that can be refactored.

Refactoring 1: Extract Method
All 3 methods have a dependency in common:

var webQueryInputProperty = new WebQueryInputProperty();

...and they all do the same thing with it (that would be even more obvious if the two properties were set in the same order in all 3 methods):

webQueryInputProperty.WQIP_Key = id;
webQueryInputProperty.WQI_Key = onlineReportId;
var webQueryInputProperties = new List<WebQueryInputProperty>{webQueryInputProperty};

There's this instruction, too, that the AddProperty method isn't calling:

_queryPermissions.GetInputPropertyByKey(id, ref webQueryInputProperty);

This means you could have a 4th, private method like this, whose job is to provide the other 3 methods with a list that contains a webQueryInputProperty object:
private IEnumerable<WebQueryInputProperty> GetWebQueryInputProperties(int id, int onlineReportId, bool isNewItem = false)
{
    var webQueryInputProperty = new WebQueryInputProperty();
    if (!isNewItem)
    {
        _queryPermissions.GetInputPropertyByKey(id, ref webQueryInputProperty);
    }

    webQueryInputProperty.WQIP_Key = id;
    webQueryInputProperty.WQI_Key = onlineReportId;

    return new List<WebQueryInputProperty> { webQueryInputProperty };
}

Notice the method is returning an IEnumerable<T>, not a List<T> - you could just as well return an inline array, like this:
    return new[] { webQueryInputProperty };

That is, if the 3rd-party library methods you're passing it to don't actually require a List<T>.
At this point you have this already:
public StatusReturn UpdateProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId);
    return _queryPermissions.UpdateWQInputProperties(properties);
}

public StatusReturn AddProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId, true);
    return _queryPermissions.UpdateWQInputProperties(properties);
}

public StatusReturn DeleteProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId);
    return _queryPermissions.DeleteWQInputProperties(properties);
}

That's nice, but the bool parameter is sticking out like a sore thumb. It's begging for an abstraction:
private enum DataOperation
{
    Update,
    Add,
    Delete
}

And then:
private IEnumerable<WebQueryInputProperty> GetWebQueryInputProperties(int id, int onlineReportId, DataOperation operation)
{
    var webQueryInputProperty = new WebQueryInputProperty();
    if (operation == DataOperation.Add)
    {
        _queryPermissions.GetInputPropertyByKey(id, ref webQueryInputProperty);
    }

    ...

Which turns the call sites into this:
public StatusReturn UpdateProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId, DataOperation.Update);
    return _queryPermissions.UpdateWQInputProperties(properties);
}

public StatusReturn AddProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId, DataOperation.Add);
    return _queryPermissions.UpdateWQInputProperties(properties);
}

public StatusReturn DeleteProperty(int id, int onlineReportId)
{
    var properties = GetWebQueryInputProperties(id, onlineReportId, DataOperation.Delete);
    return _queryPermissions.DeleteWQInputProperties(properties);
}

Refactoring 2: Rename
WQI_Key and WQIP_Key are bad names. Don't use underscores in the middle of an identifier, and don't use abbreviations. If WQI_Key is supposed to be a report ID, why not call it ReportId?
If the class this code is written is is cohesive, then you can drop the Property suffix (the type name says Property too, right?) and have Add, Update and Delete methods instead of AddProperty, UpdateProperty and DeleteProperty.
If it's not cohesive, then you have an AddXxxx scheme and you probably have a whole other class to extract.
you're way too verbose here:

var webQueryInputProperty = new WebQueryInputProperty();

I would have had this:
var property = new WebQueryInputProperty();

I suspect _QueryPermissions is a typo; the leading underscore makes me believe it's referring to a private or private readonly field (in which case the correct naming is with the lowercase q).

Lastly, StatusReturn is a bad name for a type - literally anything with a Return suffix is a bad name for a type.
Actually, it's probably a bad idea to even have to deal with that type in the first place, let alone to spread it into your own code.
Instead of leaving it to the client code to verify the status and - I bet - make sure no error occurred and that the method call was successful, you could have a private static method to analyze a StatusReturn, and throw an exception in case of a non-successful method call.
Assuming ReturnResult is an enum (and that you create custom exception types as needed), it could look something like this (don't default to success!):
private static void ValidateResultOrThrow(ReturnResult result)
{
    switch (result)
    {
        case ReturnResult.Success:
            return;

        //case ReturnResult.ConnectionError:
        //    throw new NoNetworkException("Unable to connect.");

        //case ReturnResult.InvalidIdError:
        //    throw new ArgumentException("Specified id doesn't exist.");

        case default:
            throw new UnknownResultException("Something bad happened.");
    }
}

And then your methods could return void, and either succeed or throw - which is probably what the client code would be expecting - and you keep the ReturnResult ugliness nicely abstracted away:
public void Update(int id, int reportId)
{
    var properties = GetWebQueryInputProperties(id, reportId, DataOperation.Update);
    var result = _queryPermissions.UpdateWQInputProperties(properties);
    ValidateResultOrThrow(result);
}

public void Add(int id, int reportId)
{
    var properties = GetWebQueryInputProperties(id, reportId, DataOperation.Add);
    var result = _queryPermissions.UpdateWQInputProperties(properties);
    ValidateResultOrThrow(result);
}

public void Delete(int id, int reportId)
{
    var properties = GetWebQueryInputProperties(id, reportId, DataOperation.Delete);
    var result = _queryPermissions.DeleteWQInputProperties(properties);
    ValidateResultOrThrow(result);
}

Hmm.. Perhaps the GetWebQueryInputProperties calls would read better if the DataOperation parameter was first. A reorder parameters (or change signature) refactoring would fix that.


User form validation using a POPO I'm trying to sketch out a stripped down generic form validation example.  I started by creating Plain Old PHP Objects (referred as POPOs) for form input with some classes such as a validator and html renderer class, that I then reduced to explicit functions.

The POPOs are useful in as much as you can comment and glance at expected properties.  Looking at the code, one starts to wonder if I'd be better off with well defined arrays as inputs and not bother with the plain objects, as some of the object wrangling (the popo functions) are effectively just wrappers for array functions, and perhaps an array would just do.

Are these plain objects muddying the waters?

Any comments welcome.

<?php
class UserForm
{
    public $email;
}

class UserFormErrors extends UserForm
{}

function map_array_to_popo(array $array, $object)
{
    foreach($object as $key => $value)
        $object->$key = isset($array[$key])
            ? $array[$key]
            : null;

    return $object;
}

function popo_has_empty_values($object)
{
    $array = (array) $object;

    return empty(array_filter($array));
}

function user_form_validate(UserForm $input)
{
    $errors = new UserFormErrors;
    if($input->email == '') {
        $errors->email[] = 'Email address is required.';
    } elseif(filter_var($input->email, FILTER_VALIDATE_EMAIL) === false) {
        $errors->email[] = 'Email address must be valid.';
    }

    return $errors;
}

function user_form_html_render(
        $action = '',
        UserForm $values,
        UserFormErrors $errors
    )
{
    $escape = function($string) {
        return htmlspecialchars($string);
    };
    $error = function($messages) use ($escape) {
        return
            empty($messages)
            ? ''
            : '

' .
            implode('

', array_map($escape, $messages)) .
            '

';
    };
    ?>
    <form method='POST' action="<?= $action ?>">
        Email address:
        <input type="text" name="email" id="email" value=
        "<?= $escape($values->email) ?>">
        <?= $error($errors->email) ?>
        <input type="submit">
    </form>
    <?php
}

if($_SERVER['REQUEST_METHOD'] == 'POST') {
    $input   = array_filter($_POST, 'is_string');
    $input   = array_map('trim', $input);
    $values  = map_array_to_popo($input, new UserForm);
    $errors  = user_form_validate($values);
    $valid   = popo_has_empty_values($errors);

    if($valid) {
        // Form values look good, do what you want with values.
        echo 'Email address entered is: ' . $values->email;
    } else {
        // Display form with data and errors
        user_form_html_render('', $values, $errors);
    }
} else {
    user_form_html_render('', new UserForm, new UserFormErrors);
}

Personally I don't see any value in introducing such objects. Especially given you are inclined to functional programming, which is a real trademark of your code, whereas such "POPOs" indeed look alien in it.

Either way, there is one plain mistake in the code, an Error by no means is a Form, so UserForm never can be a parent for the UserFormErrors. Even for sake of sparing you from writing a couple lines of code.


Bubble Sort Implementation in Python 3 I have implemented a Bubble Sort. It works well. But if you think something needs to be improved, say it. This code was tested in Python 3.7.4.

def bubble_sort(nums):

    for i in range(len(nums)-1):
        for j in range(0,len(nums)-1-i,1):
            if nums[j] > nums[j+1]:
                swap(nums, j, j+1)

    return nums

def swap(nums, i, j):
    temp = nums[i]
    nums[i] = nums[j]
    nums[j] = temp

if __name__ == "__main__":

   a = [0,0,0,-1,-0,1,2,3,2,1]
   print(bubble_sort(a))

Unnecessary Function

Your swap function is unnecessary. Simply replace the function call with this line:

nums[j], nums[j + 1] = nums[j + 1], nums[j]

This does the swapping for you.

Spacing

There should be spaces between values in lists

[1, 2, 3, 4, 5]

between numbers/strings and operators

if nums[j] > nums[j + 1]

and between parameters in a function call

for j in range(0, len(nums) - 1 - i, 1):

Type Hints

Your function header can look like this:

from typing import List, Union

def bubble_sort(nums: List[Union[int, float]]) -> List[Union[int, float]]:

What this says is that the function accepts a list of integers/floats, and returns a list of integers/floats. It adds another layer of descriptiveness to your code.

Docstrings

You should include a docstring at the beginning of every class/method/module you write. This allows you to describe in words what your code is doing.

def bubble_sort(nums: List[Union[int, float]]) -> List[Union[int, float]]:
    """
    A bubble sort algorithm, etc etc etc

    :param nums -> List: A list of integers/floats to sort

    :return List: The sorted list of integers/floats, from smallest -> biggest
    """


Create new UIImage each time or once I am writing a control that has a toggle on (show/hide password). The UITextfield has a UIImageView in that acts as the toggle.

The question is, is it better to create the state images once and store them in the control, or to create them on demand?

Essentially this:

if let imageView = imgToggle {
    if self.isSecureTextEntry {
        imageView.image = UIImage(named: "eye.png")
    } else {
        imageView.image = UIImage(named: "no_eye.png")
    }
}

vs this:

private var showPassword: UIImage? = UIImage(named: "eye.png")
private var hidePassword: UIImage? = UIImage(named: "no_eye.png")

///...

if let imageView = imgToggle {
    if self.isSecureTextEntry {
        imageView.image = showPassword
    } else {
        imageView.image = hidePassword
    }
}

Is there any overhead in constantly recreating the image? How does that compare to storing the image in the control?

There is no overhead in calling UIImage(named: ...) repeatedly,
because the UIKit framework already caches the image, as documented
in the UIImage(named:) reference:

  This method looks in the system caches for an image object with the specified name and returns the variant of that image that is best suited for the main screen. If a matching image object is not already in the cache, this method locates and loads the image data from disk or from an available asset catalog, and then returns the resulting object.

So keep it simple and assign the image object where needed:

if self.isSecureTextEntry {
    imageView.image = UIImage(named: "eye.png")
} else {
    imageView.image = UIImage(named: "no_eye.png")
}

or shorter:

imageView.image = UIImage(named: isSecureTextEntry ? "eye.png" : "no_eye.png")

Note that self is not necessary to access instance members.

Or use image literals, a feature introduced in Xcode 8.
You can simply type the name of an image resource in the asset catalog. This will be autocompleted to #imageLiteral(resourceName: "<name>"), but displayed as the image itself

in the Xcode source editor.

This is useful to avoid typing errors in the image name, and to visually
check if the right image was chosen. You can even double-click on
the image icon and choose a different one.


Set the correct server request path Can this if-else statement be simplified and/or cleaned up?

var path = require('path')
var express = require('express')
var app = express()
app.get('/*', function (req, res) {
  var page;
  if (req.params[0] === '') {
    // if requesting root dir, add `index.html`
    page = path.parse(path.join(__dirname, 'index.html'))
  } else {
    // otherwise add requested path
    page = path.parse(path.join(__dirname, req.params[0]))
    if (page.ext === '') {
      // if requested page has no extension, change page, adding `index.html`
      page = path.parse(path.join(__dirname, req.params[0], 'index.html'))
    }
  }
  if (page.ext === '.html') {
    /*... do html stuff ...*/
  }
  else if (page.ext === '.css') {
    /*... do css stuff ...*/
  }
  /*...*/
})

It works correctly but I feel it’s "bad coding". There are nested if statements and I’m repeating a lot of code. Also, below the first condition, there are other conditions testing the type of page (HTML, CSS, etc.) Since in the page.ext==='' condition, the value of page may change, it's not a mutually exclusive case of the other conditions.

It’s weird and feels wrong, but I can’t quite explain why.

I tried using the ternary expression (?:), but I still can’t avoid the if statement afterward.

var page = path.parse(path.join(__dirname, (req.params[0] === '') ? 'index.html' : req.params[0]))
if (page.ext === '') page = path.parse(path.join(__dirname, req.params[0], 'index.html'))

Is there a way to do it without repeating code? I’d like to avoid assigning page more than once.

+++
I’ve come up with a less verbose way of doing it, and it only requires assigning page once. Though it still requires calling path.parse() more than once.

var filepath = path.join(__dirname, req.params[0] || 'index.html')
var page = path.parse(path.join(filepath, (path.parse(filepath).ext === '') ? 'index.html' : ''))

+++
After discovering path.extname(<String>), I realized I don’t need to parse it beforehand. So this simplifies to:

var filepath = path.join(__dirname, req.params[0] || 'index.html')
var page = path.parse(path.join(filepath, (path.extname(filepath) === '') ? 'index.html' : ''))

Perhaps consider building an array of path segments through your logic and then setting page with it.

app.get('/*', function (req, res) {
  var page
  var base = req.params[0]
  var baseExtension = path.parse(base).ext
  // determine path segments
  var pathSegments = [__dirname]
  if (base !== '') {
    pathSegments.push(base)
  }
  if (baseExtension === '') {
    pathSegments.push('index.html')
  }

  // set page
  // here we use spread operator
  page = path.parse(path.join(...pathSegments))
  // if not using ES6, you could alternatively use
  // page = path.parse(path.join.apply(pathSegments))

  // perhaps consider switch here instead of if-else if you are going
  // to have more than just html and css extension types
  if (page.ext === '.html') {
    /*... do html stuff ...*/
  }
  else if (page.ext === '.css') {
    /*... do css stuff ...*/
  }
  /*...*/
})

Be consistent on your usage or non-usage of semicolons to end lines of code.  It seems like you have chosen to use the no semi-colon style, so why is there one after var page;?


Select row based on another table row index I have 2 tables. One holds only first column of my "data table" rest is stored in another table, that is placed right to first. What I need to do is to get all the data from specific row to pass it to jqplot.

My tables look like this. My code basically works, but I think it can be improved.

$('table#baseTable  > tbody > tr > td').click(function() {
    var rowIndex = $(this).parent().index();
    $('div#log').html(rowIndex);

    var myData = [];

    $('#dataTable tbody tr:eq(' + rowIndex + ')').map(function() {
            return $(this.cells).get();
        }).each(function() {
    var headerVal = $(this).closest("table").find("thead > tr > th").eq($(this).index()).html();
    myData.push([headerVal, $(this).html()]);
})

    alert(myData);
    console.log(myData);
});

I'm using this code in ASP page, so that every time I use UpdatePanel I must call my functions again.

This is my plot function:

function plot() {
    var $plot;
    var dataTableRows = $('#Grid3_br tbody tr');

    $('td.akcje').on('click', function() {
        var $newTitle = $(this).next().text();
        var myData = [],
            element = $(this),
            rowIndex = element.parent().index();

        $(dataTableRows[rowIndex]).map(function() {
            return $(this.cells).get();
        }).each(function(index, value) {
            myData.push(['M' + index, parseFloat($(this).html())]);
        })
        $('#wykres1').empty();//clears old plot. Don't know why re-plotting don't work for me
        $plot = $.jqplot('wykres1', [myData], {
            title: $newTitle,
            axes: {
                xaxis: {
                    renderer: $.jqplot.CategoryAxisRenderer,
                    tickOptions: {
                        formatString: '%s'
                    }
                },
                yaxis: {
                    tickOptions: {
                        formatString: '%.2f zł'
                    }
                }
            },
            highlighter: {
                show: true,
                sizeAdjust: 7.5,
                tooltipAxes: 'y'
            },
            cursor: {
                show: false
            }
        });

        $(window).resize(function() {
            $plot.replot({
                resetAxes: true
            });
        });
    });
}

I'll start with your selectors:

First off, you should never have a selector with anything left of an id - searches for ids are about the fastest you can get on the DOM.

Second, if you don't want to wrap other tables in your tables (I assume you don't), get rid of the immediate child stuff (P > C). Searching by tag is also pretty fast, so we use the id-element as our base for that.

$('#baseTable tbody td').click(function() {
    var rowIndex = $(this).parent().index();
    $('#log').html(rowIndex);

    var myData= [];

    $($('#dataTable tbody tr')[rowIndex]).map(function() {
        return $(this.cells).get();
    }).each(function() {
        var headerVal = $(this).closest('table').find('thead th')
            .eq($(this).index()).html();
        myData.push([headerVal,parseFloat($(this).html())]);
    })

    alert(myData);
    console.dir(myData);
});

Next, you should cache elements you found, use more than once and which don't change. And usually, it's a bad choice to bind events to the elements directly. I'm assuming jQuery 1.7 is ok and you don't really want to use jQuery 1.4, so we can use on for event binding.

var logDiv = $('#log'),
    dataTableRows = $('#dataTable tbody tr');

$('#baseTable tbody').on('click', 'td', function() {
    var myData = [],
        element = $(this),
        rowIndex = element.parent().index();

    logDiv.html(rowIndex);

    $(dataTableRows[rowIndex]).map(function() {
        return $(this.cells).get();
    }).each(function() {
        var headerVal = $(this).closest('table').find('thead th')
            .eq($(this).index()).html();
        myData.push([headerVal,parseFloat($(this).html())]);
    })

    alert(myData);
    console.dir(myData);
});

Next, accessing the DOM is rather slow. If you need the data for jqplot, do you need the table headers and can the data change?

Anyway, I'd store all data in an array beforehand. Now it looks like this:

var logDiv = $('#log'),
    dataTable = $('#dataTable'),
    dataTableHeaders = [],
    dataTableData = [];

// cache data table headers
dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
});

// cache full row (including headers) for data table
dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
        rowData[2 * col] = dataTableHeaders[col];
        rowData[2 * col + 1] = parseFloat($(this).html());
    });
});

// the onClick - but on tr instead of td and with a fast lookup
$('#baseTable tbody').on('click', 'tr', function() {
    var rowIndex = $(this).index(),
        myData = dataTableData[rowIndex];
    logDiv.html(rowIndex);
    alert(myData);
    console.dir(myData);
});

I still don't like the need for index(). I like it even less than event binding on elements, so I'll compromise:

var logDiv = $('#log'),
    dataTable = $('#dataTable'),
    dataTableHeaders = [],
    dataTableData = [];

dataTable.find('thead th').each(function(col) {
    dataTableHeaders[col] = $(this).html();
});

dataTable.find('tbody tr').each(function(row) {
    var rowData = (dataTableData[row] = []);
    $(this).children().each(function(col) {
        rowData[2 * col] = dataTableHeaders[col];
        rowData[2 * col + 1] = parseFloat($(this).html());
    });
});

$('#baseTable tbody tr').each(function(row) {
    $(this).on('click', function() {
        var data = dataTableData[row];
        logDiv.html(row);
        alert(data);
        console.dir(data);
    });
});

Here's my last step (this is not really needed, but I like it): we isolate this code from the outside world and make accidental messups a lot harder. And we enable a recalculation and rebinding of the click event if anything changes (number of rows / cols or the content).

var tableData = (function(){
    var logDiv = $('#log'),
        cacheRows = function() {
            var dataTable = $('#dataTable'),
                dataTableHeaders = [],
                dataTableData = [];
            // fetch headers
            dataTable.find('thead th').each(function(col) {
                dataTableHeaders[col] = $(this).html();
            });
            // fetch column data, prepare row arrays
            dataTable.find('tbody tr').each(function(row) {
                var rowData = (dataTableData[row] = []);
                $(this).children().each(function(col) {
                    rowData[2 * col] = dataTableHeaders[col];
                    rowData[2 * col + 1] = parseFloat($(this).html());
                });
            });
            // return row arrays
            return dataTableData;
        },
        bindOnClick = function() {
            var dataTableData = cacheRows();
            $('#baseTable tbody tr').each(function(row) {
                $(this).on('click', function() {
                    var data = dataTableData[row];
                    logDiv.html(row);
                    alert(data);
                    console.dir(data);
                });
            });
        };
    bindOnClick();
    return {
        refresh: bindOnClick
    };
})();

Now, when the tables change, you can preserve the intended behavior by calling tableData.refresh(). This is still a little messy because we never call off, but as we only use jQuery to change the DOM, we rely on it to unbind event handlers if we change elements. It's probably better to change the returned object so it has two functions: one to refresh the cache and one to rebind. rebinding is needed when the whole table was rebuilt and no DOM node is reused. Otherwise, this leaks event handlers. If you update the table infrequently, you should be fine. For frequent updates, I'd change it back to using the index function again.

Edit: Here's an example for what I described in my comment below.

var prepareForPlotting = function(baseSelector, dataSelector, clickhandler) {
    var cacheRows = function() {
            var dataTable = $(dataSelector),
                dataTableHeaders = [],
                dataTableData = [];
            // fetch headers
            dataTable.find('thead th').each(function(col) {
                dataTableHeaders[col] = $(this).html();
            });
            // fetch column data, prepare row arrays
            dataTable.find('tbody tr').each(function(row) {
                var rowData = (dataTableData[row] = []);
                $(this).children().each(function(col) {
                    rowData[2 * col] = dataTableHeaders[col];
                    rowData[2 * col + 1] = parseFloat($(this).html());
                });
            });
            // return row arrays
            return dataTableData;
        },
        bindOnClick = function() {
            var dataTableData = cacheRows();
            $(baseSelector + ' tbody tr').each(function(row) {
                $(this).on('click', function() {
                    clickhandler(row, dataTableData[row]);
                });
            });
        };
    bindOnClick();
    return {
        recache: cacheRows,
        rebind: bindOnClick
    };
}

var logDiv = $('#log'),
    baseHandler = prepareForPlotting('#baseTable', '#dataTable',
        function(row, data) {
            logDiv.html(row);
            alert(data);
            console.dir(data);
        }
    );


Using app.use only in routes that start with /admin I have several routes, and I need Facebook authentication only in the routes that start with /admin:

app.get('/admin/dashboard', passport.ensureAuthenticated, .....);
app.get('/admin/dashboard/new/email', passport.ensureAuthenticated, ....);
app.get('/admin/dashboard/new/question', passport.ensureAuthenticated, ....);
app.get('/admin/dashboard/new/answer', passport.ensureAuthenticated, ....);
app.get('/admin/dashboard/new/etc', passport.ensureAuthenticated, ....);

If I can't use the app.use, is there anyone to call passport.ensureAuthenticated repetition in routes that start with /admin?

From Express docs:

Path

// will match paths starting with /abcd
app.use('/abcd', function (req, res, next) {
  next();
})

Path Pattern

// will match paths starting with /abcd and /abd
app.use('/abc?d', function (req, res, next) {
  next();
})

// will match paths starting with /abcd, /abbcd, /abbbbbcd and so on
app.use('/ab+cd', function (req, res, next) {
  next();
})

// will match paths starting with /abcd, /abxcd, /abFOOcd, /abbArcd and so on
app.use('/ab*cd', function (req, res, next) {
  next();
})

// will match paths starting with /ad and /abcd
app.use('/a(bc)?d', function (req, res, next) {
  next();
})

Regular Expression

// will match paths starting with /abc and /xyz
app.use(/\/abc|\/xyz/, function (req, res, next) {
  next();
})

Array

// will match paths starting with /abcd, /xyza, /lmn, and /pqr
app.use(['/abcd', '/xyza', /\/lmn|\/pqr/], function (req, res, next) {
  next();
})


Monty Hall in BBC BASIC and Python How would I be able to improve the speed of this Monty Hall program?
 Interestingly, the same code written using BBC BASIC for Windows completes the task in half the time of the Python code.

Python:

import random

t = 10000001
j = 0
k = 0

for a in range(1, t):
    p = int(random.random() * 3) + 1
    g = int(random.random() * 3) + 1

    if p == g:
        r = int(random.random() * 2) + 1
        if p == 1:
            r += 1
        if p == 2 and r == 2:
            r = 3
    else:
        r = p ^ g
    s = g
    f = g ^ r
    if s == p:
        j = j + 1
    if f == p:
        k = k + 1

print(f"After a total of {t - 1} trials,")
print(f"The 'sticker' won {j} times ({int(j/t*100)}%)")
print(f"The 'swapper' won {k} times ({int(k/t*100)}%)")

BBC BASIC for Windows:

T% = 10000000

for A% = 1 to T%
  P% = rnd(3)
  G% = rnd(3)
  if P% = G% then

    R% = rnd(2)
    if P% = 1 then R% += 1
    if P% = 2 and R% = 2 then R% = 3
  else
    R% = P% eor G%
  endif
  S% = G%
  F% = G% eor R%
  if S% = P% then J% = J% + 1
  if F% = P% then K% = K% + 1
next

print "After a total of ";T%;" trials,"
print "The 'sticker' won ";J%;" times (";int(J%/T%*100);"%)"
print "The 'swapper' won ";K%;" times (";int(K%/T%*100);"%)"

So this is not the typical code review, as the code in question is pretty basic in terms of what it does. It is a given algorithm, and you need to do the given operations. The one possible varying factor is the random.random() lines. Can this affects times?

Let us test the random generation of some numbers using various random generators:

import random
import numpy
import timeit

t = 100001
def org_random_random(runs=t):
  for a in range(t):
    int(random.random() * 3) + 1

def org_random_randint(runs=t):
  for a in range(t):
    random.randint(1, 3)

def numpy_random_randint(runs=t):
  for a in range(t):
    numpy.random.randint(1, 3)

def numpy_random_randint_x(runs=t):
  numpy.random.randint(1, 3, runs)

orr = timeit.timeit(org_random_random, number=5)
ori = timeit.timeit(org_random_randint, number=5)
nrr = timeit.timeit(numpy_random_randint, number=5)
nrx = timeit.timeit(numpy_random_randint_x, number=5)

Sorry for terrible naming, it's just something I threw together, and when I ran it using https://repl.it/languages/Python3, it gave me the following output:

org_random_random     : 0.18020408600023075
org_random_randint    : 0.8966416030016262
numpy_random_randint  : 0.9019350050002686
numpy_random_randint_x: 0.004406830999869271

All three variants has been 'normalised' to produce a number between 1 and 3, and the last variant generates all of the numbers in one go. And the last one is way faster than the other.

In other words, I would suggest to pre-build the random numbers, and make a custom generator feeding you from this pre-built array of random numbers. This would potentially drastically reduce the running time of your algorithm.

Using a prebuilt random number array

Here is a simple rebuild using a pre-built array of random numbers from numpy:

runs = 10000001
random_numbers = numpy.random.randint(1, 3, runs + 6)

j = 0
k = 0

for i in range(1, runs):
    p = random_numbers[i]
    g = random_numbers[i+1]

    if p == g:
        r = random_numbers[i+2] % 2 + 1
        if p == 1:
            r += 1
        if p == 2 and r == 2:
            r = 3
    else:
        r = p ^ g
    s = g
    f = g ^ r
    if s == p:
        j = j + 1
    if f == p:
        k = k + 1

print(f"After a total of {t - 1} trials,")
print(f"The 'sticker' won {j} times ({int(j/t*100)}%)")
print(f"The 'swapper' won {k} times ({int(k/t*100)}%)")

When timed (without the print statements), this ran about 33% faster than the original. It does kind of reuse the random numbers slightly, but it shouldn't affect the overall randomness too much, I think.

Other possible factors

Just for the fun of it, I also tried exchanging the range() with a while a < runs loop, but that only had a minor effect for the larger number of runs. NB! If you was using Python 2.x, there would most likely be a massive effect out of changing range() into xrange().

The if s == p could be eliminated, and the j += 1 could be moved into the p == g block. This also, has little to no effect in my tests.

Lastly, using various python implementation will also effect timings. Using PyPy, IPython, C-Python(?), or other variations could change the time usage. I've used https://repl.it/languages/Python3, which reports the following on sys.version():

3.6.1 (default, Apr 26 2017, 20:23:36)
[GCC 4.9.2]

I also tried with an actual generator with both the large prebuilt list of random numbers, and a loopable generator with 1013 random numbers. Those had similar run times to the original code.

So, with the exception of making a python wrapper and implement the algorithm in another language or re-build the algorithm, I would be surprised to see a faster implementation. But I'll happily be proven wrong!


Shortening Oracle SQL containing several unions I believe the provided code can be rewritten shorter. Now one block is basically copy-pasted. In reality I have 6 such blocks.
This would be the data:
WITH
x(id, yr, more_info) AS
    (SELECT 1, 2020, 'info1' FROM DUAL UNION
     SELECT 2, 2021, 'info2' FROM DUAL UNION
     SELECT 3, 2021, 'info3' FROM DUAL UNION
     SELECT 4, 2020, 'info4' FROM DUAL )
,
n(n_id, n_id2, n_year) AS
    (SELECT 1, 3, 2021 FROM DUAL UNION
     SELECT 2, 4, 2021 FROM DUAL )

Desired result - complemented table x with up-to-date column yr values and info taken from previous year:

Current code:
WITH
x(id, yr, more_info) AS
    (SELECT 1, 2020, 'info1' FROM DUAL UNION
     SELECT 2, 2021, 'info2' FROM DUAL UNION
     SELECT 3, 2021, 'info3' FROM DUAL UNION
     SELECT 4, 2020, 'info4' FROM DUAL )
,
n(n_id, n_id2, n_year) AS
    (SELECT 1, 3, 2021 FROM DUAL UNION
     SELECT 2, 4, 2021 FROM DUAL )

SELECT * FROM x

-- ========>  BLOCK_1 for column n_id  <=========
UNION
SELECT n_id, n_year, more_info
FROM (SELECT *
      FROM x
      WHERE (id, yr) IN (SELECT id, MAX(yr)
                         FROM x
                         GROUP BY id) ) t1,
     (SELECT n_id, n_year
      FROM n
      WHERE (n_id, n_year) NOT IN (SELECT id, yr
                                   FROM x) ) t2
WHERE t1.id = t2.n_id

-- ========>  BLOCK_2 for column n_id2  <========
UNION
SELECT n_id2, n_year, more_info
FROM (SELECT *
      FROM x
      WHERE (id, yr) IN (SELECT id, MAX(yr)
                         FROM x
                         GROUP BY id) ) t1,
     (SELECT n_id2, n_year
      FROM n
      WHERE (n_id2, n_year) NOT IN (SELECT id, yr
                                   FROM x) ) t2
WHERE t1.id = t2.n_id2

Found a possibility employing UNPIVOT
WITH
x(id, yr, more_info) AS
    (SELECT 1, 2020, 'info1' FROM DUAL UNION
     SELECT 2, 2021, 'info2' FROM DUAL UNION
     SELECT 3, 2021, 'info3' FROM DUAL UNION
     SELECT 4, 2020, 'info4' FROM DUAL )
,
n(n_id, n_id2, n_year) AS
    (SELECT 1, 3, 2021 FROM DUAL UNION
     SELECT 2, 4, 2021 FROM DUAL )

SELECT * FROM x
UNION
SELECT id2, n_year, more_info
FROM (SELECT *
      FROM x
      WHERE (id, yr) IN (SELECT id, MAX(yr)
                         FROM x
                         GROUP BY id) ) t1,
     (SELECT * FROM n
      UNPIVOT(
          id2
          FOR id_type
          IN (n_id, n_id2) ) ) t2
WHERE t1.id = t2.id2


Using JavaScript's Array.splice with assigning the replacements item as an array I've got to delete / replace elements in an array.
For accomplishing these task one usually uses Array.splice().

Problem: Array.splice() expects the replacement items as a comma separated list. But I got the values as an array. I've made this
helper function which makes it possible to use splice with an array instead of single values.

    // Deletes elements in an array and
    // replaces the elements with other
    // values.
    // -- Parameter -------------------
    // 1. Array - The array from which
    // elements shall be deleted.
    // 2. Number - The index at which to
    // start the deletion.
    // 3. Number - The count of elements
    // to delete.
    // 4. Array - The elements to insert
    // into the target array.
    // -- Return ----------------------
    // Array - The deleted elements.

    function replaceArrayElements(
                                   targetArray,
                                   startIndex,
                                   deleteCount,
                                   replacements ) {
    // 1. Parameter of .apply():
    //   Defining the context (this-keyword).
    // 2. Parameter:
    //   a.) Start with the element startIndex.
    //   b.) Delete 3 elements.
    //   c.) Put these elements into the array
    //   as new values.
    //   a.), b.), c.) concated together
    //   into one array. The array is then
    //   used as the parameters of .splice().
    return Array.prototype.splice.apply(
                 targetArray,
                 [startIndex, deleteCount].concat(replacements));
    }

    // -------- TEST ---------------------------------
    var test = [];

    for (var i = 1; i <= 10; i++) {
      test.push(i);
    }

    console.log('Before insert: %o', test);

    var ret =
      replaceArrayElements(test, 5, 3, ['Six', 'Seven', 'Eight']);

    console.log('After insert: %o', test);
    console.log('Return: %o', ret);
    // -- RESULTS -----
    // Before insert: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
    // After insert: [1, 2, 3, 4, 5, "Six", "Seven", "Eight", 9, 10]
    // Return: [6, 7, 8]

The function does as expected.
But anyway: If someone knows a better way to accomplish the task then please let me know. The same for improvement suggestions.

This looks straightforward and solid under ES5. I have no comments or criticism for that.

However, under ES6, there's the spread operator (which is relatively well supported in most modern browsers, and in Babel), with which you can effectively oneline the whole thing like so:

targetArray.splice(start, end, ...replacements);


Executing method on a certain interval + offset I would like to execute a certain method at a given interval. The reason why I can't use a timer + its event handler right away is because I want to add an offset to each tick (100ms in my example below).

It seems to be working just as intented. The reason why I'm starting it manually is because if the _action(); takes more than the given tick time, I want to let it finish before starting a new "tick".

Perhaps this could be done in a much simpler way. I would like your thoughts on the code:

class RandomIntervalTask : IDisposable
{
    private readonly Timer _timer;
    private readonly int _initialInterval;
    private readonly int _range;
    private readonly Action _action;

    public RandomIntervalTask(int initialInterval, int range, Action action)
    {
        this._range = range;
        this._initialInterval = initialInterval;
        this._action = action;

        _timer = new Timer { AutoReset = false, Interval = initialInterval };
        _timer.Elapsed += TimerElapsed;
        _timer.Start();
    }

    private void TimerElapsed(object sender, ElapsedEventArgs e)
    {
        var rnd = new Random();

        _timer.Interval = rnd.Next(_initialInterval, _initialInterval + _range);
        _action();
        _timer.Start();
    }

    public void Dispose()
    {
        _timer.Stop();
        _timer.Dispose();
    }
}

Executed like this:

var rit = new RandomIntervalTask(400, 100, DoStuff);

private void DoStuff()
{
    Debug.WriteLine("stuff");
}

Do not put the random number generator creation in the TimerElapsed method. This could wind up generating the same value if called in succession rapidly. Instead put it at the class level:

private readonly Random _rnd = new Random();

and change appropriately in the method.


Program for converting an array of bytes to hexadecimal I have written a program that outputs the hexadecimal representation of an array of bytes. The program works just fine, and I am confident (thanks to valgrind) that I don't have any memory leaks. However, I am not so confident I am doing this the simplest, nor most readable way. It seems to me I have written a lot of code for a small task.

I am looking for general feedback, including code style, types, variable naming, usage of malloc, and simpler ways of appending a char array to a string, possibly avoiding sprintf.

The program was written to exercise storing a hexadecimal representation of a byte array in a string, not printing it out directly using printf().

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

int main() {
    const unsigned char bytearr[] = { 0x0b, 0x34, 0x33, 0x32 };
    const int hexlen = 2; // outputting hex value with leading zero
    const int outstrlen = sizeof(bytearr) * sizeof(char) * sizeof(bytearr[0]) * hexlen;

    char *outstr;
    if ((outstr = malloc(outstrlen + 1)) == NULL) {
        printf("Failed to allocate memory");
        exit(1);
    }
    bzero(outstr, outstrlen + 1);

    int i;
    char tmp[hexlen + 1];
    for (i = 0; i < (sizeof(bytearr) / sizeof(bytearr[0])); i++) {
        sprintf(tmp, "%.2x", bytearr[i]);
        strcat(outstr, tmp);
    }

    printf("Hexadecimal string:\n%s\n", outstr);

    free(outstr);

    return 0;
}

For what it's worth, after this review, my program ended up like this.

Well, aside from the fact that you could just reduce the program to outputting a single string-literal, let's look at it:

Excellent code-formatting. That's far too rare.
The calculation for outstrlen is all kinds of bent. Both middle factors don't make any sense.

sizeof(bytearr) * sizeof(char)   * sizeof(bytearr[0]) * hexlen
size_in_bytes   * bytes_per_byte * bytes_per_byte     * digits_per_byte

Just to be "safe", you zero outstr before overwriting it...
That isn't safe, that's redundant.
There's no good reason to use bzero() and make your program unportable. Just use memset() if you must.
Why do you print into a temporary buffer, only to append it to the output-string?

Print directly into the final destination. No need for an intermediate.
If you build a string bit by bit, don't be Shlemiel The Painter. No need to make a linear algorithm quadratic.

As you immediately exit the program, freeing memory is just busy-work. Still, as I'm not sure what you are trying to learn there, it might be excused.
As an aside, return 0; is implicit for main() in C99+ and C++. Not sure you that's really relevant, but here you go.

Equivalent program (C99+ or C++):

#include <stdio.h>

int main() {
    printf("Hexadecimal string:\n0b343332\n");
}


Programming of 3 x 7 trick The puzzle is:

  The deck contains 21 unique numbers
  The numbers should be laid up in rows of 7 (3 rows in total)
  ask the player to memorize a number to start the game
  once the game starts, rearrange the numbers in random in all rows.
  ask the player to select the row the number is in.
  once the player selects, rearrange the numbers for a second time

  this time, it is not random but sort the numbers in bundles per row
  stack all the bundles on top of each other, keeping the row selected to be in the centre.
  rearrange it back into rows of 7 such that first number in stack goes to row 1, second to row 2, third to row 3, 4th to row 1 again and so on.

  again ask the player to select the row the number is in.
  repeat step 6
  again ask the player to select the row the number is in.
  point to the middle number of the row selected as the answer.

  You can use any programming language to solve this.

I'd like feedback on this.

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#define numRows 3
#define numCols 7
#define TotalNum (numRows*numCols)

void random_rearrange_num(int p[][numCols]) ;
void display(int p[][numCols]) ;
void start();
int row_select(int count);
void stackup(int p[][numCols],int *s,int count);
void back_to_array(int p[][numCols],int *s);

int main()
{
    //for random numbers
    srand(time(NULL));

    //Step 1
    int arr[numRows][numCols] = {{0,1,2,3,4,5,6},{7,8,9,10,11,12,13},{14,15,16,17,18,19,20}};
    int stack[TotalNum] = {0};
    int row = 0;

    //Step 2
    display(arr);

    //Step 3
    start();

    //step 4
    random_rearrange_num(arr);

    //Step 5 (first time)
    display(arr);
    row = row_select(row);

    //Step 6
    stackup(arr,stack,row);
    back_to_array(arr,stack);

    //Step 7 (second time)
    display(arr);
    row = row_select(row);

    //Step 8:repeat step 6
    stackup(arr,stack,row);
    back_to_array(arr,stack);

    //Step 9 (third time)
    display(arr);
    row = row_select(row);

    //Step 10 and Step 11
    printf("The answer is : %i",arr[row][3]);

    return 0;
}
void random_rearrange_num(int p[][numCols])
{
  int temp = 0,k= 0,l = 0;
  for(int i = numRows-1 ; i > 0 ; i--)
  {
    for (int j = numCols-1;j>0; j--)
    {
          k = 0 + rand()/(RAND_MAX/(2-0+1)+1);
          l= 0 + rand()/(RAND_MAX/(6-0+1)+1);
          temp = p[i][j];
          p[i][j] = p[k][l];
          p[k][l] = temp;
    }
  }
}

void display(int p[][numCols])
{
       //printf("\n");
       for (int i = 0; i< numRows;i++)
       {
          for ( int j = 0;j< numCols;j++)
          {
            printf("%8i",p[i][j]);
          }
          printf("\n");
       }
}

void start()
{
        printf("\n");
        printf("Please memorize a number(between 0 to 20) and press enter to continue");
        char c=getchar();
        while (c != '\n')
            c=getchar();
        printf("\n");
}
int row_select(int count)
{
     printf("\n");
     printf("Please select the row the number is in(between 1 to 3):");
     scanf_s("%i",&count);
     return count-1;
}

void stackup(int p[][numCols],int *s,int count)
 {
    int k = 0,t= 0;
    int middle = (numRows-1)/2;
    for (int i = 0; i<numRows; i++)
    {
        t=i;
        if (i==middle || i==count)
            i = (i==count)?middle:count;
        for (int j = 0; j<numCols; j++)
        {
            s[k++] = p[i][j];
        }
        i=t;
    }
}
void back_to_array(int p[][numCols],int *s)
{
  int k = 0;
  for (int i = 0; i< numCols;i++)
       {
          for ( int j = 0;j<numRows;j++)
          {
             p[j][i] = s[k++];
          }

       }
}

Your random_rearrange_num function has a bias.  If you want an equal chance of getting every permutation, implement something like the Fisher-Yates shuffle.  Your implementation is actually a common implementation error.  You can read about it here.

I would implement it as follows: (my C is mighty rusty and I don't have a compiler on hand to verify syntax or correctness)

void random_rearrange_num(int p[][numCols])
{
  int r, k, l, temp;
  for (int i = numRows * numCols - 1; i > 1; i--)
  {
    r = rand() % i; //select from only the remaining numbers
    k = r / numRows;
    l = r % numRows;
    temp = p[i][j];
    p[i][j] = p[k][l];
    p[k][l] = temp;
  }
}


Test Driving Interface Design I have been doing TDD since I have started my first job out of university (about 5 months ago), most of which is working with legacy code. I started a personal project today and thought I would TDD it. I don't have much "green field" dev experience so I am unsure if this is the correct way of driving interface design. I am testing the interface has the expected methods, is a service contract etc... I get the feeling this is too much?

The code is shown below:

[TestFixture]
public class IPairSessionTests
{
    [TestCase]
    public void Then_IPairSession_is_a_service_contract()
    {
       var iPairSessionType = typeof(IPairSession);
       Assert.That(iPairSessionType.IsDefined(typeof(ServiceContractAttribute),true));

    }
    [TestCase]
    public void Then_IPairSession_has_Insert_Method()
    {
        var iPairSessionType = typeof(IPairSession);
        Assert.That(iPairSessionType.GetMethod("Insert"), Is.Not.Null);
    }

    [TestCase]
    public void Then_IPairsSession_Insert_is_operation_contract()
    {
        var iPairSessionType = typeof(IPairSession);
        var insertMethod = iPairSessionType.GetMethod("Insert");
        Assert.That(insertMethod.IsDefined(typeof(OperationContractAttribute), true));
    }
}

Yes, it is too much.  Testing that methods exist on a contract is both overkill and high-maintenance.  If you decide to change a method name, it breaks tests, and the test breakage doesn't tell you something useful.  Ask yourself what value you are getting out of this test.

I tend to test interfaces into existence by TDD'ing the classes that use them.  I don't think that more is useful.


react component optimization Dears
I have following component
import React from 'react';
import styled from 'styled-components';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { createSelector } from 'reselect';
import { filterTransactionsByDates } from 'Utils/functions';
import {ReactComponent as BilanceIcon} from 'assets/icons/balanceDashboardIcon.svg';
import { ReactComponent as IncomeIcon } from 'assets/icons/incomeDashboardIcon.svg';
import { ReactComponent as SavingsIcon } from 'assets/icons/savingsDashboardIcon.svg';
import { ReactComponent as ExpensesIcon } from 'assets/icons/expensesDashboardIcon.svg';

import SingleSummaryField from 'components/UI/SingleSummaryField'

const Wrapper = styled.div`
    width: 96%;
    height: 100%;
    display: flex;
    flex-flow: column;
    gap: 21px;
`

const Row = styled.div`
    display: flex;
    flex-flow: row;
    gap: 21px;
`

const BudgetSummary = ({ budgetSummary }) => {
  const {expensesSum, incomesSum, balance} = budgetSummary;
  return (
    <Wrapper>
      <Row>
        <SingleSummaryField amount={balance} name="Balance">
          <BilanceIcon />
        </SingleSummaryField>
        <SingleSummaryField amount={incomesSum} name="Income">
          <IncomeIcon />
        </SingleSummaryField>
      </Row>
      <Row>
        <SingleSummaryField amount={1000} name="Savings">
          <SavingsIcon />
        </SingleSummaryField>
        <SingleSummaryField amount={0 - expensesSum} name="Expenses">
          <ExpensesIcon />
        </SingleSummaryField>
      </Row>
    </Wrapper>
  );
};

BudgetSummary.propTypes = {
  budgetSummary: PropTypes.shape({
    expensesSum: PropTypes.number.isRequired,
    incomesSum: PropTypes.number.isRequired,
    balance: PropTypes.number.isRequired,
  }),
};

const getBudgetSummary = createSelector(
  (state) => state.expenses.expenses,
  (state) => state.incomes.incomes,
  (state) => state.datesRange.datesRange,
  (expenses, incomes, datesRange) => {
    const filteredExpenses = filterTransactionsByDates(expenses, datesRange);
    const filteredIncomes = filterTransactionsByDates(incomes, datesRange);

    const expensesSum = filteredExpenses.reduce((sum, currentTransaction) => {
      return sum + currentTransaction.amount;
    },0);

    const incomesSum = filteredIncomes.reduce((sum, currentTransaction) => {
      return sum + currentTransaction.amount;
    }, 0);

    const balance = incomesSum - expensesSum;

    return {
      expensesSum: expensesSum,
      incomesSum: incomesSum,
      balance: balance,
    };
  }
)

const mapState = (state) => {
  return {
    budgetSummary: getBudgetSummary(state)
  }
}

export default connect(mapState)(BudgetSummary);

What do you think about providing data
to the component ? Do you think that method which is being used right now is fine or it would be better to separate the logic to some kind of container component or maybe shall I create separate component which will handle only js operations on data ?

Cześć Łukasz,
I'd personally move some of the logic to the separate file. Eg. getBudgedSummary.That way, Your component will be more readable and easier to test.
btw,
instead of expensesSum: expensesSum, You can simply go for expensesSum


"Images" exercise from A Tour of Go The "Images" exercise in A Tour of Go asks for an implementation of image.Image.  I wrote two solutions, one easy and one harder (just for practice in dealing with interfaces).

SOLUTION 1: (HARD WAY)

package main

import (
    "golang.org/x/tour/pic"
    "image"
    "image/color"
)

type Image struct{
    // one array for x value, one array for y values, array of integer for pixel RGBA values
    image_ [][][]uint8
    x0, y0, x1, y1 int
}

func (i *Image) NewImage(x, y int) *Image {
    image := make([][][]uint8, y)
    for i := 0; i < x; i++ {
        image[i] = make([][]uint8, x)
        for j := 0; j < y; j++ {
            image[i][j] = make([]uint8, 4)
        }
    }
    return &Image{image, 0, 0, x, y}
}

func (p *Image) ColorModel() color.Model { return color.RGBAModel }

func (p *Image) At(x, y int) color.Color {
    var c color.Color
    rgba := color.RGBA{
        p.image_[x][y][0],
        p.image_[x][y][1],
        p.image_[x][y][2],
        p.image_[x][y][3],
    }
    c = rgba
    return c
}

func (p *Image) Bounds() image.Rectangle {
    rectangle := image.Rect(p.x0, p.y0, p.x1, p.y1)
    return rectangle
}

func (p *Image) crazyColors() *Image {
    for i := 0; i < len(p.image_) - 1; i++ {
        for j := 0; j < len(p.image_[i]) - 1; j++ {
            p.image_[i][j][0] = uint8(i + j / 2)
            p.image_[i][j][1] = uint8(i + j * 2)
            p.image_[i][j][2] = uint8(i + j * 2)
            p.image_[i][j][3] = uint8(255)
        }
    }
    return p
}

func main() {
    var i *Image
    i = i.NewImage(255, 255)
    i.crazyColors()
    pic.ShowImage(i)
}

SOLUTION 2: (EASY WAY)

package main

import (
    "golang.org/x/tour/pic"
    "image"
)

type Image struct{
    image.RGBA
}

func (i Image) crazyColors() Image {
    for p := 0; p < len(i.Pix); p += 4 {
        i.Pix[p] = uint8(p + p*2)
        i.Pix[p+1] = uint8(p + p*2)
        i.Pix[p+2] = uint8(p + p*2)
        i.Pix[p+3] = uint8(p + p*2)
    }
    return i
}

func main() {
    rect := image.Rect(0,0,255,255)
    m := Image{*image.NewRGBA(rect)}
    m.crazyColors()
    pic.ShowImage(&m)
}

Storing a rectangle

The Image stores the parameters to create a rectangle. Those values are not used for anything else in the lifetime of the object, except to create the bounding rectangle when needed. You could instead just store the rectangle itself, so could simply return it when needed.

Pointless local variables

Many of the local variables are pointless, you could skip declaring and assigning to them and simply return values directly, for example:

func (p *Image) At(x, y int) color.Color {
    return color.RGBA{
        p.image_[x][y][0],
        p.image_[x][y][1],
        p.image_[x][y][2],
        p.image_[x][y][3],
    }
}

Naming

The _ suffix in Image.image_ is unusual. I suggest to simply drop the suffix.

The name i is most often used in simple counting loops, I suggest to avoid using it for other purposes. A good short name for an image instance could be img, for example.


Java Regex for Chess notations I have this regex:

(\d?)[A-Za-z](?=(\d?)(.*))(?=(.* ){5})

It would be used by me for Chess FEN strings.

I wonder if regex [A-Za-z] is faster than [RNBQKrnbqk]? I only need to check the given letters (but no other letters will appear).

My thoughts are:

[A-Za-z] - the regex engine can match if a char is 65 <= c <= 90 or 97 <= c <= 122. Worst case is 4 comparisons.
[RNBQKrnbqk] - the engine checks if the inspected char equals every given character in the group. Worst case is 10 comparisons.

Am I understanding regex correctly?

No. The matching for a-zA-Z would be slower than the exact character-set you supply: RNBQKrnbqk.

You can observe this behaviour by checking the backtrace it generates. I compared 3 different patterns, two being your own, and the third I found on chess.stackexchange.com:

(\d?)[a-z](?=(\d?)(.*))(?=(.* ){5})

has 64 matches generated in 14323 steps

(\d?)[rnbqk](?=(\d?)(.*))(?=(.* ){5})

has 32 matches generated in 7512 steps

and the pattern from chess.stackexchange:

([rnbqkp1-8]+\/){7}([rnbqkp1-8]+)\s[bw]\s(-|K?Q?k?q?)\s(-|[a-h][36])

has 2 matches generated in 95 steps

Note that I have enabled the ignorecase flag in all three of them.


Check for Palindrome string in Java I was wondering if you can give me advice on this solution to assess if an user input string is a palindrome or not.
How can I optimize this solution?

import java.io.InputStreamReader;
import java.io.BufferedReader;
import java.io.IOException;

public class Palindrome {
public static void main(String[] args)
    throws IOException {

    // user enters string
    System.out.println ("Please enter string");

    InputStreamReader input = new InputStreamReader (System.in);
    BufferedReader reader = new BufferedReader (input);
    String userinput = "";
    userinput = reader.readLine();

    // test if string is valid
        if (!userinput.matches("[A-Za-z]+")){
            System.out.println ("Invalid Input");
            System.exit(1);
        }

        else if (userinput.length() > 10){
            System.out.println ("String too long");
            System.exit(2);
        }

    String stringReverse = "";
    for (int i = userinput.length()-1; i >= 0; i--){
        stringReverse += String.valueOf(userinput.charAt(i));
    }

    if (userinput.equals(stringReverse)){
        System.out.println ("String is a Palindrome");
        System.exit(0);
    }

    else if (userinput != stringReverse) {
        System.out.println ("String is not a Palindrome");
        System.exit(0);
    }

    }
}

When programming a solution like this, in order to simplify the problem create a separate function for each component. So, I would break it up into three different parts:

String getUserInput() {
    // Get input string and return it...
}

boolean isValidInput(String input) {
    // Return true or false based off of the validity of the input string.
}

boolean isPalindrome(String input) {
    int left = 0;
    int right = input.length();
    while(left < right) {
        if(input.charAt(left++) != input.charAt(--right)) return false;
    }
    return true;
}

The reason for returning a boolean rather than throwing an exception is that a try catch can be expensive especially if there is a good chance of an error being thrown. Now, a boolean means later that there may be some sort of conditional branch, but that is less expensive than if there is a high chance of an error being thrown. Also, there is more usability with returning a boolean, because now you can decide if you want to throw an exception or just print true or false.

Since the question was more geared towards the optimization of the palindrome detection algorithm, I provided a solution that I have found to work the best for when detecting palindromic inputs.

The solution cuts the number of steps down to a quarter of your solution. This is done by assuming that if the first half is the same as the second half then the String must be palindrome. So, we start from both ends and work our way to the center (essentially when left reaches or passes right). If any where along the way we find a point in which is not equal, then we know that we do not have a palindromic String. Therein, we can exit returning false. If we successfully exit the loop, then we know we were given a palindromic String. Therein, we return false.


Validating a search string for an SQL text search Is it safe to allow these special characters when validating a string? The user is currently allowed to send alpha numeric and the following special characters from the browser:

',.!&()_-

My main concern is code injection. I'm still learning Laravel and I understand it handles a lot security wise. I'd like to make sure I'm taking solid precautions for good security.

I have the following code which uses the PHP Laravel framework to validate on the server side:

$this->validate($request, [
    'str' => 'bail|required|regex:/^[a-zA-Z0-9 \',.!&_-]+$/u'
]);

$search_str = $request->str;

$query = Story::where('story_text', 'LIKE', '%'.$search_str.'%');

Is this sufficiently secure or is there more I should be doing?

Laravel's Eloquent uses PDO's parameterized queries, so you get the same level of protection from SQL injection attacks as using PDO directly. Which is to say, you don't generally need to worry about sanitizing your input with Laravel.

The main exception being if you ever use Eloquent's raw query. You should avoid passing user-supplied input to a raw query at pretty much any price, but that's where you'd need to sanitize.

As for the input validation, you should just be using it to make sure the inputs conform to your requirements. For instance, ensuring that something looks like an email or is alphanumeric. Or to make sure inputs correspond to an extant record in the database. It's almost never necessary to use it for input sanitization and will probably end up confusing both you and the user, later on.