build up object[] from raw data set I have a list of raw data like following mockData and need to generate to a object[] format, some key-value pair are required from raw data like first, last, state. but some key-value pair are option like nickname. Eventually, I need to build a object array with some same key-value like raw data, and some key's value depends on another key's value like fullname, address. If there is a better way for me to do it and make it more dynamically? because I will have more than 20 different key-value set like fullname, address that need to be built by some function related on raw data

export const mockData: Human[] = [
  {
    first: 'name1',
    last: 'v1',
    state: 'ny',
  },
  {
    first: 'name2',
    last: 'v2',
    state: 'pa',
    nickname: 'rocketman',
  }]

export interface Human {
  first: string;
  last: string;
  state: string;
  nickname?: string;
}

export interface Student extends Human{
  fullname: string;
  address: string;
}

function setFullName(first: string, last: string): string {
    return `${first} ${last}`
}

function setAddress(first: string, last: string, state: string): string {
    return `${first} ${last} live a ${state}`
}

function buildStudentData(jsons: Human[]): Student[] {
  return jsons.map((json) =>  <Student> {
      first: json.first,
      last: json.last,
      nickname:  json.hasOwnProperty('nickname') ? json.nickname : json.first,
      fullname: setFullName(json.first, json.last),
      address: setAddress(json.first, json.last, json.state)})
}

console.log(buildStudentData(mockData));

You have kind of "logic" in your mapping (nickname, fullname and adress). And I assume that this will be the case also for the other data sets you mentioned. That makes generalizing hard. Especially when there is not much in common.

You COULD write a generator were you can configurate for each attribute the mapping strategy, but this would take more time then write the mapping of those 20-30 values directly.

I personally use in quite some cases classes instead of interfaces and then apply a public static "fromJSON(...)" or in your case "fromHuman(...)" method onto it. Its MORE code to write, but it makes everything explicit. And you are able to ensure that a object (a student) ALWAYS has a proper name. Such things make refactoring later easier. But as i said, the negative part is that you implement more code to gain this safety.

For example in the following code there is only ONE way to create a student. And if the primary identifier of this object (the firstname / lastname combination) is missing then it errors out.

export class Student implements Human {
  readonly first: string;
  readonly last: string;
  state: string;
  nickname?: string;
  fullname: string;
  address: string;

  private constructor(firstname: string, lastname: string{
    if(firstname && lastname){
      throw Error('Students must have a first and a lastname!');
    }
    this.first = firstname;
    this.last = lastname;
  }

  public static fromHuman(human: Human): Student {
    const student: Student = new Student(human.first, human.last);
    student.state= human.state;
    student.nickname = human.nickname | human.first
    // if you are using TSD 3.7 or above you should use the ?? operator
    // ...
  }
}

If some of the values may change, then i would think about NOT storing calculated values (like the adress) but use a GET method. The reason is, IF one of the values used for calculation (e.g. the state) changes, then a stored "adress" would have to get updated manually.

export class Student ...{
   // ...
   public get adress(): string{
      return `${this.first} ${this.last} live a ${this.state}`
    }
 }
}

PRO: Values get updated automaticly
CON: Values have to get recalculated each time. This may be a performance issue, depending on the usage.

At the end (as always) it depends what you want to achieve. If you need a quick way to generate data, then i would personally stick to your way.
If you are using the objects heavily in your application, then i would think about how they are used from the business perspective (which values may not change, which may change often ,...) and treat them as Business Objects. And then guard the objects against misuse. Even if only you are using that code, this may guard you against mistakes and help you understand your code when you have to fix a bug or add a feature a few months later.


Do you see any flaws in these prepared statements to avoid SQL injection? I have used this guide to implement prepared statements in order to avoid SQL Injection.

I made some tests and no error was shown. However, I would like to ask you if you see any flaw in the code, so that I can improve it.

This is the code for a simple login:

<form action="conexion_PDO.php" method="post">
User:
<input type="text" name="user">
Pass:
<input type="password" name="pass">
<input type="submit" value="Ingresar">
</form>

This is Conexion_PDO.php:

<?php

try {
$gbd = new PDO('mysql:host=localhost;dbname=database', 'root', '');
$statement = $gbd->prepare("SELECT * FROM user where email = ? AND
password = ?");
if ($statement->execute(array($_GET['user'],md5($_POST['pass'])))) {
while ($rs = $statement->fetch()) {
print_r($rs);
}

$gbd = null;
} catch (PDOException $e) {
print "¡Error!: " . $e->getMessage() . "
";
die();
}

?>

Your code does not have an SQL injection vulnerability because you are using placeholders instead of interpolating the variables. The way you interface with the database is laudable.

Other aspects of your code can be improved:

The variables $gdb, $statement, $rs, and $e are globals, which is not acceptable within a larger system. Enclose your code with a function to make them locals.

Otherwise, you could accidentally overwrite variables from other parts in your code (e.g. when you include this snippet). Consider what happens if $gdb is already assigned: You try to clean up your usage by assigning null when you are finished, but the other code now suddenly has its contents for $gdb deleted.
You do not seem to be using any intendation. Indent your code properly so that it can be understood easily.
You use the literal ¡ inside a string, and print it directly to the output. As this is a non-ASCII character, this can get problematic depending on the encoding of your source code and the character encoding in the HTTP header and the HTML document.
HTML markup is a kind of code as well – it too wants to be indented :)

But now something important:

  Do not use md5 to hash passwords!

MD5 is extremely weak, and an increasing number of attacks are being discovered. Do not use it for anything in new systems.
It is meant for checksums (e.g. when comparing files), not for hashing passwords. Such a checksum is designed that a subtle change in the input changes the output of the hash function drastically. It is not designed to irreversibly scramble a secret.
It is far to easy to calculate the original password from a given hash, e.g. using rainbow tables. You do not even use a salt to defend against this (but even with a salt, an MD5-hashed password can be cracked within reasonable time).

You should be using a key-stretching algorithm like bcrypt. I already wrote a rant about secure password hashing in another answer which I would ask you to consult for a better hashing solution.


C++ Fast Fourier transform This is a very simple FFT, I am wondering what I can do to make this faster and more memory efficient from the programming side (better data types, and maybe some tricks like unrolling loops or using the preprocessor if that is useful here), and not by using a more efficient mathematical algorithm. Obviously I would appreciate advice on best practices as well.
#include <stdio.h>
#include <vector>
#include <iostream>
#include <complex>
#include <cmath>
#include <algorithm>

#define N 1048576
#define PI 3.14159265358979323846

/*
Creating the table of all N'th roots of unity.
We use notation omega_k = e^(-2 pi i / n).
*/
template<typename U>
std::vector< std::complex<U> > rootsOfUnityCalculator() {
    std::vector< std::complex<U> > table;

    for (size_t k = 0; k < N; k++) {
        std::complex<U> kthRootOfUnity(std::cos(-2.0 * PI * k / N), std::sin(-2.0 * PI * k / N));
        table.push_back(kthRootOfUnity);
    }

    return table;
}

/*
Fast Fourier transform, T is the precision level, so float or double.
table is a look up table of the roots of unity. Overwrites the input.
For now only works for N a power of 2.
*/
template<typename T>
void FFT(std::complex<T>* input, const std::vector< std::complex<T> >& table, size_t n) {

    if (n % 2 == 0) {
        // Split up the input in even and odd components
        std::complex<T>* evenComponents = new std::complex<T>[n/2];
        std::complex<T>* oddComponents = new std::complex<T>[n/2];

        for (size_t k = 0; k < n/2; k++) {
            evenComponents[k] = input[2 * k];
            oddComponents[k] = input[2 * k + 1];
        }

        // Transform the even and odd input
        FFT(evenComponents, table, n/2);
        FFT(oddComponents, table, n/2);

        // Use the algorithm from Danielson and Lanczos
        for (size_t k = 0; k < n/2; k++) {
            std::complex<T> plusMinus = table[N / n * k] * oddComponents[k]; // omega_n^k = (omega_N^(N/n))^k = omega_N^(Nk/n)
            input[k] = evenComponents[k] + plusMinus;
            input[k + n/2] = evenComponents[k] - plusMinus;
        }

        delete[] evenComponents;
        delete[] oddComponents;

    } else {
        // The Fourier transform on one element does not do anything, so
        // nothing needed here.
    }
}

int main() {
    std::complex<double>* input = new std::complex<double>[N];

    for (size_t k = 0; k < N; k++) {
        input[k] = k;
    }

    const std::vector< std::complex<double> > table = rootsOfUnityCalculator<double>();

    // Overwrites the input with its Fourier transform
    FFT<double>(input, table, N);

    delete[] input;

    return 0;
}

Prefer C++ headers (such as <cstdio>) to the C compatibility headers (<stdio.h>).  The C++ headers define identifiers in the std namespace where we want them.
It seems that this header isn't even used here, so we can omit it completely.

std::size_t is misspellt throughout as size_t.  This commonly happens when writing on a platform that declares it in the global namespace as well as in std, but that's not a portable assumption.

We only use PI as a double, so we can declare it as a constant rather than as a macro:
constexpr double pi = 3.14159265358979323846;

Or to make it more obviously correct:
const double pi = std::acos(-1.0);

We can avoid naked new[] and delete[] using std::vector.  If we use std::vector::reserve before adding any elements, there should be no performance impact, only improved code safety.
Using a vector saves having to pass the array size explicitly, too.

A helpful technique to determine whether a number is a power of two is that bitwise-and of the number and its arithmetic predecessor is zero for powers of two:
constexpr bool is_power_of_two(unsigned n)
{
    return n && (n & (n-1) == 0);
}

A single value of N is probably not very useful in general.  We could make it a template parameter, or a simple function argument, to make our code useful for other sizes of input.

Modified version
I split FFT() into a simple public function that ensures arguments are valid and the internal recursive implementation.  I've used a requires to constrain the complex type we accept (integer types aren't useful here), but that can be omitted for plain C++17.
#include <complex>
#include <vector>

namespace internal
{
    /*
      Creating the table of all N'th roots of unity.
      We use notation ω_i = e^(-2πi/n).
    */
    template<typename U>
    constexpr auto rootsOfUnity(std::size_t n)
    {
        constexpr auto pi = std::acos(U{-1.0});
        std::vector<std::complex<U>> table;
        table.reserve(n);

        for (std::size_t i = 0;  i < n;  ++i) {
            table.emplace_back(std::cos(-2.0 * pi * i / n), std::sin(-2.0 * pi * i / n));
        }

        return table;
    }

    template<typename V>        // V is vector of complex
    void FFT(V& input, const V& table, std::size_t total_size)
    {
        if (input.size() <= 1) {
            // The Fourier transform on one element does not do
            // anything, so nothing needed here.
            return;
        }

        const auto n2 = input.size() / 2;

        // Split up the input in even and odd components
        V evenComponents;  evenComponents.reserve(n2);
        V oddComponents;   oddComponents.reserve(n2);

        for (std::size_t i = 0;  i < input.size();  ) {
            evenComponents.push_back(input[i++]);
            oddComponents.push_back(input[i++]);
        }

        // Transform the even and odd input
        FFT(evenComponents, table, total_size);
        FFT(oddComponents, table, total_size);

        // Use the algorithm from Danielson and Lanczos
        for (std::size_t i = 0;  i < n2;  ++i) {
             // ω_n^i = (ω_N^(N/n))^i = ω_N^(Nk/n)
            auto const plusMinus = table[total_size / n2 * i] * oddComponents[i];
            input[i] = evenComponents[i] + plusMinus;
            input[i + n2] = evenComponents[i] - plusMinus;
        }
    }
}

/*
  Fast Fourier transform
  T is the precision level: float, double, long double.
  Overwrites the input.
  Only works for input sizes that are a power of 2.
*/
template<typename T>
    requires std::is_floating_point<T>
void FFT(std::vector<std::complex<T>>& input)
{
    if (input.size() == 0) {
        throw std::invalid_argument("input is empty");
    }
    if (input.size() & (input.size() - 1)) {
        throw std::invalid_argument("input length is not a power of two");
    }

    auto const table = internal::rootsOfUnity<T>(input.size());
    internal::FFT(input, table, input.size());
}

// Test program
#include <iostream>

int main()
{
    static constexpr size_t n = 1048576;
    std::vector<std::complex<double>> input;
    for (std::size_t i = 0;  i < n;  ++i) {
        input.emplace_back(i);
    }

    // Overwrite input with its Fourier transform
    FFT<double>(input);

    // use the result, to avoid optimising out
    std::cout << input[0] << '\n';
}


Efficient way to iterate over elements of array for matching contents of another array Using jQuery I'm trying to find an efficient way to iterate through an array excludeMe and check if elements from another array needsExclusions are contained in the excludeMe array. Not sure if this is the most efficient, but it works. Fiddle here: http://jsfiddle.net/yef9q/3/

//Iterate through needsExclusions for an excludeMe string
//someFlag raised on matching strings

var excludeMe = ["in west", "philadelphia", "born and", "raised"];
var needsExclusions = ["philadelphia", "raised"];
var someFlag = true;

$.each(excludeMe, function(i,val) {
  //alert("value: " + val);
  if ($.inArray(val, needsExclusions) != -1) {
      alert("matching value: " + val);
      someFlag = false;
  }
});

Your particular example gets 10 times faster by just removing the jQuery and using a for loop and .indexOf() instead of the jQuery versions.  It's also worth stopping the iteration when you find an excluded value since you don't need to look any more once the flag is already set:

var excludeMe = ["in west", "philadelphia", "born and", "raised"];
var needsExclusions = ["philadelphia", "raised"];
var someFlag = true;

for (var i = 0; i < excludeMe.length; i++) {
    if (needsExclusions.indexOf(excludeMe[i]) !== -1) {
        someFlag = false;
        break;
    }
}

Here's a jsperf that illustrates the speed difference: http://jsperf.com/exclusion-test

If you had a much large data set, then it might pay off to put the needsExclusions values into an object to serve as a direct lookup index (faster than array.indexOf()), but that's probably only faster when the list gets a lot longer because there's setup overhead to build the initial index.


Copy Cell on Worksheet_Change event if some criteria is met This code is working fine, but it looks awful (yes, I am a rookie on this thing). How can I improve it to look better?

Private Sub Worksheet_Change(ByVal Target As Range)

'Conditional Formatting
Application.ScreenUpdating = False

If Target.Column > 6 And Target.Column < 8 And (Target.Value = "G" Or Target.Value = "Y" Or Target.Value = "R" Or Target.Value = "g" Or Target.Value = "y" Or Target.Value = "r") Then
Sheets("Pending").Cells(1, 1).Copy

Call ChangeG
End If

If Target.Column > 4 And Target.Column < 6 And (Target.Value = "High" Or Target.Value = "Medium" Or Target.Value = "Low" Or Target.Value = "high" Or Target.Value = "medium" Or Target.Value = "low") Then
Sheets("Pending").Cells(1, 1).Copy

Call ChangeE
End If

Application.ScreenUpdating = True

End Sub

Indent everything one level inside of the Sub...End Sub block. Go one more inside of If blocks and loops.

Private Sub Worksheet_Change(ByVal Target As Range)
    ' all code starts at this level of indentation

    If condition Then
        ' one more level
    End If

End Sub

There shouldn't be more than one blank line between instructions.

Private Sub Worksheet_Change(ByVal Target As Range)
    'Conditional Formatting
    Application.ScreenUpdating = False

    If Target.Column > 6 And Target.Column < 8 And (Target.Value = "G" Or Target.Value = "Y" Or Target.Value = "R" Or Target.Value = "g" Or Target.Value = "y" Or Target.Value = "r") Then
    Sheets("Pending").Cells(1, 1).Copy

This condition can be simplified.

Target.Column > 6 And Target.Column < 8

means the same thing as Target.Column = 7. Same goes for

Target.Column > 4 And Target.Column < 6

That can simply be Target.Column = 5.
I don't normally advise using line continuations, but they can be justifiably used to reduce the amount of horizontal scrolling.

If Target.Column = 7 And _
    (Target.Value = "G" _
        Or Target.Value = "Y" _
        Or Target.Value = "R" _
        Or Target.Value = "g" _
        Or Target.Value = "y" _
        Or Target.Value = "r") Then

    Sheets("Pending").Cells(1, 1).Copy

    Call ChangeG
End If

That last change brought to light the fact that you want to match on upper or lower case versions of these letters. Using VBA's Like operator will help clean that up with some built in pattern matching.

If Target.Column = 7 And Target.Value Like "[GgYyRr]" Then

    Sheets("Pending").Cells(1, 1).Copy

    Call ChangeG
End If

Note that it cleans it up so well, that the line continuation isn't needed anymore.
Don't use the Call keyword. It's unneeded and archaic. It's simply a leftover from an ancient version of VBA.
The second condition is a little harder to clean up, but we can still use Like to make it better.

If Target.Column = 5 And _
    (Target.Value Like "[Hh]igh" _
        Or Target.Value Like "[Mm]edium" _
        Or Target.Value Like "[Ll]ow") Then

    Sheets("Pending").Cells(1, 1).Copy
    ChangeE
End If

You've hard coded the string literal for the "Pending" sheet twice. It would be better to initialize a Worksheet variable upon entering the event routine.

Dim sourceSheet As Worksheet
Set sourceSheet = Worksheets("Pending")

Note that I used the Worksheets collection instead of Sheets. That's because the Sheets collection can contain charts as well as worksheets. It's a rare issue, but it can be one.

And there ya go. All cleaned up.

Private Sub Worksheet_Change(ByVal Target As Range)
    Application.ScreenUpdating = False

    Dim sourceSheet As Worksheet
    Set sourceSheet = Worksheets("Pending")

    If Target.Column = 7 And Target.Value Like "[GgYyRr]" Then
        sourceSheet.Cells(1, 1).Copy
        ChangeG
    End If

    If Target.Column = 5 And _
        (Target.Value Like "[Hh]igh" _
            Or Target.Value Like "[Mm]edium" _
            Or Target.Value Like "[Ll]ow") Then

        sourceSheet.Cells(1, 1).Copy
        ChangeE
    End If

    Application.ScreenUpdating = True
End Sub

One last thing that I have not addressed, but was mentioned in the comments by Tim Williams. There is a potential bug waiting to rear its ugly little head. Target can be a multi-cell range object and Range.Value is not available in multi-cell ranges. You need to be prepared to deal with that situation should it arise. Consider the example below.

Public Sub test()
    Dim Workbook As Workbook
    Set Workbook = ThisWorkbook

    Dim sheet As Worksheet

    'Set sheet = Workbook.Worksheets("Sheet1")
        Set sheet = Workbook.Worksheets(1)

    Dim rng1 As Range
    Set rng1 = sheet.Range("C2")

    Dim rng2 As Range
    Set rng2 = sheet.Range("C2:C10")

    Debug.Print rng1.Value ' prints the value stored in C2
    Debug.Print rng2.Value ' raises type mismatch error #13

End Sub


Getting the current IP from the command line I made a tiny little bash script, that prints the current IP Address to the command line. I checked the script with ShellCheck (0.4.6-1) on a local debian install and I get the following message:

  SC2026: This word is outside of quotes. Did you intend to 'nest '"'single quotes'"' instead'?

What is wrong with the way I have been line-breaking the pipe, or what is proper practice?

#!/bin/bash

 curl -s POST whatismyip.org\
  | grep -A1 'Your IP Address'\
  | awk -F '>' '{print $2}'\
  | sed 's/<\/h2//g'\
  | sed 's/<\/span//g'

I would use a different service that provides output in a format that is good for tools (not humans (html is for displaying for humans)).

Hit this url:

https://ifconfig.co/json

You're output looks like this:

{
  "ip": "154.140.296.288",
  "ip_decimal": 926648668,
  "country": "United States",
  "city": "Seattle",
  "hostname": "154-140-296-288.Bob.com"
}

Which is obviously json which can be parsed using the jq tool.

> curl -s https://ifconfig.co/json  | jq .ip -r
154.140.296.188


Sales, Taxes and Totals I'm working to get some practice with SQL functions which are often used in a financial setting, and I created this simple table with products, prices and tax rates basically.

I then wrote a query to simulate a purchase invoice type of result set (I realize things like that are most often done at application level, or as a separate DB transaction).

After I wrote this query, I found myself using so many parentheses for operations, and repeating the same formulas over and over, and I thought; there's got to be a better way to do this! So here I am seeking peer review.

You can try it using SEDE.

Here is the table:

CREATE TABLE #Sales (
    Id INT NOT NULL IDENTITY PRIMARY KEY
  , Product NVARCHAR(MAX)
  , Price DECIMAL(6,2)
  , Qty INT
  , Import BIT
  , SalesTaxRate DECIMAL(6,2)
  , ImportTaxRate DECIMAL(6,2)
);
GO

INSERT INTO #Sales (Product, Price, Qty, Import, SalesTaxRate, ImportTaxRate)
VALUES
  ('Apple Pie',          3.99,  10, 0, 7.50, NULL)
, ('Swiss Chocolate',    6.99,  5,  1, NULL, 5.50)
, ('French Wine',        12.99, 3,  1, 7.50, 5.50)
, ('Complimentary Gift', NULL,  1,  0, NULL, NULL)
;
GO

And here is the query which results in this table being returned:

SELECT
  Id
, Product
, [Price] = COALESCE(Price, 0)
, Qty
, [Subtotal] = (ISNULL(Price,0) * Qty)
, [Sales Tax] = CAST((ISNULL(SalesTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2))
, [Import Tax] = CAST((ISNULL(ImportTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2))
, [Total Tax] = COALESCE(
    CAST(((ISNULL(SalesTaxRate,0) * (ISNULL(Price,0) * Qty)) + (ISNULL(ImportTaxRate,0) * (ISNULL(Price,0) * Qty))) / 100 AS DECIMAL(6,2))
    , 0)
, [Line Total] = (
    ISNULL(Price,0) * Qty -- Subtotal
    + COALESCE(
    CAST(((ISNULL(SalesTaxRate,0) * (ISNULL(Price,0) * Qty)) + (ISNULL(ImportTaxRate,0) * (ISNULL(Price,0) * Qty))) / 100 AS DECIMAL(6,2))
    , 0)) -- Total Tax
FROM #Sales

UNION

SELECT
  NULL
, 'Totals'
, NULL
, NULL
, SUM((ISNULL(Price,0) * Qty)) -- Subtotal
, SUM(CAST((ISNULL(SalesTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2)))
, SUM(CAST((ISNULL(ImportTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2)))
, SUM(COALESCE(
    CAST(((ISNULL(SalesTaxRate,0) * (ISNULL(Price,0) * Qty)) + (ISNULL(ImportTaxRate,0) * (ISNULL(Price,0) * Qty))) / 100 AS DECIMAL(6,2))
    , 0))
, SUM(
    ISNULL(Price,0) * Qty -- Subtotal
    + COALESCE(
      CAST(((ISNULL(SalesTaxRate,0) * (ISNULL(Price,0) * Qty)) + (ISNULL(ImportTaxRate,0) * (ISNULL(Price,0) * Qty))) / 100 AS DECIMAL(6,2))
      , 0)) -- Total Tax
FROM #Sales

Don't Repeat Yourself

Okay, a good way of getting rid of repeated formulas is to use move them into a CROSS APPLY.

We can move the following into a CROSS APPLY, which I named Calcs.

, SUM((ISNULL(Price,0) * Qty)) -- Subtotal
, SUM(CAST((ISNULL(SalesTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2)))
, SUM(CAST((ISNULL(ImportTaxRate,0) * ISNULL(Price,0) * Qty) / 100 AS DECIMAL(6,2)))

I'm actually not going to include the divide by 100 bit in my CROSS APPLY, which makes stuff a little easier for us later.

Applying Yourself to the Problem

For now I'm going to ignore the Totals bit, you'll see why a little later.
This is what the query looks like now:

SELECT
    Id,
    Product,
    [Price] = COALESCE(Price,0),
    Qty,
    [Subtotal] = Calcs.Item_Price,
    [Sales Tax] = CAST(Calcs.Sales_Tax / 100 AS DECIMAL(6,2)),
    [Import Tax] = CAST(Calcs.Import_Tax / 100 AS DECIMAL(6,2)),
    [Total Tax] = COALESCE(CAST((Calcs.Sales_Tax + Calcs.Import_Tax) / 100 AS DECIMAL(6,2)),0),
    [Line Total] =
    Calcs.Item_Price
    + COALESCE(CAST((Calcs.Sales_Tax + Calcs.Import_Tax) / 100 AS DECIMAL(6,2)),0)
FROM #Sales
CROSS APPLY
(
    SELECT
        ISNULL(Price,0) * Qty AS Item_Price,
        (ISNULL(SalesTaxRate,0) * ISNULL(Price,0) * Qty) AS Sales_Tax,
        (ISNULL(ImportTaxRate,0) * ISNULL(Price,0) * Qty) AS Import_Tax
) Calcs

Rollup, Rollup!

Now, a SQL keyword you don't see used very often. We are going to use GROUP BY X WITH ROLLUP to automatically generate a total column for us.

What you want to do is add a GROUP BY Product WITH ROLLUP below the CROSS APPLY. The query needs a couple of modifications now though, because we've added aggregation.

Basically for anything that will be included in both the data and the totals, you take either the SUM, if it is a numeric column, or either the MIN or MAX for text fields.

For fields that you only want to include in either the data or totals, you have to add a CASE statement to the select query similar to one of the following:

--For the total column, GROUPING(Product) will be 1
-- for the data columns it will be equal to 0
CASE
    WHEN GROUPING(Product) = 0
    THEN MAX(Product) --For data rows, return the product
    ELSE 'Totals' --For the total row, return the text 'Totals'
END AS Product,

CASE
    WHEN GROUPING(Product) = 0
    THEN MAX(Price) --For data rows, return the price
    --We don't include an else because we don't want to
    -- want anything to be returned for this field in the totals column.
END AS Price,

All Together Now!

Here is what I ended up with, which I think has much less repeated formulas and avoids having to basically repeat the entire query twice.

SELECT
    CASE
        WHEN GROUPING(Product) = 0
        THEN MAX(Id)
    END AS Id,
    CASE
        WHEN GROUPING(Product) = 0
        THEN MAX(Product)
        ELSE 'Totals'
    END AS Product,
    CASE
        WHEN GROUPING(Product) = 0
        THEN ISNULL(MAX(Price), 0)
    END AS Price,
    CASE
        WHEN GROUPING(Product) = 0
        THEN MAX(Qty)
    END AS Qty,
    [Subtotal] = SUM(Calcs.Item_Price),
    [Sales Tax] = SUM(CAST(Calcs.Sales_Tax / 100 AS DECIMAL(6,2))),
    [Import Tax] = SUM(CAST(Calcs.Import_Tax / 100 AS DECIMAL(6,2))),
    [Total Tax] = SUM(COALESCE(CAST((Calcs.Sales_Tax + Calcs.Import_Tax) / 100 AS DECIMAL(6,2)),0)),
    [Line Total] =
    SUM(Calcs.Item_Price
    + COALESCE(CAST((Calcs.Sales_Tax + Calcs.Import_Tax) / 100 AS DECIMAL(6,2)),0))
FROM #Sales
CROSS APPLY
(
    SELECT
        ISNULL(Price,0) * Qty AS Item_Price,
        (ISNULL(SalesTaxRate,0) * ISNULL(Price,0) * Qty) AS Sales_Tax,
        (ISNULL(ImportTaxRate,0) * ISNULL(Price,0) * Qty) AS Import_Tax
) Calcs
GROUP BY Product WITH ROLLUP
ORDER BY GROUPING(Product),Id


Improving readability of enabling method There doesn't seem to be anything really wrong with this method. But it has a little smell to me. Does the name make sense (do what you think it would do). Is there some logic I could use to make the enabler a bit more elegant?

private void tryEnableCRUD(bool tryEnable = true)
{
    bool enable = false;
    if (_securityLevel < 3)
    {
        enable = tryEnable;
    }
    else
    {
        enable = tryEnable;
    }

    tsbDelete.Enabled = enable;
    tsbReplace.Enabled = enable;
    tsbUpload.Enabled = enable;
}

UPDATE

private void tryEnableCRUD(bool tryEnable = true)
{
    bool enable = false;
    if (_securityLevel < 3)
    {
        enable = tryEnable;
    }
    tsbDelete.Enabled = enable;
    tsbReplace.Enabled = enable;
    tsbUpload.Enabled = enable;
}

A method like this should do at least two things in addition to MrSmith42's answer:

It should return boolean to indicate whether it succeeded or not (i.e. return enable; at the end)
if it declares a parameter tryEnable = true then it should either use it, or remove it.

EDIT: after edit and comment from OP.

After your changes I think my point-1 remains valid, but your changes make the 'smell' even worse.... ;-)

You should change the method name to trySetCRUDEnabled(...)
what if the settings are currently enabled, and the user calls trySetCRUDEnabled(false) ... it makes the logic hard to understand.... in fact, it's broken because now it does not matter what the security-level is ... ;-) (you are changing the code in haste, and making mistakes).

Incorporating MrSmith42's suggestion for simplifying the code, it should read:

bool enable = tryEnable && _securityLevel < MINSECURITY;


How can I improve this version of the board game Mastermind? It's a standalone HTML file on GitHub

I included the CSS and JavaScript with the HTML so it would be easier to e-mail the game.

I'm most interested in comments about the solution-checking functions. This is the main function:

var evaluateSolution = function () {
//"In play" is the default state for the game.
var state = stateGame;

//Look for exact matches.
checkColorPositionMatches();

//If every peg matches exactly, the game was won
if (colorPositionMatches == codeLength) {
    state = stateWin;

} else {
    //Look for color matches among the unmatched pegs.
    checkColorMatches();
    state = stateLose;
}

//Reset the match state of each codePeg
unmatchCodePegs();

return state;
}

This one checks for exact matches.

var checkColorPositionMatches = function() {
colorPositionMatches = 0;

codePegLoop:
for (var codePegIdx=0;codePegIdx<codePegs.length;codePegIdx++) {
    var codePeg = codePegs[codePegIdx];

    //evaluate each gamePeg, stopping if we hit an exact match.
    gamePegLoop:
    for (var gamePegIdx=0;gamePegIdx<gamePegs.length;gamePegIdx++) {
        var gamePeg = gamePegs[gamePegIdx];

        if ((gamePeg.color == codePeg.color) &&
            (gamePeg.position == codePeg.position)) {

                gamePeg.colorPositionMatched = true;
                codePeg.colorPositionMatched = true;

                colorPositionMatches++;

                break gamePegLoop;
        }
    }
}
}

This one checks for color-only matches.

var checkColorMatches = function() {
colorMatches = 0;

codePegLoop:
for (var codePegIdx=0;codePegIdx<codePegs.length;codePegIdx++) {
    var codePeg = codePegs[codePegIdx];

    //evaluate each unmatched game peg, recording matches as they occur
    gamePegLoop:
    for (var gamePegIdx=0;gamePegIdx<gamePegs.length;gamePegIdx++) {
        var gamePeg = gamePegs[gamePegIdx];

        if (gamePeg.color == codePeg.color &&
           (gamePeg.colorMatched == false) &&
           (codePeg.colorMatched == false) &&
           (gamePeg.colorPositionMatched == false) &&
           (codePeg.colorPositionMatched == false)) {

            gamePeg.colorMatched=true;
            codePeg.colorMatched=true;

            colorMatches++;

            break gamePegLoop;
        }
    }
}
}

The first thing I notice is that you're "manually" handling both color and position. This isn't actually necessary if you set everything up using simple arrays.

An array is, by definition, an indexed list of values. So if you use a simple array for your code and the pegs, then positions are the array's indices and colors are its values. This simplifies things considerably.

For instance, to find exact (color and position) matches, you only need to check if code[i] === pegs[i]. And to check if a peg matches anywhere in the code, you only need to check code.indexOf(peg[i]) !== -1. So if you can get something like this:

var code = ["white","black","red","blue","green"];  // the code
var pegs = ["white","black","blue","red","orange"]; // the player's selections

it'll make the checking a lot easier.

Now, along the way you'll need to do some extra work to keep track of the number of matches, and make sure you're not double-matching stuff. I.e. you need to pare down your arrays as you go, so they only contain the remaining unmatched values.

Here's an example:

function evaluateSolution(code, pegs) {
  var i, l,
      foundIndex,
      exactMatches = 0,
      valueMatches = 0;

  // copy the arrays, so we don't ruin the orignals
  code = code.slice(0);
  pegs = pegs.slice(0);

  // First pass: Look for value & position matches
  // We're looping "backwards", so we can safely remove items
  // "behind us"
  for( i = pegs.length - 1 ; i >= 0 ; i-- ) {
    // If there's a match, remove the matched index from both
    // the code and the pegs so it isn't matched again later.
    if(pegs[i] === code[i]) {
      exactMatches++;
      pegs.splice(i, 1);
      code.splice(i, 1);
    }
  }

  // Now, pegs and code only contain unmatched values

  // Second pass: Look for value matches anywhere in the code
  for( i = 0 , l = pegs.length ; i < l ; i++ ) {
    // attempt to find the peg in the remaining code
    foundIndex = code.indexOf(pegs[i]);
    if( foundIndex !== -1 ) {
      valueMatches++;
      // remove the matched code peg, since it's been matched
      code.splice(foundIndex, 1);
    }
  }

  // Now, return the number of exact and inexact matches
  // as an object, so it's easy to use
  return {
    exactMatches: exactMatches,
    valueMatches: valueMatches,
    totalMatches: exactMatches + valueMatches
  };
}

If you feed the arrays above to evaluateSolution() you'll get this back:

{
  exactMatches: 2,
  valueMatches: 2,
  totalMatches: 4
}

If exactMatches === code.length, then congrats, you've won! Otherwise, use the numbers to render the hint.

Other observations:

You leave a lot of blank lines in your code, but the non-blank lines are pretty crammed (e.g. your whitespace-less for-loop declarations). Give you code (and the reader) some space to breathe.
I looked briefly at the code on GitHub, and there's a lot going on. Too much really. But here's a hint: You can build most of HTML using JavaScript. Copy/pasting the HTML for each "turn" and giving it a new ID is a nightmare to deal with. Just loop and create elements in code.


Read numeric data from a text file using C++ I need to read Numeric Data from a text file that looks like this:

2
cell X-cord Y-cord
1    1.1    2.2
2    2.1    2.0

The first line indicates the dimension (2 or 3), the second line is just trash. The file can be comma separated, so it may look like this:

3
cell X-cord Y-Cord Z-Cord
1,   1.1,  2.2,    3.2

Of course this is just a short example, the real file has hundreds of thousands of cells, so I want to improve the performance.

When I have a comma separated file I use this code:

std::istringstream iss;     //used iterate over the elements in line
while (std::getline(myfile,line))
{
    replace(line.begin(),line.end(),',',' ');  //this replaces commas for spaces
    iss.str(line);          //this will let us iterate over the elements of line
    if (dim==2)             // if it is a 2d problem
    {
        iss>>cell>>x>>y;
        cell_vector.push_back(cell);
        x_vector.push_back(x);
        y_vector.push_back(y);
    }
    else
    {
        iss>>cell>>x>>y>>z;
        cell_vector.push_back(cell);
        x_vector.push_back(x);
        y_vector.push_back(y);
        z_vector.push_back(z);
    }
iss.clear();  //so that we can assing a new line to it
}

When the file is not comma separated I use this code:

int i=0;      //used to know where to save a
while (my_file>>a)
{
    if (dim==2)
    {
        switch(i%3)
        {
            case 0:
                cell_vector.push_back(a);
                break;
            case 1:
                x_vector.push_back(a);
                break;
            case 2:
                y_vector.push_back(a);
                break;
        }
        i++l;
    }
    else
    {
        //similar but for 3 dimensions
    }
}

I have tested both codes independently and they work fine. However, I don't know beforehand if the file is comma separated or not, so I thought of doing something like this:

int dim;
input_v>>dim;  //to get the information regarding dimension

std::getline(input_v,line);
std::getline(input_v,line);
std::getline(input_v,line);   //3 times to skip the "cell X-cord...." part

bool flag_comma=false;   //used to later determine if file is comma separated
for (int i=0;i<=line.length();i++)
{
    if (line[i]==',')
    {
        flag_comma=true;
        break;
    }
}

if (flag_comma)
{
    //code for comma separated file
}
else
{
    //code for space separated file
}

Everything works fine except that the program only starts to save everything starting from cell 2 onwards... I can't save the data from cell 1.

The only way I found around it is to add this:

    replace(line.begin(),line.end(),',',' ');  //this replaces commas for spaces
    iss.str(line);          //this will let us iterate over the elements of line
    if (dim==2)             // if it is a 2d problem
    {
        iss>>cell>>x>>y;
    }
    else
    {
        iss>>cell>>x>>y>>z;
    }
iss.clear();  //so that we can assing a new line to it

Before the if (flag_comma) part, regardless of the case (comma or space). It is the same piece of code used inside the while loop for the comma separated part.

It is also worth noting that the "comma separated code" works just as well when the file has no commas (i.e. it is space separated). The reason I made the space separated part is because I want to improve performance. I don't really know how complex is the replace function.

So my questions are:

A) Do I really improve performance this way? Instead of just keeping the comma separated version of the code?

B) How can I improve my code (in general)?

C) Is there a better way to get the data from cell 1?

D) Can I improve something memory-wise?

A) I don't think that you improved performance of your code in this way. In both methods you get the single numbers with ">>", which is IMHO the best way to get the numbers. So I think it does not matter much.

B) First you can improve your code by regarding the "push_back" commands. This command regularly needs to reallocate your data to fit in a specific part of the main memory. If you know your total number of lines beforehand, you can call "reserve" before the first push_back with the number of lines. Depending on the number of lines you have in total this approach might save you a lot of time. There are more opportunities, just look it up here.
Second you can use binary data instead of a textual representation. In your example the code has to read multiple chars for every single number. With binary data every data has a fixed size, independent of the decimal length. Moreover the numbers then are already in the correct representation memory wise.

C) IMHO your approach is ok.

D) That, I cannot answer. I think there is a way to use streams, if you don't already do this.


Custom nullptr_t class I tried to write a nullptr_t class based on the official proposal to be used in C++03 only. The only differences with the proposal are that we can compare two nullptr_t instances and that it is convertible to bool via an overload to void* to avoid unwanted behaviour such as int a = nullptr; for example. Here is the class:

const class nullptr_t
{
    public:

        // Return 0 for any class pointer
        template<typename T>
        operator T*() const
        {
            return 0;
        }

        // Return 0 for any member pointer
        template<typename T, typename U>
        operator T U::*() const
        {
            return 0;
        }

        // Used for bool conversion
        operator void*() const
        {
            return 0;
        }

        // Comparisons with nullptr
        bool operator==(const nullptr_t&) const
        {
            return true;
        }
        bool operator!=(const nullptr_t&) const
        {
            return false;
        }

    private:

        // Not allowed to get the address
        void operator&() const;

} nullptr = {};

I would like to know if there is any actual flaw in this completed implementation or if there is a difference in the behaviour compared to the C++11 type std::nullptr_t besides the namespace that I can't see.

At the moment, your implementation allows

auto& p = nullptr;

This is forbidden in C++11 as nullptr is an rvalue.  You also do not allow the following:

auto p = nullptr;
auto pp = &p;

While C++11 does allow it.  You are also missing overloads for comparison operators.

A simple workaround would be to remove the operator& overload and add a macro:

#define nullptr (nullptr_t())

Also, I'd generally use struct X { ... } const x; instead of const struct X { ... } x;.


How to optimize this SQL delete I want to optimize the performance of this SQL query. If I populate this hashtable with one million keys the query will take around minute. How I can optimize this Java method for faster execution?

private HashMap<String, Boolean> selectedIds = new HashMap<>();

public void deleteSelectedIDs() throws SQLException {

        if (ds == null) {
            throw new SQLException();
        }

        Connection conn = ds.getConnection();
        if (conn == null) {
            throw new SQLException();
        }

        PreparedStatement ps = null;
        ResultSet resultSet = null;

        try {
            conn.setAutoCommit(false);
            boolean committed = false;
            try {
                String sqlDeleteQuery = "DELETE FROM ACTIVESESSIONSLOG WHERE ASESSIONID = ?";

                Set<String> keySet = selectedIds.keySet();
                String[] keys = new String[]{};
                keys = selectedIds.keySet().toArray(keys);
                ps = conn.prepareStatement(sqlDeleteQuery);

                for (int i = 0; i < keys.length; i++) {
                    if (selectedIds.get(keys[i]).booleanValue()) {
                        ps.setString(1, keys[i]);
                        ps.executeUpdate();
                        ps.clearParameters();
                        selectedIds.put(keys[i], false); //get(keys[i]) = false;
                    }
                }

                conn.commit();
                committed = true;

                //selectedIds.clear();
            } finally {
                if (!committed) {
                    conn.rollback();
                }
            }
        } finally {
            ps.close();
            conn.close();
        }
    }
private HashMap<String, Boolean> selectedIds = new HashMap<>();

public void deleteSelectedIDs() throws SQLException {

        if (ds == null) {
            throw new SQLException();
        }

        Connection conn = ds.getConnection();
        if (conn == null) {
            throw new SQLException();
        }

        PreparedStatement ps = null;
        ResultSet resultSet = null;

        try {
            conn.setAutoCommit(false);
            boolean committed = false;
            try {
                String sqlDeleteQuery = "DELETE FROM ACTIVESESSIONSLOG WHERE ASESSIONID = ?";

                Set<String> keySet = selectedIds.keySet();
                String[] keys = new String[]{};
                keys = selectedIds.keySet().toArray(keys);
                ps = conn.prepareStatement(sqlDeleteQuery);

                for (int i = 0; i < keys.length; i++) {
                    if (selectedIds.get(keys[i]).booleanValue()) {
                        ps.setString(1, keys[i]);
                        ps.executeUpdate();
                        ps.clearParameters();
                        selectedIds.put(keys[i], false); //get(keys[i]) = false;
                    }
                }

                conn.commit();
                committed = true;

                //selectedIds.clear();
            } finally {
                if (!committed) {
                    conn.rollback();
                }
            }
        } finally {
            ps.close();
            conn.close();
        }
    }

Per the other example - step 1 should be to get out your profiler.

In any case -
I suspect you would get significantly reduced round trip times by taking advantage of JDBC batching capability. Here's an example: http://www.tutorialspoint.com/jdbc/jdbc-batch-processing.htm (see the second one - using a prepared statement).

In short - you create your DELETE prepared statement, drop the values into a batch, and then execute the batch in one go. Use a prepared statement to reduce parse time for the server.

Depending on if you want to ramp up the complexity, you might want to split the rows into tasks and run them in parallel threads.


Is usage of friend class appropriate in this case? I want to have a class that stores data from an input file, for use in a simulation program. It is like a struct but may require some small functions for accessors.

A seperate class reads all the data from a text input file. I want to store most of this data in the above mentioned object. Therefore this class needs to be able to set the data variables.

My current approach is to give the loader class friend access, so that these variables can be set. No other classes should be able to set the data, therefore I am not using setters.

Is this appropriate friend class usage?

I have included a demonstration example of the situation, as simple as possible:

In reality the data holder I have called "Pod" here, holds 10 to 20 different strongly related variables.

#include <iostream>
class Pod {
 // holds the data that will be used later in simulation
 friend class Loader;
 public:
    double get_angle() {return angle_ * 3 / 180;};
 protected:
    double angle_;
};

class Loader {
 // loads data from text file.
 public:
    bool CheckFile() {
        // checks file:
        return true;
    }
    Pod ReadData() {
        double a = 90; // this line simulates reading from file.
        Pod pod;
        pod.angle_ = a;
        return pod;
    }
};

void DoesThingsWithData(Pod data) {
    std::cout << data.get_angle() << std::endl;
}

int main() {
    Pod my_data;
    {
        Loader loader;

        if (loader.CheckFile()) {
            my_data = loader.ReadData();
        }
    }
    // do things with data
    DoesThingsWithData(my_data);
    return 0;
}

Personally, I don't have major objections with that design. It gets the job done in a fairly straightforward way. But it does creates coupling, which might become a nuisance to you as the project grows. So another option with less coupling, as mentioned in comments, would be defining a default parameterized constructor:

class Object {
public:
    Object(<several params>);
private:
    <the data>
};

class Loader {
public:
    Object LoadObject()
    {
        // many fields of data...
        int a    = ...
        double b = ...
        std::vector<int> c = ...

        return Object(a, b, std::move(c), ...);
    }
};

In C++11, you also don't have to worry about unnecessary copies of complex objects such as a std::vector. You can make the constructor take a move ref, e.g.: std::vector<int> && vec and apply std::move in the call site.


CSV manipulation with Python I've prepared a script to manipulate csv files in Windows that simply adds quotes as requested.

The script creates a backup of the original files and overwrite the wrong originals in its same folder.

It works as expected but I'd be really happy to learn from gurus how to improve my code, my thinking and my Python.

#!/usr/bin/python
# coding: utf-8
#
#    ______________________________________
#   / CSV Fix                              \
#   \ LtPitt - 25/02/2015                  /
#    --------------------------------------
#           \   ^__^
#            \  (oo)\_______
#               (__)\       )\/\
#                   ||----w |
#                   ||     ||
#
# This script will fix the manually-edited csv files before import into application

import os
import csv
import shutil
import sys
import time

script_path = os.path.dirname(sys.argv[0])
backup_path = script_path + "\\backup\\"
tmp_path = script_path + "\\tmp\\"

os.chdir(script_path)

paths = [backup_path, tmp_path]

for path in paths:
  if not os.path.exists(path):
    os.makedirs(path)

def fixCsv(csv_file):
  # This function fixes the broken csv adding ; and " where needed and saves the fixed file into the script path
  rows = []
  with open(tmp_path + csv_file, 'rb') as csvfile:
    spamreader = csv.reader(csvfile, delimiter=';', quotechar='"')
    for row in spamreader:
      if not ''.join(row).strip():
        pass
      else:
        rows.append(row)

  with open(script_path + "\\" + csv_file, 'wb') as csvfile:
    spamwriter = csv.writer(csvfile, delimiter=';', quotechar='"', quoting=csv.QUOTE_ALL)
    for row in rows:
      spamwriter.writerow(row)

def prepareFiles():
  # This function saves original files in case of errors / reviews and moves originals into tmp folder for elaboration
  current_datetime = str(time.strftime('%Y%m%d%H%M%S') )
  os.makedirs(backup_path + current_datetime)
  for file in os.listdir(script_path):
    if file.endswith(".csv"):
      shutil.copyfile(script_path + "\\" + file, backup_path + current_datetime + "\\" + file)
      shutil.move(script_path + "\\" + file, tmp_path + "\\" + file)

# Let's rock
prepareFiles()
for file in os.listdir(tmp_path):
  if file.endswith(".csv"):
    fixCsv(file)
shutil.rmtree(tmp_path)

Long story short: read the style guide.

Imports should be in alphabetical order;
Function names should be lowercase_with_underscores, not mixedCase;
Functions should start with """docstrings""", not # comments; and
Indents should be four spaces, not two.

time.strftime(...) always returns a string (that's what it's for!) so wrapping it in a str call is redundant.

There should be much less code at the top level of your script - move it into an entry point function (e.g. call it main) then guard its call:

if __name__ == '__main__':
    main()

This makes it easier to import the functionality elsewhere later.

if not ''.join(row).strip():
    pass
else:
    rows.append(row)

could be simplified to:

if ''.join(row):
    rows.append(row)

csv.reader already strips newlines, and there's no point including the pass block.

You can (and should!) use os.path.join instead of e.g. script_path + "\\" + csv_file.


Finding exact matches from a 2D array I have this code that finds exact matches from a 2D array in a range of two columns on a sheet.

It works ok until the data is formatted as a table then it is slow, really slow. There must be a better way to do this.

Sub compareManifest()
Dim manifest As Worksheet: Set manifest = ActiveWorkbook.Worksheets("Manifest")
Dim approved As Worksheet: Set approved = ActiveWorkbook.Worksheets("Approved Master")
Dim result As Range
Dim findInRange As Range
Dim softwareArray As Variant
' 2d array of software name and version
softwareArray = manifest.Range("A2:B" & lastRow(manifest))
' one of several ranges of software names and versions
Set findInRange = approved.Range("E1:E" & lastRow(approved))
For i = LBound(softwareArray) To UBound(softwareArray)
    Set result = findSoftware(approved, softwareArray(i, 1), softwareArray(i, 2), findInRange)
    If Not result Is Nothing Then
        ' write back some data from successful matches which is the whole point of this exercise
        manifest.Cells(i + 1, 3) = result.Offset(0, -2)
    End If
Next i
End Sub

Function findSoftware(sheet, softwareName, softwareVersion, findInRange) As Range
Dim found As Range
Dim firstFoundAddress As String
Set found = findInRange.Find(what:=softwareName, LookIn:=xlValues, lookat:=xlWhole)
If Not found Is Nothing Then
    firstFoundAddress = found.Address
    Do
        If softwareVersion = sheet.Range(found.Address).Offset(0, 1) Then
            Set findSoftware = sheet.Range(found.Address)
            firstFoundAddress = found.Address
        Else
            Set found = findInRange.FindNext(found)
        End If
    Loop While firstFoundAddress <> found.Address
End If
End Function

Consider using a Collection when matching values in multiple list.  Scripting Dictionaries are well suited for this task.  You'll need to create a Key by concatenate the multiple values with a delimiter.  You will then store the Key with its associated Cell reference  in the Dictionary.

This solution can be further optimized using array but should process 2 list with 10k entries each in seconds (less than a second if optimised using Arrays).

Sub CompareLists()
    Dim cell As Object, dic As Object, key As String
    Set dic = CreateObject("Scripting.Dictionary")

    With ActiveWorkbook.Worksheets("Approved Master")
        For Each cell In .Range("E1" & .Range("E" & .Rows.count).End(xlUp))
            key = cell.Value & "|" & cell.Offset(0, 1).Value
            If Not dic.Exists(key) Then dic.Add key, cell
        Next
    End With

    With ActiveWorkbook.Worksheets("Manifest")
        For Each cell In .Range("B2" & .Range("A" & .Rows.count).End(xlUp))
            key = cell.Value & "|" & cell.Offset(0, 1).Value
            If dic.Exists(key) Then dic.Add key, cell.Offset(0, 2).Value = dic(key).Offset(0, -2).Value
        Next
    End With

End Sub

Here is a good reference to learn how to use Scripting Dictionaries: Excel VBA Introduction Part 39 - Dictionaries.


Checkboxes array to delete images Basically I'm displaying an array of images on a page with checkboxes. The idea is when a user is editing a post and the checkbox is checked, the image will get deleted. Each image also has an order_id and description.

I'm pretty new to Ruby and programming and I'm looking for a way to simplify the following code.

Codeclimate is saying:

  Cyclomatic complexity for update is too high

lessons_controller.rb:

def update

@lesson.attachments.each.with_index do |attachment, index|

  unless params[:delete_array].nil?
    if params[:delete_array][index].to_i == attachment.id && params[:delete_array][index].to_i != 0
      puts "DELETE: #{attachment.attachment_file_name}"
      attachment.destroy
    end
  end

  unless attachment.nil?
    unless params[:order_array].nil?
      if params[:order_array][index].to_i != attachment.order
        attachment.order = params[:order_array][index]
        puts "ORDER: #{attachment.order} = #{params[:order_array][index]}"
        @save = true
      end
    end

    unless params[:description_array].nil?
      if params[:description_array][index] != attachment.description
        attachment.description = params[:description_array][index]
        puts "DESCR: #{attachment.description} = #{params[:description_array][index]}"
        @save = true
      end
    end
  end
end

if @save == true
  @lesson.attachments.each(&:save)
end

  respond_to do |format|
  if @lesson.update(lesson_params)
    format.html { redirect_to @lesson, notice: 'Lesson was successfully updated.' }
    format.json { render :show, status: :ok, location: @lesson }
  else
    format.html { render :edit }
    format.json { render json: @lesson.errors, status: :unprocessable_entity }
  end
end
end

view/_form.html.erb

    <% if @lesson.attachments.any? %>
    <button type="button" id="check_all" class="center"><%="Check all/Uncheck all"%></button>

      <% @lesson.attachments.order(:order).each do |attachment| %>

          <%= image_tag attachment.attachment.url(:thumb), class: "thumbnail img-responsive center" %>

          <%= check_box_tag 'delete_array[]', attachment.id, checked = false %> Delete

          <%= number_field_tag 'order_array[]', attachment.order %>
          <%= text_field_tag 'description_array[]', attachment.description, placeholder: "Description" %>

      <% end %>
    <% else %>
      No attachments found.
    <% end %>

I try to start from part dealing with removing of checked attachments.

First, I can't find purpose of puts lines. Only if it used for debugging in development.

Second, @lesson.attachments.each.with_index - every time updating single attachment, you need to go through all existing records. I think, that is what 'Codeclimate' "says". So, that structure must be simplified.

Here is how I suggest to rewrite 'deletion' part:

def update

  params[:delete_array].try :each do |id|
    @lesson.attachments.find(id).destroy
  end

# other code

end

Other parts (for params[:order_array] and params[:description_array]) can be refactored in the same way. I hope, you've got the idea.

If you need more detailed explanation of above code or need help with other parts - let me know and I'll explain it in details.


Return a grouped sum from a list I just started with Elixir and this is what I have now. Is there an easier or better way to write this?

defmodule GroupBySum do
  def sum(col), do: sum(%{}, col)
  defp sum(r, [h|t]) do
    {k, v} = h
    r = r |> Dict.update(k, v, fn(val) -> val + v end)
    sum(r, t)
  end
  defp sum(r, []), do: r
end

list = [a: 1, b: 2, c: 3, a: 2, b: 1, c: 0]
IO.inspect GroupBySum.sum(list) #%{a: 3, b: 3, c: 3}

It can be done several ways.  Instead of explicitly using recursion we can, as @alxndr suggested, make use of the recursion capabilities of the Enum module.  I am going to use Enum.map_reduce simply because I find it useful when troubleshooting to key the items and the accumulator in that form.

In general the solution is the same as yours.

  def sum(list) do
    list |> Enum.map_reduce([],
      fn({key, val}, acc) ->
        {{key, val}, Keyword.update(acc, key, val, &(&1 + val))}
      end
    ) |> elem(1)
  end

The anonmous function in map_reduce accepts and item and an accumulator.  Since the item is a keyword list, it has the form {key,val} so I match that  in the function definition.  The return from this function must be a tuple with the accumulator as the second value.  For testing purposes I returned the same {key, val} as the first item.  The Keyword.update function is the accumulator.  A form of this same routine that helps the intermediate values be visible is:

def sum(list) do
  list |> Enum.map_reduce([],
    fn({key, val}, acc) ->
      x = Keyword.update(acc, key, val, &(&1 + val))}
      IO.inspect(x)
      {{key, val}, x}
    end
  ) |> elem(1)
end

In the case of the input you provided, the IO.inspect helps me understand how the routine collected the desired result, as seen below.

[a: 1]
[a: 1, b: 2]
[a: 1, b: 2, c: 3]
[a: 3, b: 2, c: 3]
[a: 3, b: 3, c: 3]
[a: 3, b: 3, c: 3]
[a: 3, b: 3, c: 3]

I found this a fun exercise -- thanks for posting.


Basic Pong game in Pygame I have just wrote this Pong game in Pygame:

import pygame
import sys
import math

class Ball(object):
    def __init__(self, x, y, width, height, vx, vy, colour):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self.vx = vx
        self.vy = vy
        self.colour = colour

    def render(self, screen):
        pygame.draw.ellipse(screen, self.colour, self.rect)

    def update(self):
        self.x += self.vx
        self.y += self.vy

    @property
    def rect(self):
        return pygame.Rect(self.x, self.y, self.width, self.height)

class Paddle(object):
    def __init__(self, x, y, width, height, speed, colour):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self.vx = 0
        self.speed = speed
        self.colour = colour

    def render(self, screen):
        pygame.draw.rect(screen, self.colour, self.rect)

    def update(self):
        self.x += self.vx

    def key_handler(self, event):
        if event.type == pygame.KEYDOWN:
            if event.key == pygame.K_LEFT:
                self.vx = -self.speed
            elif event.key == pygame.K_RIGHT:
                self.vx = self.speed
        elif event.key in (pygame.K_LEFT, pygame.K_RIGHT):
                self.vx = 0

    @property
    def rect(self):
        return pygame.Rect(self.x, self.y, self.width, self.height)

class Pong(object):
    COLOURS = {"BLACK": (  0,   0,   0),
               "WHITE": (255, 255, 255),
               "RED"  : (255,   0,   0)}
    def __init__(self):
        pygame.init()
        (WIDTH, HEIGHT) = (640, 480)
        self.screen = pygame.display.set_mode((WIDTH, HEIGHT))
        pygame.display.set_caption("Lewis' Pong")
        self.ball = Ball(5, 5, 50, 50, 5, 5, Pong.COLOURS["BLACK"])
        self.paddle = Paddle(WIDTH / 2, HEIGHT - 50, 100,
                             10, 3, Pong.COLOURS["BLACK"])
        self.score = 0

    def play(self):
        clock = pygame.time.Clock()
        while True:
            clock.tick(50)
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    pygame.quit()
                    sys.exit()
                if event.type in (pygame.KEYDOWN, pygame.KEYUP):
                    self.paddle.key_handler(event)

            self.collision_handler()
            self.draw()

    def collision_handler(self):
        if self.ball.rect.colliderect(self.paddle.rect):
            self.ball.vy = -self.ball.vy
            self.score += 1

        if self.ball.x + self.ball.width >= self.screen.get_width():
            self.ball.vx = -(math.fabs(self.ball.vx))
        elif self.ball.x <= 0:
            self.ball.vx = math.fabs(self.ball.vx)

        if self.ball.y + self.ball.height >= self.screen.get_height():
            pygame.quit()
            sys.exit()
        elif self.ball.y <= 0:
            self.ball.vy = math.fabs(self.ball.vy)

        if self.paddle.x + self.paddle.width >= self.screen.get_width():
            self.paddle.x = self.screen.get_width() - self.paddle.width
        elif self.paddle.x <= 0:
            self.paddle.x = 0

    def draw(self):
        self.screen.fill(Pong.COLOURS["WHITE"])

        font = pygame.font.Font(None, 48)
        score_text = font.render("Score: " + str(self.score), True,
                                 Pong.COLOURS["RED"])
        self.screen.blit(score_text, (0, 0))

        self.ball.update()
        self.ball.render(self.screen)
        self.paddle.update()
        self.paddle.render(self.screen)

        pygame.display.update()

if __name__ == "__main__":
    Pong().play()

This is my first ever game in Pygame, so I'm asuming there's a few bad practices. Can anyone improve my code and find those bad practices?

Because Paddle and Ball have a lot in common, I think it would make sense if they were to inherit from a common class (PhysicalObject for instance) with size, position, velocity, color, etc...

self.ball.vy = -self.ball.vy can be written self.ball.vy *= -1.

You could try to handle collision in a slightly more generic way by creating 4 walls inheriting from PhysicalObject around the area and then handle the collisions between 2 PhysicalObject. Also, it would be nice to have some fancy behavior like the speed of the paddle having an impact on the speed of the ball when they interact.


Parsing a string into a double I'm trying to understand the strtod() function, and how I can handle any user input given to the function. I assume I tested for everything, however I am hoping for a review to make sure and to catch anything I missed. (I know I should separate everything into functions, however, I am just trying to understand the nature of the function, therefore I left everything in main.)

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

/*@param MAXSIZE max number of chars that are converted to a 64bit double*/
#define MAXSIZE 1077

int main() {

    char str[MAXSIZE];
    printf("Enter a rational number: ");

    for (;;) {
        if (fgets(str, sizeof str, stdin) != NULL) {
            if (strchr(str, '\n')) {
                str[strcspn(str, "\n")] = '\0';
            }
            if (str[0] != '\0') {
                break;
            }
            printf("\nTry again: ");
        }
    }

    char* endptr = NULL;
    errno = 0;
    double number = strtod(str, &endptr);

    //passes over trailing whitespace
    for (; isspace(*endptr); ++endptr);

    if (errno == ERANGE) {
        fprintf(stderr, "Error out of range...\n");
    }
    else if (*endptr != '\0') {
        fprintf(stderr, "error could not convert: %s\n", str);
    }
    else {
        printf("Your string was converted into the rational number: %lf\n", number);
    }
    printf("Your string was: %s\n", str);
    printf("Press any key to continue...\n");
    getch();
}

str[strcspn(str, "\n")] = '\0'; is an overkill. fgets reads (at most) one string, that is there is at most one '\n' in the buffer, and strchr will return its position. Consider

    char * newline = strchr(str, '\n');
    if (!newline) {
        .... // handle input too long condition
    } else if (newline == str) {
        .... // handle empty input condition
    }
    *newline = '\0';
    break;

Notice that even newline = '\0' is redundant: '\n' is a whitespace.

Do not invent error messages. This is what strerror and perror are for. Also keep in mind that strtod may set errno to errors other than ERANGE (some implementations use EINVAL as well). Consider

    if (errno != 0) {
        perror("strtod");
    }


Determining the number of days in a month Overall logic: determine number of days in a month. Loop through table (wsUploadTable) and increment value if condition is met. Loop to next day in month and repeat.

For example: 10/1/2016, loops through table for date match and increment value. Next date, 10/2/2016 loop table for match... until last day of month 10/31/2016, loop table, find match and increment value.

'Determine DaysinMonth and assign DaysinMonth_Distro value
DaysInMonth = DateSerial(dtTrickle_Year, dtTrickle_Month + 1, 1) - _
              DateSerial(dtTrickle_Year, dtTrickle_Month, 1)
DoM_Distro = 1 / DaysInMonth

ReDim Days(1 To DaysInMonth)
For i = 1 To DaysInMonth
    Days(i) = DateSerial(dtTrickle_Year, dtTrickle_Month, i)

  'loop table and increment cell value if condition is met

    With wsUploadTable
    lngER_PrimaryID = .Cells(1048576, 2).End(xlUp).Row
        For intPrimaryID = 2 To lngER_PrimaryID
            'store current cell value
            dblLeadsValue = .Cells(intPrimaryID, col_Upload_Leads)
            'match UploadTable row on user input and increment new value
              If.Cells(intPrimaryID, 3).Value = Days(i) Then
              .Cells(intPrimaryID, 11).Value = dblLeadsValue + DoM_Distro
            End If
        Next 'Next PrimaryID
    End With
Next i

Well havent tried either Excel or VBA , but the logic will go like this ,

Loop through the table
Say the date is day/month/year , where day,month and year are variables
lets have an array of months which is [31,28,31,30,31,30,31,31,30,31,30,31]
if year is divisible by 4 or 400 , then months[1]= months[1] + 1
now just display months[month]

Here months is the array and month is the number of month present in the date , for more info in arrays for VBA you can look here .


Swift Hackerrank 2D Array I solved this question in Swift, but was wondering if there is a better way or if my code can be improved:

  Given a 6x6 2D Array:

1 1 1 0 0 0

0 1 0 0 0 0

1 1 1 0 0 0

0 0 0 0 0 0

0 0 0 0 0 0

0 0 0 0 0 0

  We define an hourglass in to be a subset of values with indices
  falling in this pattern in's graphical representation:

a b c

   d

e f g

  There are hourglasses in, and an hourglass sum is the sum of an hourglass' values.

import Foundation

var inputAsArray = [Int]()
for _ in 0...5 {
    inputAsArray += readLine()!.componentsSeparatedByString(" ").map{ Int($0)! }
}
//var inputAsArray = [
//    1, 1, 1, 0, 0, 0,
//    0, 1, 0, 0, 0, 0,
//    1, 1, 1, 0, 0, 0,
//    0, 9, 2, -4, -4, 0,
//    0, 0, 0, -2, 0, 0,
//    0, 0, -1, -2, -4, 0
//]

// method creates all possible "hourglasses" and returns a nested array of hourglasses
typealias hourglass = [Int]
private func hourGlasses(input: [Int]) -> [hourglass] {
    var hourglasses = [hourglass]() // initializes hourglass to hold array of hourglass
    for index in 0..<22 { // there are 36 indexes (6x6) but we +14 in the loop below when creating the hourglass so we only need to go up to 24. We can't make hourglass with 23 and 24, so we go up to 22
        guard [4,5,10,11,16,17].indexOf(index) == nil else {continue} // continue to next iteration because we can't create hourglass with the last 2 indexes from the right wall
        hourglasses.append([ // creates the hourglass array
                input[index], input[index+1], input[index+2],
                                        input[index+7],
            input[index+12], input[index+13], input[index+14],
            ])
    }
    return hourglasses
}

// method takes in array of input and sends it to hourGlasses to make a nested array. Then it finds the largest sum by using reduce over the nested array
private func maxHourglassSum(input: [Int]) -> Int {
    let hourglasses: [hourglass] = hourGlasses(input)
    // reduce the nested arrays to get sum in each. then compare that sum to maxSum
    var maxSum = Int?() // set to optional instead of 0 because all numbers in input can be negative
    for singleHourglass in hourglasses {
        let singleHourglassSum = singleHourglass.reduce(0, combine: +) // reduce adds up sum for a single hourglass
        if maxSum == nil || singleHourglassSum > maxSum {
            maxSum = singleHourglassSum
        }
    }
    return maxSum!
}

print(maxHourglassSum(inputAsArray))

Your code works correctly, but can be improved (simplified and made clearer and faster).

Let't start with

func hourGlasses(input: [Int]) -> [hourglass] {
    var hourglasses = [hourglass]() // initializes hourglass to hold array of hourglass
    for index in 0..<22 { // there are 36 indexes (6x6) but we +14 in the loop below when creating the hourglass so we only need to go up to 24. We can't make hourglass with 23 and 24, so we go up to 22
        guard [4,5,10,11,16,17].indexOf(index) == nil else {continue} // continue to next iteration

        // ...
}

There are far too many magic numbers.
The fact that extened comments are necessary  indicates that the approach
it be too complicated.
I would consider this usage of guard a bit artificial.
Membership in a collection can be checked with contains(), you don't need the
actual index.

How would you extend this to 15x20 arrays? – Better iterate
over the permitted rows and columns:

for row in 0 ..< 4 {
    for col in 0 ..< 4 {
        index = row * 4 + col
        // ...
    }
}

Even better, replace the integer literals by descriptive constants:

let numRows = 6
let numCols = 6

for row in 0 ..< numRows - 2 {
    for col in 0 ..< numCols - 2 {
        let index = row * numCols + col
        // ...
    }
}

Your hourGlasses() function computes an array of arrays. Then in maxHourglassSum(),
the sum of each inner array is computed and the maximal element determined.

It is more effective to sum the values of an "hour glass" immediately instead of
storing it as an array. To compute the maximum value, use the existing method maxElement().

func hourGlassSum(input: [Int], atIndex index: Int) -> Int {
    var sum = input[index] + input[index + 1] + input[index + 2]
    sum += input[index + numCols + 1]
    sum += input[index + 2*numCols] + input[index + 2*numCols + 1] + input[index + 2*numCols + 2]
    return sum
}

func maxHourglassSum(input: [Int]) -> Int {
    var values: [Int] = []
    for row in 0 ..< numRows - 2 {
        for col in 0 ..< numCols - 2 {
            let index = row * numCols + col
            values.append(hourGlassSum(input, atIndex: index))
        }
    }
    return values.maxElement()!
}

(Remark: The sum of is computed in three separate steps because otherwise the compiler
fails with an "Expression is too complex ..." error.)

This is short, clear, works with other dimentions as well, and is faster than your original code.
If even more speed is required then you can compute the maximum "on the fly" instead of storing the
values in an array first:

func maxHourglassSum(input: [Int]) -> Int {
    var maxSum = -100 // Any initial value < -9*7 = -63
    for row in 0 ..< numRows - 2 {
        for col in 0 ..< numCols - 2 {
            let index = row * numCols + col
            let sum = hourGlassSum(input, atIndex: index)
            if sum > maxSum {
                maxSum = sum
            }
        }
    }
    return maxSum
}

More suggestions: If you plan to use the code for other shapes as well
then use an array with the indices describing the shape:

let hourGlassShape = [0, 1, 2, 7, 12, 13, 14]

func hourGlassSum(input: [Int], atIndex index: Int) -> Int {
    return hourGlassShape.reduce(0) { $0 + input[index + $1] }
}

Finally: The challenge is called "2D Array", so you might want to work with
a two-dimensional (i.e. a nested) array in Swift as well:

let inputArray2D = [
    [ 1, 1, 1, 0, 0, 0 ],
    [ 0, 1, 0, 0, 0, 0 ],
    [ 1, 1, 1, 0, 0, 0 ],
    [ 0, 0, 2, 4, 4, 0 ],
    [ 0, 0, 0, 2, 0, 0 ],
    [ 0, 0, 1, 2, 4, 0 ]
]

The sum calculation for a single hourglass then becomes more intuitive:

var sum = input[row][col] + input[row][col + 1]] + input[row][col + 2]
sum += input[row + 1][col + 1]
sum += input[row + 2][col] + input[row + 2][col + 1] + input[row + 2][col + 2]

Test code:

var sum = 0
let startTime = NSDate()
for _ in 1 ... 100 {
    sum += maxHourglassSum(inputAsArray)
}
let endTime = NSDate()
print(endTime.timeIntervalSinceDate(startTime) * 1000.0, "ms")

Results (on a MacBook, compiled in Release mode):

Original code: 0.8 ms
First variation: 0.25 ms
Second variation: 0.008 ms

The values did not change significantly when using a 2D (nested) array instead of a
1D (simple) array.


Returning an array of integers with 1000 elements Write a function that returns an array of integers with 1000 elements
  containing the values 1 to 1000 in random order. No number can be
  repeated or omitted.Your answer should show good programming style,
  technique and attention to accuracy and performance.

  Hint: use Random rnd=new Random() to create a new instance of the
  random number generator and rnd.Next(1,1000) to get a random integer
  value between 1 and 1000. No other .Net framework classes should be
  needed outside of the intrinsic data types (i.e. DO NOT use
  collections).

For that I have developed the following code:

import java.util.Random;

public class Domain {

public int[] randomIntArray(int size) {

    Random random = new Random();
    int intArrayHolder[] = new int[size];

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

        boolean isDuplicate = false;

        /*
         * because random.nextInt(int var) return between 0 and given-no and
         * we do not want 0. So size +1 and later in program, we do not
         * assign 0 to the intArrayHolder.
         */

        int randomNextInt = random.nextInt(size + 1);

        for (int j = 0; j < i; j++) {
            if (intArrayHolder[j] == randomNextInt) {

                isDuplicate = true;
                break;
            }
        }

        if ((!isDuplicate) && (randomNextInt != 0)) {
            intArrayHolder[i] = randomNextInt;
        } else {

            i--;

        }

    }

    return intArrayHolder;
}

}

JUnit test class:

import java.util.HashSet;
import java.util.Set;

import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public class DomainJunitTest {

Domain domain = null;
int arraySize = 0;

@Before
public void initialize() throws Exception {
    domain = new Domain();
    arraySize = 1000;
}

@Test
public void randomIntArray_size_check() {

    int intArrayHolder[] = domain.randomIntArray(arraySize);

    Assert.assertEquals(arraySize, intArrayHolder.length);
}

@Test
public void randomIntArray_duplicate_value_check() throws Exception{

    int intArrayHolder[] = domain.randomIntArray(arraySize);

    Set<Integer> intSet = new HashSet<>();

    boolean isDublicate = false;

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

        if (intSet.contains(intArrayHolder[i])) {
            isDublicate=true;
        }else{
            intSet.add(intArrayHolder[i]);
        }
    }

    Assert.assertEquals(false, isDublicate);

}

@Test
public void randomIntArray_size_check_after_removing_dublicate() {

    int intArrayHolder[] = domain.randomIntArray(arraySize);

    Set<Integer> intSet = new HashSet<>();

    for (int i = 0; i < intArrayHolder.length; i++) {
        intSet.add(intArrayHolder[i]);
    }

    Assert.assertEquals(arraySize, intSet.size());

}

}

And I am being advised it is not good enough. Can someone point out where I went wrong?

Your approach is not incorrect. It will get the job done (albeit, slower). If you want to improve your approach, keep reading. But first a few points on your code:

Points on Your code

intArrayHolder seems a bit wordy to me. I believe intArray or even array would serve your purpose, without sacrificing readibility. Same for randomNextInt.
for (int i = 0; i < intArrayHolder.length; i++) is redundant because the length of intArrayHolder will be equal to size owing to the fact that you declared it like so. for (int i = 0; i < size; i++) will serve you better.
Please note that random.nextInt(size + 1) does  not return an integer between 1 and 1000. It returns an integer in the range \$[0, 1001)\$, i.e. a number \$x\$ such that \$0\leq x < 1001\$. If you do a count, you will find that you are actually getting \$1001\$ elements. The extra number is \$0\$. To fix it, you must write int randomNextInt = random.nextInt(size) + 1; It is quite easy to understand: you just add a \$1\$ to shift the range from \$[0,1000)\$ to \$[1,1001)\$, i.e. the correct range.
((!isDuplicate) && (randomNextInt != 0)) can  be shortened to !(isDuplicate || ramdomNextInt == 0), by using De Morgan's laws. It would be even better if you flip the condition and force next iteration before adding the element to the array.
A general advice is to update the loop counter only once. People generally estimate the number of times the loop will execute by stealing a glance at the for (... statement. A while loop is preferred when the number of iterations is unknown or updation is irregular.
You seem to be using snake_case along with camelCase. The standard in Java is to use camelCase.

Therefore, the refactored code would be:

import java.util.Random;

public class Domain {

    public int[] randomIntArray(int size) {

        Random random = new Random();
        int array[] = new int[size];

        int i = 0;

        while (i < size) {
            boolean isDuplicate = false;
            int randomInteger = random.nextInt(size) + 1;

            for (int j = 0; j < i; j++) {
                if (array[j] == randomInteger) {
                    isDuplicate = true;
                    break;
                }

            }
            if (isDuplicate) {
                continue;
            } else {
                array[i++] = randomInteger;
            }

        }
        return array;
    }
}

A Better Solution

Your solution's best case running time is \$\Omega(n^2)\$. Moreover, as Boris the Spider pointed out: "... there is absolutely no guarantee that Random.nextInt() won't decide to return 1 for ever and ever. ..." Therefore, it might be possible that your code never terminates (although it will be very unlikely). You can make it faster in the following manner:

import java.util.Random;

public class Domain {

    public int[] randomIntArray(int size) {

        Random random = new Random();
        int array[] = new int[size];
        // populate the array with sequential values
        for (int i = 0; i < size; i++) {
            array[i] = i + 1;
        }
        // "shuffle" the array
        for (int i = 0; i < size; i++) {
            int j = random.nextInt(size - i) + i;
            // swap
            int temp = a[i];
            a[i] = a[j];
            a[j] = temp;
        }
        return array;
    }
}

It's running time is linear i.e. \$\Theta(n)\$1. I am basically describing the Fisher-Yates shuffle

Some Background Info: The Big-O stuff

The Big-O notation is used to define an upper limit of the number of operations executed (the maximum running time, in simpler terms). For example, if the exact number of iterations based on a given parameter (like the number of elements, digits, etc) called \$n\$ is
$$2n^2+3n-4$$
we can say that a reasonable upper limit is \$3n^2\$. In the Big-O notation, the constant term gets absorbed. When we represent this in the Big-O notation, we say: "the (maximum) running time is proportional to \$O(n^2)\$".

People tend to forget the other two notations: the Big-Omega (\$\Omega\$) and the Big-Theta (\$\Theta\$) The former defines a lower limit and the latter is the Big-O and the Big-Omega combined together. In the example above, the lower limit can be \$2n^2\$. Absorbing the constant, we say:"the (minimum) running time is proportional to \$\Omega(n^2)\$". The function can therefore be bounded within both upper and lower limits, or both the upper and the lower limits are in the order of \$\Theta(n^2)\$.

Your code has no upper limit. It only has a lower limit. So, only the \$\Omega\$ is defined for your code (unless we set out to do a lengthy mathematical analysis of the behaviour of the nextInt() function).

I suggest that you read the related Wikipedia articles. (I didn't cover \$o, \omega\$ and \$\theta\$, that's your homework). Remember, Google is your friend!

1 Assuming that the nextInt() method takes \$O(1)\$ (i.e. constant) time to execute.


Python bubble sort algorithm How can this algorithm be improved? I mean, a list with 3000 items takes about 5 seconds to sort, according to Sublime Text. Using Python's sorted function is immediate.

from random import randint

def bubble_sort(iterable):
    while True:
        corrected = False
        for item in range(0, len(iterable) - 1):
            if iterable[item] > iterable[item + 1]:
                iterable[item], iterable[item + 1] = iterable[item + 1], iterable[item]
                corrected = True
        if not corrected:
            return iterable

if __name__ == '__main__':
    random_list = [randint(0, 100) for _ in range(3000)]
    print(bubble_sort(random_list))

Python's sorted() uses an adaptive sorting algorithm called "timsort" (from its author name Tim Peters), which has worst case complexity of \$O(n log n)\$.

Bubble Sort on the other hand has best, worst and average time complexity of \$O(n ^ 2)\$ which is much worse than \$O(n log n)\$ with growing \$n\$:

In other words, the Bubble Sort sorting algorithm is inefficient by definition.

We can though still apply some optimizations - adjust the end of the working range - skipping every last item on each iteration since it is already in place:

def bubble_sort_new(iterable):
    swapped = True
    while swapped:
        swapped = False
        end = len(iterable) - 1

        for item in range(0, end):
            if iterable[item] > iterable[item + 1]:
                iterable[item], iterable[item + 1] = iterable[item + 1], iterable[item]
                swapped = True

        end -= 1

    return iterable

My timeit results:

In [1]: import random

In [2]: random_list = [randint(0, 100) for _ in range(3000)]

In [3]: %timeit -r 100 bubble_sort(random_list)
1000 loops, best of 100: 402 µs per loop

In [4]: %timeit -r 100 bubble_sort_new(random_list)
1000 loops, best of 100: 382 µs per loop


Permutation function in C# One thing lead to another and eventually I wanted to know the "proper" way of writing the permutation generating function in C#. Below is my code for generating permutations. This will go into an internal library, so the calling context is really generic. Expected calling is in a single-threaded context

/// <summary>
/// Generate all possible permutations of all elements in the list taken r at a time
/// Permutations: Choose 'r' letters from 'n' possible letters to form a word (sequence is important).
/// Formula for counting: nPr = n! / (n - r)!
/// </summary>
/// <param name="list">The list whose permutations are needed</param>
/// <param name="startPos">The position of first element of interest in the list</param>
/// <param name="r">Number of items required in the permuted list</param>
/// <returns>An enumeration containing a permutation of the original list</returns>
public static IEnumerable<IList<T>> GetPermutations<T>(this IList<T> list, int r, int startPos = 0) {
    for (int i = startPos; i < list.Count; ++i) {
        if (r > 1) {
            list.Swap(startPos, i);
            foreach (var permute in GetPermutations(list, r - 1, startPos + 1))
                yield return new List<T>().AddRange(list[startPos].AsEnumerable(), permute);
            list.Swap(startPos, i);
        } else
            yield return new List<T>() { list[i] };
    }
}

The Utils class has the extensions used in the GetPermutations function

public static class Utils {
    public static bool Swap<T>(this IList<T> list, int i1, int i2) {
        if (i1 == i2)
            return false;
        var tmp = list[i1];
        list[i1] = list[i2];
        list[i2] = tmp;
        return true;
    }

    public static IEnumerable<T> AsEnumerable<T>(this T item) {
        yield return item;
    }

    public static List<T> AddRange<T>(this List<T> list, params IEnumerable<T>[] collections) {
        foreach (var collection in collections)
            list.AddRange(collection);
        return list;
    }

}

The code will be invoked as below

var items = new [] { 'a', 'b', 'c', 'd'};
foreach (var permutation in items.GetPermutations(3))
    Console.WriteLine(string.Join(", ", permutation));

Couple of things on which I would like to get feedback are:

yield return in a recursive function.
Generation of new Lists to be yield returned, given that we will have a really large number of permutations with small increase in the input set

To answer your (1) yield return in a recursive method ... Eric Lippert does it in Producing Permutations series, Part Three , specifically in his HamiltonianPermutationsIterator recursive method.

I won't dive into your (2) only to suggest that perhaps you read all of Eric Lippert's series to learn more.

Braces Need Improving

On coding style, your use (or lack thereof) of braces could be improved.  The C# convention is to put the opening brace on a new line (this differs from Java).

More importantly is that you lack braces on so many one-liners.  This is heavily discouraged.

I would suggest cleaning your code to change a snippet from this:

if (r > 1) {
    list.Swap(startPos, i);
    foreach (var permute in GetPermutations(list, r - 1, startPos + 1))
        yield return new List<T>().AddRange(list[startPos].AsEnumerable(), permute);
    list.Swap(startPos, i);
} else
    yield return new List<T>() { list[i] };

To this:

if (r > 1)
{
    list.Swap(startPos, i);
    foreach (var permute in GetPermutations(list, r - 1, startPos + 1))
    {
        yield return new List<T>().AddRange(list[startPos].AsEnumerable(), permute);
    }
    list.Swap(startPos, i);
}
else
{
    yield return new List<T>() { list[i] };
}

Swap Method

You really don't do anything with the bool returned from the Swap method.  Plus I see little reasoning in not swapping 2 values that happen to be equal.  This method could simply be void.

You are also encouraged to have meaningful names in C#, so i1 would be better named as index1.  Ditto for i2.

public static void Swap<T>(this IList<T> list, int index1, int index2)
{
    var temp = list[index1];
    list[index1] = list[index2];
    list[index2] = temp;
}


Three sum problem using HashMap in Java Given an array and a value, find all the triplets in the array whose sum is equal to the given value. For example, if the given array is {12, 3, 4, 1, 6, 9} and the given sum is 24, then this is one triplet (12, 3 and 9) which contributes to the total sum of 24.
Solution for given example:
6, 9, 9
6, 6, 12
3, 9, 12
Conditions:

The ordering of the numbers in the solution does not matter.

Duplicate triplet is not allowed.

A number is not allowed to be used multiple times.

This question was asked here.
Here is my code which addresses all the issues. One concern is the code looks a little muddy which I want to avoid in the production grade code. Any constructive feedback is appreciated.
Runtime: O(n2)
GitHub
public class ThreeSum {

    static class Triplet {
        final List<Integer> triplets;

        public Triplet(int x, int y, int z) {
            triplets = Arrays.asList(x, y, z);
            Collections.sort(triplets);
        }

        @Override
        public int hashCode() {
            return Objects.hash(triplets.get(0), triplets.get(1), triplets.get(2));
        }

        @Override
        public boolean equals(Object o) {
            if (o instanceof Triplet) {

                Triplet other = (Triplet) o;
                return other.triplets.get(0) == this.triplets.get(0) &&
                        other.triplets.get(1) == this.triplets.get(1) &&
                        other.triplets.get(2) == this.triplets.get(2);
            }

            return false;
        }

        @Override
        public String toString() {
            return String.format("(%d, %d, %d)", triplets.get(0), triplets.get(1), triplets.get(2));
        }
    }

    public static Set<Triplet> findTriplets(int numbers[], int targetSum) {
        Set<Triplet> triplets = new HashSet<>();

        // insert all pairs in the look up table
        Map<Integer, int[]> lookup = new HashMap<>();
        for (int i = 0; i < numbers.length - 1; i++) {
            for (int j = i + 1; j < numbers.length; j++) {
                int total = numbers[i] + numbers[j];
                lookup.put(total, new int[]{i, j});
            }
        }

        // look for the complement, if found viola! here you go with the triplet
        for (int currentIndex = 0; currentIndex < numbers.length; currentIndex++) {
            int complement = targetSum - numbers[currentIndex];

            if (lookup.containsKey(complement)) {
                int indexes[] = lookup.get(complement);

                if (currentIndex != indexes[0] && currentIndex != indexes[1]) {
                    int x = numbers[indexes[0]], y = numbers[indexes[1]];
                    triplets.add(new Triplet(x, y, numbers[currentIndex]));
                }
            }
        }

        return triplets;
    }
}

You are storing the integers of a Triplet as a List internally, yet you are not really taking advantage of the interface List's functionality. For example:

return Objects.hash(triplets.get(0), triplets.get(1), triplets.get(2));

Why not simply do this:

return triplets.hashCode();

Or, another example:

return other.triplets.get(0) == this.triplets.get(0) &&
        other.triplets.get(1) == this.triplets.get(1) &&
        other.triplets.get(2) == this.triplets.get(2);

This can be replaced by this:

return other.triplets.equals(this.triplets);

There's an issue I didn't catch in the old question but which was already present there: Your code doesn't consider the possibility that the same sum is formed by more than one pair of numbers, because you are simply mapping integers to one pair of indices, and if you encounter a pair that adds up to a sum that is already stored in lookup, the old index pair gets overridden.

A way to overcome this problem could be to map the integers to a List<int[]>), but this would make things even more complicated, and I think the reason that this would get overly complicated is that you are, in fact, hard-coding a recursive process (first you create the pairs, then from the pairs you create the triplets etc.). You could instead try to write a method that accumulates all possible n-tuplets, with n being any arbitrary positive integer, through recursion, e.g. like this:

If n is 1, then return an array/list of all integers
If n is greater than 1, then iterate through the original array and, for every integer in it, combine this integer with all n-1-tuplets formed by the numbers that follow this integer in the array (taking only the following numbers and not the preceding numbers prevents duplicate tuplets that only differ in the order of the integers).

I imagine that this would be much easier to code than what you currently have (maybe this is also what you mean by your code being "muddy"), and also much more flexible. True, if it's just about pairs, then it might not be as fast as what you did here, but if it comes to tuplets of more than 2 numbers, I don't think there is much benefit of what you are trying to do here (although this is just a guess, I didn't actually benchmark it).


Constructor start, destructor end timer in C++ The following is a short timing utility I've written for a larger project that I'm working on. The larger project is designed to be called (via ffi) from a Scala/Java codebase, which makes using standard profiling tools somewhat difficult - especially at the granularity that we want.

Essentially, we want to be able to track roughly how long a few key functions are taking, and we want a low overhead way of doing so. The following code is my implementation of a timer class that starts timing when constructed, and then finishes (and reports data) when it destructs.

I chose this way of doing things as it gives quite a succinct way of benchmarking a function, take for example, a function foo that we wanted to benchmark, all we would have to do is the following:

void foo(/*some arguments*/) {
  auto timer = CSDSTimer("foo", "example");
  /*do some more stuff*/
}

I think this is quite a nice solution, but I'd be interested in feedback on my implementation, especially of my handling of streams, and whether there's any way to set the stream statically (see the commented out code).

Thanks!

#pragma once

#include <chrono>
#include <ctime>
#include <iostream>

class CSDSTimer {

public:
  // Construct and start the timer
  CSDSTimer(std::string name, std::string context = "global",
            std::ostream &stream = std::cout)
      : _name(name), _context(context), _default_str(&stream) {
    _start = std::chrono::system_clock::now();
  }

  // Destruct and stop the timer, reporting the elapseed time
  ~CSDSTimer() {
    _end = std::chrono::system_clock::now();
    std::chrono::duration<double> elapsed_seconds = _end - _start;
    *_default_str << "PROFILING_DATUM(\"" << _name << "\", \"" << _context
                  << "\", " << elapsed_seconds.count() * 1000 << ", \"C++\")"
                  << std::endl;
  }

  // static void SetStream(std::ostream &str) { _default_str = &str; }

private:
  std::string _name;
  std::string _context;
  std::chrono::time_point<std::chrono::system_clock> _start, _end;
  std::ostream *_default_str;

  // static std::ostream *_default_str;
};

Design:

The problem I have is that you need to modify the thing you want to time (which means changing the timing).

  void functionIWantToTime()
  {
       CSDSTimer time("functionIWantToTime");

       // Stuff that takes time
  }

What would be better is a way to time a function without modifying it.

 CSDSTimer("functionIWantToTime", &functionIWantToTime);

This also allows you to time random set of stuff that is not a function by using a lambda to combine them.

 CSDSTimer("Stuff", [](){
     function1();
     function2();
 )};

Code Review

Pass big parameters by const reference

To avoid copying objects pass by const reference.

CSDSTimer(std::string name, std::string context = "global", std::ostream &stream = std::cout)
                  ^^^^^^ Add const&  ^^^^^

Avoid holding pointers:

std::ostream *_default_str;

You pass by reference. Why not store a reference?

Prefer '\n' over std::endl

The difference is a flush. Flushing is usually sub-optimal; the stream will flush at the optimal time. Manual flushing usually just retards performance.

Timer Accuracy

I did not spot it. But @hoffmale suggested using a more accurate timer. I agree that is probably a good idea (you have nothing to lose with more accuracy).


IsNullOrWhiteSpace check for Java I am needing to write my own IsNullOrWhiteSpace check for Java but am wondering if this is the best method to do so

public static boolean isStringNullOrWhiteSpace(String value) {
    if (isStringNullOrEmpty(value)){
        return true;
    }

    for (Character chars : value.toCharArray()){
        if (!Character.isWhitespace(chars)) return false;
    }

    return true;
}

I agree that the String should be left out of the name, as this is quite obvious in StringUtils. How about this short version?

public static boolean isNullOrWhiteSpace(String value) {
    return value == null || value.trim().isEmpty();
}