Java sort by selection: which is better in regards to readability and efficiency? Which algorithm do you think is better in regards to readability and efficiency?

selection():

public static int[] selection(int[] tab) {
    int minor;
    int aux;
    for (int i=0; i<tab.length-1; i++) {
        minor = tab[i];
        for (int j=i+1; j<=tab.length-1; j++) {
            if (tab[j] < minor) {
                aux = tab[j];
                tab[j] = minor;
                minor = aux;
            }
        }
        tab[i] = minor;
    }
    return tab;
}

otherSelection():

public static int[] otherSelection(int A[]) {
    int i, j, minor, pos, tmp;
    for (i = 0; i < A.length - 1; i++) {
        minor = A[i];
        pos = i;
        for (j = i + 1; j < A.length; j++){
            if (A[j] < minor) {
                minor = A[j];
                pos = j;
            }
        }
        if (pos != i){
            tmp = A[i];
            A[i] = A[pos];
            A[pos] = tmp;
        }
    }
    return A;
}

Readability:

The first one is far more readable because it can be translated at so:

selection(tab)
    for each value of tab: i = (0->size-1)
        minor = tab[i]
        for each value of tab: j = (i->size-1)
            if (tab[j] < minor)
                swap(minor, tab[j])
        tab[i] = minor;
    return tab;

This is very simple and symmetric.  The otherSelection is a bit more difficult to read because it tracks a pos.  This is a uncommon way to do this sort (not necessarily bad).  Then it does the swap post inner loop.  This was personally more difficult for me to read.  The first wins here.

Time Efficiency:

Both run \$O(n^2)\$.  However, otherSelection does only swap once per n-iteration.  This actually is more efficient to do.

Space Efficiency:

selection only needs 4 int variables where as otherSelection needs 5.  It's a bit more space efficient to use selection.  As user @200_success mentioned, the space efficiency is the same at worse case since both are constant space \$O(1)\$.  This isn't counting the input space (which you shouldn't really anyways).

Final verdict:

selection is a better option than otherSelection as far as readability.  However, otherSelection is better for efficiency.  This is a better variant of otherSelection which is just as good when it comes to space efficiency:

public static void selectionSort2(int[] x) {
    for (int i=0; i<x.length-1; i++) {
        int minIndex = i;      // Index of smallest remaining value.
        for (int j=i+1; j<x.length; j++) {
            if (x[minIndex] > x[j]) {
                minIndex = j;  // Remember index of new minimum
            }
        }
        if (minIndex != i) {
            //...  Exchange current element with smallest remaining.
            int temp = x[i];
            x[i] = x[minIndex];
            x[minIndex] = temp;
        }
    }
}

If you care for efficiency, this is the better selection sort option.  You could also do a bit-swap for when swapping the variables.

Final notes:  Since this is Java, you don't need to do the return statement at the end.


Count of Maximum - CodeChef I'm doing an easy problem in codechef. Here's the problem statement for maxcount:

  Given an array A of length N, your task is to find the element which
  repeats in A maximum number of times as well as the corresponding
  count. In case of ties, choose the smaller element first.

OK, I cheated. I used std:map because I want to play with it. Please review my code:

#include<iostream>
#include<map>

int main() {
    int test_cases{};
    std::cin >> test_cases;
    for (auto i = 0; i < test_cases; ++i) {
        std::map<int,int> count;
        std::size_t size{};
        std::cin >> size;
        int x{};
        for (std::size_t i = 0; i < size; ++i) {
            std::cin >> x;
            count[x] = ++count[x];
        }
        int number{};
        int numberCount{};
        for (auto& i : count) {
            if (i.second > numberCount) {
                number = i.first;
                numberCount = i.second;
            } else if (i.second == numberCount) {
                if (i.first < number) {
                    number = i.first;
                }
            } else {
                continue;
            }
        }
        std::cout << number << " " << numberCount << '\n';
    }
}

How can I make this better and faster?

Simplify:

        count[x] = ++count[x];

        // Why not just;
        ++count[x];

Variable name length:

for (auto i = 0; i < test_cases; ++i) {

Have you ever tried searching for all occurrences of the variable i in the resulting loop. The number of false positives will be a pain in the arse. Name your loop variables so you can find them easily.

Hiding variables names:

    for (std::size_t i = 0; i < size; ++i) {

Though not technically illegal. This becomes a maintenance problem. It's OK for you today as you just wrote the code. But for anybody else (or you in years time) this is can be a pain. Try and give your variables unique meaningful names (self documenting code is a brilliant practice but it requires variable names to be meaningful).

The whole loop where you search for the largest repeat:

    for (std::size_t i = 0; i < size; ++i) {

This can be done inline while you were counting. I would not have a second loop to work it out after the fact.

I would simplify this condition:

} else if (i.second == numberCount) {
            if (i.first < number) {

// I find it easier to read as:

} else if (i.second == numberCount) && (i.first < number) {

This seems a bit redundant:

        } else {
            continue;
        }

Personally I think initializing integers with {} looks terrible (and is slightly confusing).

std::size_t size{};

std::size_t size = 0;  // Much easier to read.

Whats the point in initizliaing a variable just before you write over it?

    std::size_t size{};   // Why initialize it here
    std::cin >> size;     // Only to trash the initialization here.

Address Comments:

    int number{};
    int numberCount{};
    for (std::size_t i = 0; i < size; ++i) {

        std::cin >> x;
        std::size_t&  countV = count[x];

        ++countV;

        if (countV > numberCount || (countV == numberCount && x < number))
        {
            number      = x;
            numberCount = countV;
        }
    }


My first jQuery plugin - blurStuff() I've played with jQuery for some time now but have never written my own plugin.

A question was asked: "can I blur an image using jQuery?" and I thought this to be a decent candidate to play with.

Here's my code so far:

(function ($) {
    $.fn.blurStuff = function (options) {
        var defaults = {blurRadius:2, deblurOnHover:false};
        var settings = $.extend(defaults, options);

        $(this).wrap('<div data-blurimage />');

        var blurContainers = $(this).closest('[data-blurimage]');

        blurContainers.each(function () {
            var img = $(this).children();

            $(this).css({
                'width': img.width(),
                'height': img.height(),
                'overflow': 'hidden',
                'position': 'relative'
            });

            var clone = img.clone();
            clone.css({
                'opacity': 0.2,
                'position': 'absolute'
            });

            $(this).append(clone.clone().css({'left': +settings.blurRadius, 'top': +settings.blurRadius}));
            $(this).append(clone.clone().css({'left': -settings.blurRadius, 'top': +settings.blurRadius}));
            $(this).append(clone.clone().css({'left': +settings.blurRadius, 'top': -settings.blurRadius}));
            $(this).append(clone.clone().css({'left': -settings.blurRadius, 'top': -settings.blurRadius}));
        });

        if (settings.deblurOnHover == true) {
            blurContainers.hover(function () {
                $(this).children('img:gt(0)').toggle();
            });
        }

        return blurContainers;
    };
})(jQuery);

In action: http://jsfiddle.net/gvee/xvvWj/

Example usage:

$('img').blurStuff({deblurOnHover: true, blurRadius: 2});

My working logic is to wrap the selector in a parent container and then append 4 translucent clones to this, where each clone is positioned slightly off centre.

This works pretty well so far and I'm pleased with my progress but I can conceive a couple of potential bugs that I wanted some opinions on!

Is my approach reasonable? I realise that I'm appending an extra 4 elements to the DOM on each call which is not ideal (I could get away with using just two, laterally or diagonally, but I think 4 produces a better effect)...
What to do if a user passes a fixed position element? This will break existing flow.
Should I bother validating/sanity checking the parameter values? If so, how should I approach this? Previously I had a parameter called blurOpacity but I removed this because I realised that the wrong values (e.g. 1) effectively "breaks" things.

In the code I only would modify the variable declarations:

var defaults = {blurRadius:2, deblurOnHover:false},
settings = $.extend(defaults, options),
img, blurContainers, clone;

Personal preference, but you can read a discusion about it here.

Answering your questions:

The efect work great. I think the approach is good, don't think
that adding four elements would be a big cost.
You could check if the  has the fixed property to avoid breaking anything.
By the parameters you have rigth now I shouldn't bother. If you decide
to add the opacity again it would be a good idea to check it. If
greater than 0.x use the default instead.


Pandas updating/adding columns to rows incrementally using dictionary key values I want to populate columns of a dataframe (df) by iteratively looping over a list (A_list) generating a dictionary where the keys are the names of the desired columns of df (in the example below the new columns are 'C', 'D', and 'E')

import pandas
def gen_data(key):
    #THIS FUNCTION IS JUST AN EXAMPLE THE COLUMNS ARE NOT NECESSARILY RELATED OR USE THE KEY
    data_dict = {'C':key+key, 'D':key, 'E':key+key+key}
    return data_dict

A_list = ['a', 'b', 'c', 'd', 'f']
df = pandas.DataFrame({'A': ['a', 'b', 'c', 'd', 'f'], 'B': [1,2,3,3,2]})

for A_value in A_list:
    data_dict = gen_data(A_value)
    for data_key in data_dict:
        df.loc[df.A == A_value, data_key] = data_dict[key]

So the result of this should be:

df = pandas.DataFrame({'A': ['a', 'b', 'c', 'd', 'e','f'],
                       'B': [1,2,3,3,2,1],
                       'C': ['aa','bb','cc','dd',nan,'ff'],
                       'D': ['a', 'b', 'c', 'd', nan,'f'],
                       'E': ['aaa','bbb','ccc','ddd',nan,'fff']})

I feel that

for data_key in data_dict:
    df.loc[df.A == A_value, data_key] = data_dict[key]

is really inefficient if there are a lot of rows in df and I feel that there should be a way to remove the for loop in this code.

for A_value in A_list:
    data_dict = gen_data(A_value)
    for data_key in data_dict:
        df.loc[df.A == key, data_key] = data_dict[key]

Since there is an e missing in the input dataframe in col A provided by you, I have added it:

#input
A_list = ['a', 'b', 'c', 'd', 'f']
df = pd.DataFrame({'A': ['a', 'b', 'c', 'd','e','f'], 'B': [1,2,3,3,2,1]})

You can start by joining the list you have:

pat='({})'.format('|'.join(A_list))
#pat --> '(a|b|c|d|f)'

Then using series.str.extract() I am extracting the matching keys from the series based on the pattern we created.

s=df.A.str.extract(pat,expand=False) #expand=False returns a series for further assignment
print(s)

0      a
1      b
2      c
3      d
4    NaN
5      f

Once you have this series, you can decide what you want to do with it. For,example if I take your function:

def gen_data(key):
    #THIS FUNCTION IS JUST AN EXAMPLE THE COLUMNS ARE NOT NECESSARILY RELATED OR USE THE KEY
    data_dict = {'C':key*2, 'D':key, 'E':key*3}
    return data_dict

And do the below:

df.join(pd.DataFrame(s.apply(gen_data).values.tolist()))

We get the desired output:

   A  B    C    D    E
0  a  1   aa    a  aaa
1  b  2   bb    b  bbb
2  c  3   cc    c  ccc
3  d  3   dd    d  ddd
4  e  2  NaN  NaN  NaN
5  f  1   ff    f  fff

However I personally wouldn't use apply unless mandatory, so here is another way using df.assign() where you can pass a dictionary of the extracted series and assign it to the dataframe:

df=df.assign(**{'C':s*2,'D':s,'E':s*3})

   A  B    C    D    E
0  a  1   aa    a  aaa
1  b  2   bb    b  bbb
2  c  3   cc    c  ccc
3  d  3   dd    d  ddd
4  e  2  NaN  NaN  NaN
5  f  1   ff    f  fff


How to simplify code, which adds spaces and commas if field value is not empty? var searchDetails = "";
searchDetails += $("#last-name").val();
searchDetails += searchDetails != "" ? " "+$("#first-name").val() : $("#first-name").val();
searchDetails += searchDetails != "" ? " "+$("#middle-name").val() : $("#middle-name").val();
if ($("#dob-dd").val() != "" || $("#dob-mm").val() != "" || $("#dob-yyyy").val() != "") {
    searchDetails += searchDetails != "" ? ", " : "";
    searchDetails += "date of birth: ";
    searchDetails += ($("#dob-dd").val() != "" ? $("#dob-dd").val() : "xx") + ".";
    searchDetails += ($("#dob-mm").val() != "" ? $("#dob-mm").val() : "xx") + ".";
    searchDetails += ($("#dob-yyyy").val() != "" ? $("#dob-yyyy").val() : "xx") + ".";
}
if ($("#passport-id").val() != "") {
    searchDetails += searchDetails != "" ? ", " : "";
    searchDetails += "passport " + $("#passport-id").val();
}

searchDetails should contain values like (depending on what user has input):

lastName firstName middleName, date of birth dd.mm.yyyy, passport 123456789
firstName middleName, date of birth dd.xx.yyyy
lastName firstName, date of birth dd.mm.xxxx
date of birth dd.mm.xxxx, passport 123456789
lastName firstName middleName
passport 123456789

Firstly, I'd create a helper function for the != "" check:

function isEmptyString(value) {
    if (value == "") {
        return true;
    }
    return false;
}

It would make the code easier to read.

Then I'd extract out a function for sturctures like ($("#dob-dd").val() != "" ? $("#dob-dd").val() : "xx") + ".":

function wrapDateOfBirth(id) {
    return ($(id).val() != "" ? $(id).val() : "xx") + ".";
}

and structures like searchDetails != "" ? " "+$("#first-name").val() : $("#first-name").val();:

function wrapSearchDetails($value, $id) {
    return $value != "" ? " " + $($id).val() : $($id).val();
}

They would remove some duplication.

Furthermore, the wrapSearchDetails function could be written like this too:

function wrapSearchDetails(value, id) {
    return (isEmptyString(value) ? "" : " ") + $(id).val();
}

The wrapDateOfBirth function also could be simplified:

function wrapDateOfBirth(id) {
    var value = $(id).val();
    return (isEmptyString(value) ? "xx" : value) + ".";
}

Finally, a getDefaultIfEmpty function also could improve the readability of the wrapDateOfBirth function:

function getDefaultIfEmpty(value, defaultValue) {
    if (isEmptyString(value)) {
        return defaultValue;
    }
    return value;
}

function wrapDateOfBirth(id) {
    var value = $(id).val();
    return getDefaultIfEmpty(value, "xx") + ".";
}

(I have not tested the code, please feel free to edit if you find any typo/etc.)


Powershell zip subfolders recursively, conditionally This is a progression of the script I posted here: Zip the contents of subfolders, conditionally

It does this:

Determines all subfolders recursively
Checks each subfolder for files older than 31 days which aren't .zip
If such files found, creates a new folder within, labelled with the date
Moves the files to the new folder
Creates a zipped copy of the new folder
Deletes the new folder and its contents

There's error-handling and logging built-in

#Run under Powershell v1 or v2

#Pass the target log folder as a parameter: '-LogFolder Path\to\logfolder'
param(
    [parameter(Mandatory=$true)]
    [ValidateScript({
        if (Test-Path $_ -PathType Container) {$true}
        else {Throw "Folder $_ not found"}})]
    [String]
    $LogFolder
)

#Get working directory, for outputting log file
$WorkingDirectory = (pwd).path

#Print datestamp to the log file
Get-date >> "$WorkingDirectory\OutputLog.txt"
"Log directory: $LogFolder" >> "$WorkingDirectory\OutputLog.txt"

#Record total size of the target log folder before compression
$Logsize = Get-ChildItem -recurse $LogFolder | Measure-Object -property length -sum
$MBsize = "{0:N2}" -f ($Logsize.sum / 1MB) + " MB"
"Initial Log folder size: $MBsize" >> "$WorkingDirectory\OutputLog.txt"

#Verify that the Zip.dll is present
$testzip = Test-Path .\ICSharpCode.SharpZipLib.dll

#If the Zip.dll is found, bind it to a variable and load it, otherwise exit
#In other words, if the script cannot load the Zip module, it will do nothing
if ($testzip -eq $True){
$ZipModule = Get-ChildItem .\ICSharpCode.SharpZipLib.dll | Select -ExpandProperty FullName
[void][System.Reflection.Assembly]::LoadFrom($ZipModule)
}
Else{
"Zip dll not found or couldn't be loaded.
Check file is present and unblocked.
Exiting" >> "$WorkingDirectory\OutputLog.txt"
Exit
}

#Get all subfolders
$subfolders = Get-ChildItem $LogFolder -Recurse |
Where-Object { $_.PSIsContainer -and $_.fullname -notmatch "\\jsonTemplates\\verifier\\?" }

ForEach ($s in $subfolders) {

    $path = $s  #$s variable contains each folder
    $path
    Set-Location $path.FullName

    $fullpath = $path.FullName

    #Get all items older than 31 days, exclude zip files and folders
    $items = Get-ChildItem -Exclude *.zip |
    Where-Object {$_.LastWriteTime -lt (Get-date).AddDays(-31)
    -and -not $_.psIsContainer}

    #Verify that there are such items in this directory, catch errors
    if ( $(Try { Test-Path $items }
    Catch { "Cannot find items in $fullpath.
    Sub-folders will be processed afterwards.
    ERROR: $_" >>  "$WorkingDirectory\OutputLog.txt"  }) ) {

        $date = Get-Date -Format 'yyyy-MM-dd_HH-mm'
        $newpath = "$path-$date"
        $newpath
        $newfld = New-Item -ItemType Directory -name $newpath

        $src = $newfld.FullName

        #move items to newly-created folder
        Move-Item $items -destination $src

        $dest = "$src.zip"
        "Compressing $src to $dest"  >>  "$WorkingDirectory\OutputLog.txt"

        #the following block zips the folder
        try{
            $zip = New-Object ICSharpCode.SharpZipLib.Zip.FastZip
            $zip.CreateZip($dest, $src, $true, ".*")
            Remove-Item $src -force -recurse
        }
        catch {
            "Folder could not be compressed. Removal of $src ABORTED.
            ERROR: $_" >> "$WorkingDirectory/OutputLog.txt"
        }
    }

}

#Record total size of the target log folder after compression
$Logsize = Get-ChildItem -recurse $LogFolder | Measure-Object -property length -sum
$MBsize = "{0:N2}" -f ($Logsize.sum / 1MB) + " MB"
"Final Log folder size: $MBsize" >> "$WorkingDirectory\OutputLog.txt"

Set-Location $LogFolder

[ValidateScript({
    if (Test-Path $_ -PathType Container) {$true}
    else {Throw "Folder $_ not found"}})]

Test-Path already returns a [bool] which is what is expected by [ValidateScript()] so this can be simplified by removing the if/else:

[ValidateScript(
    (Test-Path $_ -PathType Container) -or $(throw "Folder $_ not found")
)]

pwd is an alias for Get-Location. I always recommend avoiding aliases and abbreviations in reusable scripts.

if ($testzip -eq $True)

This can be simplified to:

if ($testzip)

Else{
"Zip dll not found or couldn't be loaded.
Check file is present and unblocked.
Exiting" >> "$WorkingDirectory\OutputLog.txt"
Exit
}

This should probably be thrown as an exception:

else {
    $e = "Zip dll not found or couldn't be loaded.
    Check file is present and unblocked.
    Exiting"
    $e >> "$WorkingDirectory\OutputLog.txt"
    throw [System.IO.FileNotFoundException]$e
}

$path = $s  #$s variable contains each folder
$path
Set-Location $path.FullName

There are a two things I want to address here.

First, the middle line:

$path

This is implicitly calling Write-Object $path which in your script is probably writing this out to the console, and that's probably the desired behavior. You should be aware that this actually makes it the return value of the current scope/scriptblock you're in, which means it's part of the return value of your script.

You might not care about that right now, and it might never matter because you'll never call this script from something that checks its output, but it's bad practice (unless you very explicitly do want that to be part or all of its return value).

If all you want is to show the path on the screen, use Write-Host. Or even better, add [CmdletBinding()] above your param() block and then use Write-Verbose. That will only show the output when someone calls the script with -Verbose.

Second, I believe you should avoid changing the current path when possible. If it's not possible or it's impractical, then at least make use of Push-Location and Pop-Location along with try/finally so that you restore the original working directory.

if ( $(Try { Test-Path $items }
    Catch { "Cannot find items in $fullpath.
    Sub-folders will be processed afterwards.
    ERROR: $_" >>  "$WorkingDirectory\OutputLog.txt"  }) ) {

This is confusing. Test-Path should not throw an exception; it should return $false when the path doesn't exist, so your catch should never run. In addition, if it did run, the entire expression might still return $true making the if always satisfied.

Seems this would be better written as:

if ( Test-Path $items ) {
    # ... do stuff
} else {
    "Cannot find items in $fullpath.
    Sub-folders will be processed afterwards." >>  "$WorkingDirectory\OutputLog.txt"
}

You should wrap your logging into a function instead of having "Some message"  >>  "$WorkingDirectory\OutputLog.txt" strewn about the code. If you ever have to When you have to update that it will be a pain.

Consider something like this:

function Write-Log {
[CmdletBinding()]
param(
    [Parameter(
        Mandatory=$true
    )]
    [ValidateNotNullOrEmpty()]
    [String]
    $Message ,

    [Parameter(
        Mandatory=$true
    )]
    [ValidateNotNullOrEmpty()]
    [ValidateScript( { $_ | Test-Path -PathType Leaf -IsValid } )]
    [String]
    $LogPath
)
    $Message  >>  $LogPath
}

Now you can do:

Write-Log -Message "Compressing $src to $dest" -LogPath "$WorkingDirectory\OutputLog.txt"

And, you can do this at the top of the script (with v3 or higher):

$PSDefaultParameterValues = @{
    "Write-Log:LogPath" = "$WorkingDirectory\OutputLog.txt"
}

And then just do:

Write-Log -Message "Compressing $src to $dest"

Of course you could also make -LogPath optional, and give it a default value if you must (for v2 and below compatibility).

Even if you have to supply the log file every time though, so what, you're already doing that. And wrapping this in a function gives you more options for changing how its written later (like automatically adding a timestamp to each log entry).


Check balanced brackets Yes, I know we have a lot of these.

I'm new to Clojure, but not to lisps. After the recent (javascript?) parentheses-balancing Q, I decided to do an implementation in Clojure for practice with the language. (It also happened to make a nice accompaniment to my rant about students learning what is and isn't possible with regular expressions.)

I chose to use (reduce) over recursion mostly because I am a fan of letting functions do the work of creating loops and recursion for me, but in this case I am not sure how "elegant" I consider the :false handling.

Code

(ns parens)

(def str->chars (partial map identity))

(defn mk-balanced?
  "makes a balanced? checker from table, which maps closing characters to
  opening characters.

  see also: balanced?"
  [table]
  (fn [s]
    (let [opens (set (vals table))
          closes (set (keys table))]
      (empty?
        (reduce
          (fn [stack cur]
            (if (not= (peek stack) :false)
              (condp contains? cur
                opens (conj stack cur)
                closes (if (and (seq stack)
                                (= (peek stack) (table cur)))
                         (pop stack)
                         [:false])
                stack)
              [:false]))
          []
          (str->chars s))))))

(def balanced?
  (mk-balanced? {\) \(
                 \] \[
                 \} \{}))

I'd never thought of using reduce for this. Neat! However, you can simplify mk-balanced? a little.

str->chars is redundant: reduce and the other sequence functions accept strings as such.
You can use reduced to short circuit a reduce.
The reduced can return any non-empty sequence: no need for [:false].

The simplified version is ...

(defn mk-balanced? [table]
  (fn [s]
    (let [opens (set (vals table))
          closes (set (keys table))]
      (empty?
        (reduce
          (fn [stack cur]
            (condp contains? cur
              opens (conj stack cur)
              closes (if (and (seq stack)
                              (= (peek stack) (table cur)))
                       (pop stack)
                       (reduced [nil]))
              stack))
          []
          s)))))


Running async processes in node.js on multiple cores Using the async and child_process modules, I can create true "parallel" processing. My question is, how can I ensure that it is running on multiple cores?

var cp = require('child_process');
var async = require('async');

async.parallel([

    function(callback){
        console.log('1');
        var k = cp.fork('./doOther.js',['01']); //how can I ensure the child process k runs on a different core than this process?
        k.on('message',function(msg){
            console.log(msg);
            callback();
        });
    },
    function(callback){
        console.log('2');
        var k = cp.fork('./doOther.js',['02']); //how can I ensure the child process k runs on a different core than this process?
        k.on('message',function(msg){
            console.log(msg);
            callback();
        });
    },
    function(callback){
        console.log('3');
        var k = cp.fork('./doOther.js',['03']); //how can I ensure the child process k runs on a different core than this process?
        k.on('message',function(msg){
            console.log(msg);
            callback();
        });

    },
    function(callback){
        console.log('4');
        var k = cp.fork('./doOther.js',['04']); //how can I ensure the child process k runs on a different core than this process?
        k.on('message',function(msg){
            console.log(msg);
            callback();
        });

    }

],function(){

    console.log('done.');
    process.exit(0);

});

The above script interacts with IPC (inter-process comm) with this script:

//doOther.js, a simple script that interacts with the script above

    process.on('message', function(m) {
        console.log('CHILD got message:', m);
    });

    setTimeout(function(){
        process.send({ foo: process.argv[2] });
    },1000);

Notice a pattern in the array of your async.parallel call? All the functions are the same except for a single number.

And, judging by the repeated comment for each function, you just copied the first version of the function and changed that one number.

You should create a loop that pushes functions into the array, changing that single number each time.

for(var i = 0; i < 4; i++) {
    array.push(function(callback) {
        console.log(i + 1);
        var k = cp.fork('./doOther.js',['0' + (i + 1)]);
        k.on('message',function(msg){
            console.log(msg);
            callback();
        });
    }
}

As another tip, if you aren't going to use k again, don't store cp.form(...,...); in a variable; just continue the .on stuff that you were doing to the k but to the cp.form(...,...).

That would look like this:

cp.fork('./doOther.js', ['0' + (i + 1)]).on('message', function(msg) {
    console.log(msg)
    callback();
});

Other than that, I'm not too familiar with node.js and what a core is, so I can't help you on that part.


jQuery handler for closing a login box I'm sure there's a better more 'jQuery' like way of writing this.  In particular the selector $(this).closest('div.login-box') being in there twice. Perhaps it's possible to use $(this)?

$('.close-box').click(function () {
        $(this).closest('div.login-box').slideToggle("400");
        if ($(this).closest('div.login-box').attr('id') == "login-forgot") {
            clearForgottenPasswordInputs();
        }
    });

You could combine the calls:

if ($(this).closest('div.login-box')
           .slideToggle("400")
           .attr('id'))

However, I do not recommend this.
Although it will work, it's unnecessarily confusing.


Three function Calculator 2.0 A week ago I did a three function calculator, I took the input from you guys and made it better. Overall it works fine, I didn't find any problems. But as a beginning programer I'd like some input on this calculator.

// Three function Calculator.cpp : main project file.
#include "stdafx.h"
#include<iostream>
using std::cout;
using std::cin;

long double action(long double num1, long double num2, char act) //External function to do all the calculations
{
    long double total;
    switch (act)
    {
    case '+':
        total = num1 + num2;
        break;
    case '-':
        total = num1 - num2;
        break;
    case '*':
        total = num1*num2;
        break;
    case '/':
        total = num1 / num2;
        break;
    default:
        cout << "Input not recognized\n";
    }
    return total;    //Giving the main function a number to work with.
}

int main()
{
    cout << "Enter action as # to exit program\n";
    cout << "Possible actions:+,-,*,/\n\n";
    long double num1, num2, num3, total;
    char act1, act2;
    while (1)   //inf loop
    {
        beginning:    //label, to reset the program in case of division by zero
        cin >> num1 >> act1 >> num2 >> act2 >> num3;
        if (act1 == '#' || act2 == '#')         //user wants to exit program
        {
            system("pause");
            return 0;
        }
        else if (act1 == '/' && num2 == 0 || act2 == '/' && num3 == 0)      //division by zero happens
        {
            cout << "You can't divide by zero\n";
            cout << "\n\n";
            goto beginning;              //restarting the program from the lable, to the beginning of the 'while' loop
        }
        else if ((act2 == '*' || act2 == '/') && (act1 == '-' || act1 == '+'))     //action2 happens before action1 because of order of operation
        {
            total = action(num2, num3, act2);             //sending the function reciving the total
            total = action(num1, total, act1);            //sending the total of previos function to get overall total
        }
        else      //action1 happens before action2 because of order of operation
        {
            total = action(num1, num2, act1);      //sending the function reciving the total
            total = action(total, num3, act2);     //sending the total of previos function to get overall total
        }
        cout << num1 << act1 << num2 << act2 << num3 << "=" << total;   //printing result of overall function
        cout << "\n\n";    //new line before restarting program
    }
}

Old Calculator

Example:

  Input:-6*2+4
  Output:-6*2+4=-8

A couple quick things to make maintenance easier:

It's easier to work out where you're going if you use continue instead of the goto. If you do keep the goto, I'd at least edit the comment to refer to the beginning of the while loop instead of a specific line number. That will change if you have to change something, and you don't want to have to remember to update the comment.

In terms of readability, when checking for order of operations, you could do something like

if (precedence(act2) > precedence(act1))

Then it would be easier if you wanted to add an exponent operator.


Blackjack game: Count Hand Value method I am almost completing my BlackJack game in Javascript. But I realize that there must be a way to improve the performance for these two methods. Any pointers?

const hasAceInHand = (cardsOnHand) => {
    for (let key in cardsOnHand) {
        let arr = cardsOnHand[key];
        for (let i = 0; i < arr.length; i++) {
            let obj = arr[i];
            for (let prop in obj) {
                if (prop === "face") {
                    if (obj[prop] === "A") {
                        return true;
                    }
                }
            }
        }
    }
    return false;
}

const countHandValue = (cardsOnHand) => {
    //console.log(hasAceInHand(cardsOnHand));
    let sum = 0;
    for (let key in cardsOnHand) {
        let arr = cardsOnHand[key];
        for (let i = 0; i < arr.length; i++) {
            let obj = arr[i];
            for (let prop in obj) {
                if (prop === "faceValue") {
                    //console.log(prop + " = " + obj[prop]);
                    sum = sum + obj[prop];
                    debugger;
                    if (sum > 21 && hasAceInHand(cardsOnHand)) {
                        // Transfer Ace's face value from 11 to 1
                        sum = sum - 11;
                        sum = sum + 1;
                    }
                }
            }
        }
    }

    return sum;
}

This countHandValue method is called at below:

const cardSuit = ["hearts", "diams", "clubs", "spades"];
const cardFace = ["2", "3", "4", "5", "6", "7", "8", "9", "10", "J", "Q", "K", "A"];

// In JavaScript, functions are objects.
// You can work with functions as if they were objects.
function card(suit, face) {
    this.suit = suit;
    this.face = face;

    switch (face) {
        case "A":
            this.faceValue = 11;
            break;
        case "J":
        case "Q":
        case "K":
            this.faceValue = 10;
            break;
        default:
            this.faceValue = parseInt(face);
            break;
    }
};

const deck = {
    cards: []
}

var tempCard;
const player = {
    cards: [],
    handValue: 0
}

const dealer = {
    cards: [],
    handValue: 0
}

const dealOneCardToPlayer = () => {
    // Take a card from the top deck to be assigned to tempcard.
    tempCard = deck.cards.splice(0, 1);
    player.cards.push(tempCard);
    player.handValue = countHandValue(player.cards);
    document.getElementById("handValuePlayer").innerHTML = player.handValue;
    makeCardPlayer(tempCard[0]);
}

I have uploaded the whole program.

Review

Function hasAceInHand could be boiled down to a one-liner using array.find. It returns the first element of the array that passes the condition x => x.face === "A" provided.

const hasAceInHand = (cardsOnHand) => {
  return cardsOnHand.find(x => x.face === "A") != null;
}

Function countHandValue iterates the items, and calls the other function which iterates the items nested in the outer iteration if (sum > 21 && hasAceInHand(cardsOnHand)) { ... You could just keep track of aces to avoid iterating the items more than once.

Alternative for countHandValue

In black-jack, at most 1 ace could have value 11, all other aces in the hand must have value 1, else 21 would get exceeded (11 + 11 > 21). I would change the default value of an ace to 1.

switch (face) {
    case "A":
        this.faceValue = 1;
        break;

As a first step, Array.reduce could be used to calculate the sum of card values.

const countHandValue = (cardsOnHand) => {
  return cardsOnHand.reduce((sum, current) => {
    sum += current.faceValue;
    return sum;
  }, 0);
}

Then we can implement a relaxation allowing at most 1 ace to get value 11.

const countHandValue = (cardsOnHand) => {
  let ace;
  let value = cardsOnHand.reduce((sum, current) => {
    ace |= current.faceValue === 1;
    sum += current.faceValue;
    return sum;
  }, 0);
  if (ace && value + 10 <= 21) {
    value += 10;
  }
  return value;
}

And we also no longer need to call hasAceInHand as we're tracking the ace inline.


Remove duplicate elements from array along with element using Java 1.7 I need to write a method where a int[] will be supplied as an input and it should return an array, but with all of the numbers that occur more than n times removed entirely.

Input:

(int list) data = [1, 2, 2, 3, 3, 3, 4, 5, 5]

Output:

(int list) [1, 4]

These are the steps i have tried:

Copy the int array to ArrayList (inputList).
Create a LinkedHashset to find unique values
Iterate LH and find the collection frequency of ArrayList with iterator.

    int[] intArray = new int[0];
ArrayList<Integer> inputList = new ArrayList<Integer>(data.length);
        //System.out.println(Arrays.toString(data));
        for (int i = 0; i < data.length; i++){
            inputList.add(Integer.valueOf(data[i]));
        }

        LinkedHashSet<Integer> lhs = new LinkedHashSet<>(inputList);

        intArray = new int[lhs.size()];
        int i=0;
        int j=0;
        Iterator<Integer> itr = lhs.iterator();
        while(itr.hasNext()){
            Integer shiftNumber = itr.next();

            if(Collections.frequency(inputList, shiftNumber)==1) {
                intArray[i++] = shiftNumber.intValue();
                j++;
            }

        }
        intArray = Arrays.copyOf(intArray, j);
        return intArray;
    }
}

return intArray;

I am able to achieve the results with the above snippet.However, I need suggestions in reducing the piece of code and improving performance by using any algorithms or other collection objects.

Use the right tools

You are on the right track to use Collections.frequency, as it does a lot of the plumbing for you

Start simple

The basic idea is to create a new list, unique that will contain the numbers that are unique.

This is the piece of code that will do all the hard work (but is not optimized yet!).

ArrayList<Integer> uniqueNumbers = new ArrayList<>();

for (Integer number : numbers) {
    if (Collections.frequency(numbers, number) == 1) {
        uniqueNumbers.add(number);
    }
}

Now you only need to convert your input array to an numbers ArrayList and convert the resulting uniqueNumbers list back to an array.

Then optimize

If you care about performance, you will probably want to skip number that you have already checked and know that they are non unique. You can do this with a Set<Integer>.

Set<Integer>  nonUniqueNumbers   = new HashSet<>();
ArrayList<Integer> uniqueNumbers = new ArrayList<>();

for (Integer number : numbers) {

    if (!nonUniqueNumbers.contains(number)) {
        if (Collections.frequency(numbers, number) == 1) {
            uniqueNumbers.add(number);
        }
        else {
            nonUniqueNumbers.add(number)
        }
    }
}

More room for performance

To increase the performance, you only want to scan each input element once; this can for example be done like this:

public static Collection<Integer> getUniques( Collection<Integer> input )
{
    Set<Integer> uniques = new HashSet<>();
    Set<Integer> nonUniques = new HashSet<>();
    for ( Integer i : input )
    {
        //if i is in the nonUnique set, we already have it twice,
        //no need to do anything with it anymore
        if ( nonUniques.contains( i ) )
        {
            continue;
        }
        else
        {
            //either we never saw i, or we have seen it once.
            //if we saw it once, it will be in the uniques.
            if ( uniques.contains( i ) )
            {
                //if it was in the uniques , this is the second time we encountered it.
                //remove it from the uninques and put it in the nonUniques
                uniques.remove( i );
                nonUniques.add( i );
            }
            else
            {
                //if we never saw i, it should be in the uniques set (for now)
                uniques.add( i );
            }
        }
    }
    return uniques;
}


12/24h Time format converter I am really new at this and I was wondering if there is a more efficient way of writing this code, which converts time format to 12-hour from 24-hour format, and vice-versa.

For example 07:16:35PM converts to 19:16:35, and 13:05:25 to 1:05:25PM.

public static void convertTimeTo24(String time)
{

    if(time.substring(8,10).equals("PM") && Integer.parseInt(time.substring(0,2)) < 12)
        {
            String hour12 = time.substring(0,2);
            int hour24 = (Integer.parseInt(hour12)) + 12;

            if(hour24<24)
                {
                System.out.print(hour24+time.substring(2,8));
                }

            if(Hour24>23)
                {
                System.out.print(00+time.substring(2,8));
                }
        }

    else if(time.substring(8,10).equals("AM") && Integer.parseInt(time.substring(0,2)) == 12)
        {
        System.out.print("00"+time.substring(2, time.length()));
        }

    else
        {
       System.out.print(time.substring(0,8));
        }
}

public static void convertTimeTo12(String time)
{
    int hour24 ;
    if(Integer.parseInt(time.substring(0, 2)) > 12 ) //check if the hr is greater than 12
        {
            hour24 = Integer.parseInt(time.substring(0, 2)) - 12; //substract 12 from the hr
            if(hour24 < 10)
                {
                System.out.println("0"+hour24 + time.substring(2, time.length())+"PM"); //print hour the hour in 24hr format
                }

            else
                {
                System.out.println(+hour24 + time.substring(2, time.length())+"PM"); //print hour the hour in 24hr format
                }
        }

    if(Integer.parseInt(time.substring(0, 2)) == 0 )
        {
        hour24 = Integer.parseInt(time.substring(0, 2)) + 12; //print hour the hour in 24hr format
        System.out.println(hour24 + time.substring(2, time.length())+"AM"); //print hour the hour in 24hr format
        }

}

public static void main(String[] args) {
           Scanner in = new Scanner(System.in);
           String time = in.nextLine();      //sets time to string input
           if(time.length()<10)      //check if input is in 24hours format or 12
             convertTimeTo12(time); //call the convert function above if its 12 hrs format
           else
           convertTimeTo24(time); // calls function to convert 12hr to 24hr format

}

}

General comments:

try to indent your code properly - it is hard to read at the moment
follow the language conventions: variable should start in lower case (Hour24 => hour24)
similarly, opening braces are at the end of the line, not at the beginning of a new line (like in C-like languages)
try to do only one thing in one method - at the moment your methods do the conversion and print at the same time

Regarding your question about efficiency - a more efficient way (less time to code and fewer lines of code) would be to use existing libraries.

With Java 8, you could use the java.time API:

private static final DateTimeFormatter FMT_24 = DateTimeFormatter.ofPattern("HH:mm:ss");
private static final DateTimeFormatter FMT_12 = DateTimeFormatter.ofPattern("h:mm:ssa");

public static String convertTimeTo24(String time12) {
  LocalTime time = LocalTime.parse(time12, FMT_12);
  return FMT_24.format(time);
}

public static String convertTimeTo12(String time24) {
  LocalTime time = LocalTime.parse(time24, FMT_24);
  return FMT_12.format(time);
}

And to avoid duplicating code, you could refactor the two methods into:

public static String convertTimeTo24(String time12) {
  return convertFormats(time12, FMT_12, FMT_24);
}

public static String convertTimeTo12(String time24) {
  return convertFormats(time24, FMT_24, FMT_12);
}

private static String convertFormats(String fromTime, DateTimeFormatter fromFormat,
                                                      DateTimeFormatter toFormat) {
  LocalTime time = LocalTime.parse(fromTime, fromFormat);
  return toFormat.format(time);
}

Your main method needs to be changed accordingly.


Project Euler problem 44: sums and differences of pentagonal numbers This is my code for solving the pentagonal number problem on Project Euler:

  Pentagonal numbers are generated by the formula, \$P_n=n\dfrac{3n−1}{2}\$.

  The
  first ten pentagonal numbers are:

1, 5, 12, 22, 35, 51, 70, 92, 117, 145, ...

  It can be seen that P4 + P7 = 22 + 70 = 92 = P8. However,
  their difference, 70 − 22 = 48, is not pentagonal.

  Find the pair of
  pentagonal numbers, \$P_j\$ and \$P_k\$, for which their sum and difference are
  pentagonal and \$D = |P_k − P_j|\$ is minimized; what is the value of \$D\$?

As a beginner, I use the most basic concepts so often that sacrifices efficiency. If I could get some general tips on improving this code that would be awesome.

Key things I'd like to improve on:

How to deal with double for loops when using elements from 2 lists:

for x in a:
    for y in b:

And if there are any functions built into Python that I could use to instead of the bit of code written.

lst = []
dic = []
test = []
Final = []

for x in range(1, 3010):
    thing = int((x * (3*x - 1))/2)
    test.append(thing)
test = set(test)

for x in range(1, 3001):
    num = int((x * (3*x - 1))/2)
    lst.append(num)
lst = set(lst)

list2 = lst

for x in lst:
    for y in list2:
        num = x + y
        num2 = abs(x - y)
        dic.append({num: num2})

dic = [dict(t) for t in set([tuple(d.items()) for d in dic])]

for x in dic:
    for y in x:
        if y in test:
            Final.append(x)

for x in Final:
    for y in x:
        if x[y] in test:
            print(x)

Some observations:

To do integer division in Python 3, you should use the // operator. For example, you should prefer to do (x*(3*x-1))//2 over int(x*(3*x-1)/2); the latter expression can run into annoying rounding issues for very large values of x.
It looks like the variables test, lst, and list2 all contain the same set of numbers (in fact, lst and list2 even point to the same object). There's no reason you need different variables for all of these.
In particular, it is completely fine to do for x in lst: followed by for y in lst:; no need to rename the second list for some reason.
I am not sure what the point of the set comprehension [dict(t) for t in set([tuple(d.items()) for d in dic])] is; it seems like it will just return to you the original list dic, possibly permuted.
You don't really need to store an auxiliary dictionary at all; just keep track of which valid pair of pentagonal numbers has the smallest difference. To do this, I'd recommend adding a best variable to keep track of the best valid pair so far, and moving the checks you do in your last two nested for loops into the loop where you iterate over lst.


Fractional Knapack as asked in an interview Description:

  Given the Knapsack capacity and the weight and value of some items,
  find a way to maximize the value in the given Knapsack.

Code:

/* Name of the class has to be "Main" only if the class is public. */
class Main
{
    public static float KnapSack(float capacity, PriorityQueue<Pair> queue) {
        float val = 0;
        while (queue.size() > 0) {
            if (capacity == 0) return val;
            Pair max = queue.poll();
            float a = Math.min(max.weight, capacity);
            val += a * (max.value / max.weight);
            if (max.weight - a > 0) // to prevent division by zero
                queue.add(new Pair(max.value, max.weight - a));
            capacity -= a;
        }
        return val;
    }

    public static void main (String[] args) throws java.lang.Exception
    {
        Comparator<Pair> comparator = new FractionComparator();
        PriorityQueue<Pair> queue   = new PriorityQueue<Pair>(3, comparator);
        queue.add(new Pair(20, 4));
        queue.add(new Pair(18, 3));
        queue.add(new Pair(14, 2));
        System.out.println(KnapSack(7, queue));
    }
}

// Degenerate value class
class Pair {
    final float value;
    final float weight;

    Pair(float value, float weight) {
        this.value  = value;
        this.weight = weight;
    }
}

class FractionComparator implements Comparator<Pair>
{
    @Override
    public int compare(Pair p1, Pair p2)
    {
        if ((p2.value / p2.weight) < (p1.value / p1.weight)) return -1;
        if ((p2.value / p2.weight) > (p1.value / p1.weight)) return  1;
        return 0;
    }
}

Question:

From interview perspective what may raise eyebrows of an interviewer?

Having said that personally I am interested in readability and reducing code redundancy, the one thing where I am feeling uncomfortable is Pair class although I made it for readability but for changing even a field name I have to d changes in many places.

Last but not the least I think some knowledge about scope and visibility would be good.

float val = 0;
        while (queue.size() > 0) {
            if (capacity == 0) return val;
            Pair max = queue.poll();
            float a = Math.min(max.weight, W);
            val += a * (max.value / max.weight);
            if (max.weight - a > 0) // to prevent division by zero
                queue.add(new Pair(max.value, max.weight - a));
            capacity -= a;
        }
        return val;

Consider

        float val = 0;

        while (!queue.isEmpty()) {
            Pair max = queue.poll();
            if (max.weight < capacity) {
                val += max.value;
                capacity -= max.weight;
            } else {
                return val + capacity * (max.value / max.weight);
            }
        }

        return val;

The canonical way to check if a collection is empty is to call the isEmpty() method.

We check that the capacity hasn't been reduced to zero before the end of the loop rather than at the beginning.  Note that a side effect of this is that we don't care about passing 0.0.  In the original code, floating point rounding could allow you to pass the equality check.

Rather than take Math.min, we compare max.weight and capacity.  This simplifies the logic, as we don't have to multiply and divide by the same value in the expected case.  We only do that for the final item.


Beginner's exercise to ask for first names and surnames I just started Java programming yesterday (so don't expect too much), and I've written some code. My code works the way I want it to work, but there are obviously things wrong with it.

I was wondering how it could be improved. I think I've added unnecessary things. I'm practicing using classes and other stuff.

testing.java

import java.util.Scanner;

class testing {
    private static Scanner input_sn;
    private static Scanner input_fn;
    private static Scanner input_mem;

    public static void main(String[] args){

        String First_Name;
        String Second_Name;
        int members;
        int count;

        System.out.println("Members: ");
        input_mem = new Scanner(System.in);
        members = input_mem.nextInt();

        funcs funcsObj = new funcs();

        for (count = 0; count < members; count++)
        {
            System.out.println("What is the first name? ");
            input_fn = new Scanner(System.in);
            First_Name = input_fn.nextLine();

            System.out.println("What is the second name? ");
            input_sn = new Scanner(System.in);
            Second_Name = input_sn.nextLine();

            funcsObj.names( funcsObj.setFn(First_Name), funcsObj.setSn(Second_Name));
        }
    }
}

funcs.java

public class funcs
{
    private String firstName;
    private String secondName;
    private static int members = 0;

    public String setFn(String fn)
    {
        firstName = fn;
        return fn;
    }

    public String setSn(String sn)
    {
        secondName = sn;
        return sn;
    }

    public void names(String fn, String sn)
    {
        firstName = fn;
        secondName = sn;
        members++;
        System.out.printf("%d\t%s\t%s\n", members, fn, sn);
    }
}

(I think most of the problems can be found in the funcs.java file.)

Styling:

You have to fix the case of your identifiers to be more idiomatic. It means:

Class names should be Capitalized ("Testing", "Funcs", ...)
Class members and variables should use camel case ("firstName", "inputSn", ...)

Also:

The method names of class funcs behaves like a "setter", so it should have a name like setNames. In general, try to use action verbs in your method names.
Edit: the setXX() methods of funcs should not return anything, as they are "setters", they are there to "set".

Code quality:

Variables like "String First_Name" should not all be declared at the beginning of the method, but instead next to the place where they are first used. This way you limit their scope (which makes programming errors more clear) and the code is easier to read.
Similarily, if you don't have a compelling reason to have all the Scanner instances as class members, declare them in your main method where they are used.
Why do you use different Scanner objects for every input? You can simply use one Scanner and use its nextLine() method 3 times. That's the only "performance" tips that you can get, I think (not that you would have any performance problem with such a short program anyway).
Edit: In the line funcsObj.names( funcsObj.setFn(First_Name), funcsObj.setSn(Second_Name)); you are actually setting the members of your funcs objects twice. First in the setFn and setSn methods and then in the names method. You don't need to. Just call your setXX methods. If you want to have the members thing incremented and the data displayed, you can rename names into incrementMembers and remove all setting code from there.


Passing help options to a bash script I used this code to give -h(help) options to my bash script. It's working fine and I wanted to get the code reviewed.

while true; do
        if [ "$1" != "-l" ] && [ "$1" != "-a" ] && [ "$1" != "-h" ] && [ "$1" != "" ];
        then
                output=$(curl -k -s -u "$username:$password" -H "X-Requested-With: Curl" -d "action=list&echo_request=1" "${!url}" | grep -B 2 -w $name | grep ID | head -1 | sed 's/[^0-9]*//g')
        fi
        case "$1" in
                "")
                        echo "use -h for usage information";;
                -h)
                        echo  "-a       add"
                        echo  "-d       delete"
                        echo  "-l       name all"
                        echo  "-sv      addvlan"
                        echo  "-mv      add multiple vlan"
                        echo  "-dv      del vlan"
                        echo  "-ar      add route"
                        echo  "-mr      add multiple route"
                        echo  "-dr      delete route";;
                -a)
                        add ;;
                -d)
                        delete ;;
                -l)
                        list ;;
                -sv)
                        addvlan ;;
                -mr)
                        multipleroute ;;
                -dv)
                        delvlan "$@" ;;
                -mv)
                        addmulvlan ;;
                -dv)
                        delvlan ;;
                -ar)
                        addroute ;;
                -dr)
                        delroute "$@" ;;
        esac
        break
done;

There is a break statement at the end of the while true.
Like this, the while loop is completely pointless, you can remove it.

The output variable is never used.
Either you forgot to include the rest of the script,
or this is a pointless statement and should be removed.

The line that sets the output variable to the result of $(curl ...) is extremely long, and hard to read. It would be better to add some line breaks to reduce horizontal scrolling to read the code.
The grep-grep-head-sed chain can be shorter too with a little Awk magic.

This long condition would be simpler to write with a case:

if [ "$1" != "-l" ] && [ "$1" != "-a" ] && [ "$1" != "-h" ] && [ "$1" != "" ]; then
    output=$(curl ...)
fi

Like this:

case "$1" in
    -l|-a|-h|"") ;;
    *)
        output=$(curl ...)
        ;;
esac


A Singleton Base and derived class I have a base and derived class since I need to have different kinds of concrete classes.
I cannot use the getInstance() function in the base class since I cannot make it both static and virtual.

class ITrafficEventsReturner {

    public:    virtual ~ITrafficEventsReturner() {} ;

    protected:

       ITrafficEventsReturner() { };

       // no definition for copy constructor and assignment operator
       ITrafficEventsReturner(const ITrafficEventsReturner&);
       ITrafficEventsReturner& operator=(const ITrafficEventsReturner&); };

    class ConcreteTrafficReturner : public ITrafficEventsReturner {

    public:
        static ConcreteTrafficReturner* getInstance() {
          if (myself == 0)
          {
             myself = new ConcreteTrafficReturner();
          }

          return myself;    }

       ~ConcreteTrafficReturner()
        {
          if (myself)
          {
             delete myself;
             myself = 0;
          }
        }

    private :    ConcreteTrafficReturner() : ITrafficEventsReturner()    {

       };

       static ConcreteTrafficReturner* myself; };

    ConcreteTrafficReturner* ConcreteTrafficReturner::myself = 0;

    int main()
    {
       ITrafficEventsReturner* ConcreteTrafficReturner =
            ConcreteTrafficReturner::getInstance();

       delete ConcreteTrafficReturner;
    };

This seems to be the possible solution :

class SingletonInterface
{
public:
   template <class T>
   static T& getInstance()
   {
      return T::getInstance();
   }

private:
   SingletonInterface() { };
};

class TrafficGenerator
{
public:

   static TrafficGenerator& getInstance()
   {
      static TrafficGenerator obj;
      return obj;
   }

   void whatAmI()
   {
      std::cout << " I am Traffic Generator" << std::endl;
   }

private:

   TrafficGenerator () {};
};

int main()
{

   TrafficGenerator& hTG = SingletonInterface::getInstance<TrafficGenerator>() ;
   hTG.whatAmI();
};

Please never do this again:

    static ConcreteTrafficReturner* getInstance() {
      if (myself == 0)
      {
         myself = new ConcreteTrafficReturner();
      }

Problem 1:

You are not providing a concept of ownership. The user of your class has to actually read the code to understand that you are not only returning the pointer but ownership of the pointer.

Problem 2:

You are returning a pointer. So who decides when it should be deleted. What happens if the user of your class forgets to call delete!

A better way to implement this is:

    static ConcreteTrafficReturner& getInstance()
    {
       static ConcreteTrafficReturner    instance;    // created the first time
                                                      // get instance is called.
                                                      //
                                                      // automatically destroyed
                                                      // with other static storage
                                                      // duration objects.

        return instance;
    }

But really the above is a text book example (not a real world example) of a singelton. The singelton pattern is actually dangerous in that it binds your implementation to an object with mutable state. As such it should not be used in isolation it should be used in conjunction with a creator pattern (A factory is usually a good choice).

Note: Singelton is a design pattern. But in a lot of places it is also considered and anti-pattern so be very careful with its usage.

Template problem

Though technically correct. You may find problems with this design when using shared libraries. As each shared library may have its own instantiation of the templated object that is not shared. Thus you actually have multiple instances of your singelton.

Summary:

Personally I would not use a singelton (I think you can use other techniques). But if you have to use a singelton then it would look like this:

class MySingInterface
{
    public:
       virtual ~MySingInterface() {}  // Never foget the virtual destructor of
                                      // a class with virtual functions.
       // Other interfaces
};
class MySingFactory
{
    public:
        virtual ~MySingInterfaceFactory() {}
        virtual MySingInterface& createMySingInterface() = 0;
};

class MySingBuilder
{
    static std::vector<MySingFactory*>   factories;             // This could be a map
                                                                // of factories or
                                                                // anything else that
                                                                // makes sense to your
                                                                // project.

       static MySingInterface& buildInstance()
       {
             // decide what factory to use.
             // then call build (EG if you are in debug mode or unit test mode
             // you may want to build a different type of object from normal).
             //
             // I have shown an example where you can register multiple
             // factories but that may be overkill for you. Maybe you only need
             // to register one factory (which you decide in main and depends
             // on input arguments or config options or environment variables etc...)

             return factories[someIndex]->createMySingInterface();
       }
    public:
       MySingInterface&  getInstance()
       {
           static MySingInterface& instance = buildInstance();
           return instance;
       }

       static void addFactory(MySingFactory& factory)
       {
           factories.push_back(&factory);
       }
};

MySingFactoryForTest   mySingFactoryForTest;
MySingFactoryForNormal mySingFactoryForNormal;

int main()
{
    if (getenv("UNIT_TEST") != nullptr)
    {
        MySingBuilder::addFactory(mySingFactoryForTest);
    }
    else
    {
        MySingBuilder::addFactory(mySingFactoryForNormal);
    }

    MySingInterface& mSI = MySingInterfaceBuilder::getInstance();
}


Simplify jQuery validation I wrote a script for validate the input field for following requirements

Not null
Multiple of hundreds
Less than needed amount

Here is my script:

    $('#investAmount,#invest-button').bind('keypress', function(event){
        if(event.which == 13){
            if(amountValidation('#investAmount','#inNeedAmt','#invalidAmt','#mult100','#needAmtValidation')){
                return false;
            }
            else{$(elementId).removeClass('errorField');}
        }
    });
    $('#invest-button').click( function(event){
        if(amountValidation('#investAmount','#inNeedAmt','#invalidAmt','#mult100','#needAmtValidation')){
            return false;
        }
        else{$(elementId).removeClass('errorField');}
    });

function amountValidation(elementId,needAmt,errorMsgId,errorMsgId2,errorMsgId3){
    var gvAmount =$(elementId).val();

    var errAmount = gvAmount == '';
    if(errAmount == true){$(elementId).addClass('errorField');}
    $(errorMsgId).toggle(errAmount);

    var errGVAmount = !errAmount && !gvAmount.match(/^[1-9]\d*00$/);
    $(errorMsgId2).toggle(errGVAmount);
    if(errGVAmount == true){$(elementId).addClass('errorField');}

    var errNeedAmount =parseInt($(needAmt).val()) <  parseInt(gvAmount);
    $(errorMsgId3).toggle(errNeedAmount);
    if(errNeedAmount == true){$(elementId).addClass('errorField');}

    return errAmount || errGVAmount || errNeedAmount;
}

Actually I am going to use this function in my website multiple places please help me to make this better script.
My jsfiddle is here

I'm not much of a jQuery freak, but I still have some points on this code. In your amountValidation function, you have some duplication and overhead:

if(errAmount == true)

can be written:

if(errAmount)

Also you always do the same thing when an error occurs, but that's in each statement:

if(errAmount == true){/* mark the field */};
if(errGVAmount == true){ /* mark the exact same field */};
if(errNeedAmount == true { /* and yet the same thing again */ };

you can shorten this to:

if(errAmount || errGVAmount || errNeedAmount){ /*mark field*/ };

Your variable names could use some change too:

errAmount on the first look is like the geeneral error overall, not that amount is null / empty.
You could use errAmountNull instead.

errNeedAmount sounds like you have no amount given yet, which would be errAmount.
I would use errAmountTooSmall instead

Nobody knows, what the GV in errGVAmount stands for.
From the requirements I would call it: errAmountUnallowedValue


Getting the three next days I'm simply getting the 3 future days from an array of objects based on the current day to display for a weather component.

This is the first time I've attempted using ES6 and it's features so please take this into consideration.

Is there a better way to do this and save the values? Have I overcomplicated the whole thing?

const days = [
  {0: 'Sun'},
  {1: 'Mon'},
  {2: 'Tue'},
  {3: 'Wed'},
  {4: 'Thu'},
  {5: 'Fri'},
  {6: 'Sat'}
];
let currentDay = new Date(Date.now()).getDay();
const offset = currentDay + 1;
const daysLength = days.length;
let counter = 0;
let newDays = [];

for (const [i, day] of days.entries()) {
  // set the index start point.
  let pointer = (i + offset) % daysLength;
  newDays.push(days[pointer][pointer]);
  counter++
  if (counter === 3) { break; }
}

Yes, you have overcomplicated things. But it's nothing dramatic.

First, you defined a two-dimensional table for getting the name of a day. You can replace this with a one-dimensional array.

const dayNames = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'];

With this definition, you can just say dayNames[pointer] instead of your current dayNames[pointer][pointer].

The for loop should be of the form for (let i = 0; i < 3; i++) { }, which is the idiomatic form of saying three times.

const today = (new Date()).getDay();
const nextDays = [];
for (let i = 0; i < 3; i++) {
  nextDays.push(dayNames[(today + 1 + i) % dayNames.length]);
}

This counts i from 0 to 2. At first it may feel a bit easier to let i count from 1 to 3 instead, making the + 1 unnecessary. Amongst experienced programmers, almost all loops start at 0 though, so whenever you start a loop at a nonzero number, you will confuse them a little bit.


Simple "arrow-throwing" console game I'm trying to make a simple console application game.  The code is a bit messy and I hope anyone can point out anything wrong.

using System;
using System.Collections.Generic;

namespace CalculatePoints
{
class Program
{
    static void Main(string[] args)
    {
        Game game = new Game();
        game.PlayGame();
    }
}

class Game
{
    private List<Player> player = new List<Player>();

    public void AddPlayer(string name)
    {
        Player person = new Player(name);
        player.Add(person);
    }

    public void PlayGame()
    {
        Console.Write("enter amount of player: ");
        int playerAmount = Convert.ToInt32(Console.ReadLine());

        for (int i = 0; i < playerAmount; i++)
        {
            Console.Write("enter player name: ");
            string playerName = Console.ReadLine();
            AddPlayer(playerName);
        }

        Console.WriteLine("game start");
        foreach (Player name in player)
        {
            Console.WriteLine(name + " turn ");

            Console.WriteLine("enter score 1:");
            int score1 = Int32.Parse(Console.ReadLine());

            Console.WriteLine("enter score 2:");
            int score2 = Int32.Parse(Console.ReadLine());

            Console.WriteLine("enter score 3:");
            int score3 = Int32.Parse(Console.ReadLine());

            Throw result = new Throw(score1, score2, score3);
            Console.WriteLine(result.GetScore());
        }
        Console.ReadKey();
    }
}

class Player
{
    public string Name { get; set; }
    public List<Throw> throwList = new List<Throw>();

    public Player(string name = "")
    {
        Name = name;
    }

    public override string ToString()
    {
        return Name;
    }
}

class Throw
{
    private int throwOne;
    private int throwTwo;
    private int throwThree;

    public Throw(int throwOne = 0, int throwTwo = 0, int throwThree = 0)
    {
        this.throwOne = throwOne;
        this.throwTwo = throwTwo;
        this.throwThree = throwThree;
    }

    public int GetScore()
    {
        return throwOne + throwTwo + throwThree;
    }

    public override string ToString()
    {
        return string.Format("Your total score is {0}", GetScore());
    }
}
}

I like that your Program class is kept to a minimum. That's a very good start.

The player field in the Game class has a bad/misleading name: it's not one player, it's a list of players. players would be better. Also I'd make that field readonly to make it clear that the reference of that list cannot be tampered with. I'd also consider passing that list of players through the constructor as an argument... but that would break your code, because PlayGame is doing too many things.

The first part of PlayGame should be extracted out of the class: it's more of a GetGamePlayers method than a part of the actual game code.

The name Throw, throws me off: throw is a keyword reserved for throwing exceptions; considering the nature and purpose of the class, perhaps PlayerTurn would be a better choice.

The Player class is doing something terribly wrong: it's exposing a public field! The throwList can be reassigned and messed with from the outside, which completely ruins encapsulation. Consider exposing it with an IEnumerable<T> property getter instead.

Similarly, the Name setter should be private.

Also, playerAmount is really a playerCount - "amount" is typically better suited for quantities of something, money in particular comes to mind.

For flexibility, you shouldn't hard-code the 3 throws in your Throw (/PlayerTurn) class: should the rules change and players are now allowed a 4th dart, or if a player's turn can end with only one or two darts thrown, the hard-coded throwOne, throwTwo and throwThree become a problem. Granted, the optional parameters help a bit, but why store values that you don't need? I'd have gone with a List here too.


Monitoring navigation function: logic judgment Write a simple monitoring navigation function.

I feel logic judgment still needs to be optimized.
I don't have a clear idea right now about code logic such as reducing if statements
How can I optimize the performance of this code?

A few comments:

Array.forEach takes a callback, this can be done directly as so:  ems.forEach(watchNav)
Also, you have scope conflicts: the global variable em, and the function parameter em
A third comment: You should n ot edit your question based on feedback in comments.
Your if statement is fine.

<script>
    var ems = document.querySelectorAll(".panel"),
        viewHeight = window.innerHeight,
        clientHeight = document.body.clientHeight,
        timeOut = null;

    ems.forEach(watchNav);

    function watchNav(em) {
        var emHeight = em.offsetHeight;
        var emTop = em.offsetTop;
        clearTimeout(timeOut);

        window.addEventListener("scroll", function (e) {
            var scrollY = window.scrollY;

            timeOut = setTimeout(function () {
                // Logical judgment: how to optimize, want better implementation.
                // Such as reducing the if statement
                // ++++++++++++++++
                if (scrollY > emTop || scrollY + viewHeight / 2 > emTop || scrollY + viewHeight > emTop + emHeight) {
                    console.log("...activing...");
                    em.classList.add("view-focus");
                } else {
                    console.log("none");
                    em.classList.remove("view-focus");
                }

                if (scrollY > emTop + emHeight / 2) {
                    console.log("none");
                    em.classList.remove("view-focus");
                }
                // ++++++++++++++++
            }, 40);
        }, false);
    }
</script>
```


Performance Test and Idea: Initialize a widget I want to extend an app object with a widget that initializes itself immediately.

Immediately-Invoked Function Expression:

var app = {};

(function() {
  app.widget = {
    init: function() {
      return true;
    }
  };

  app.widget.init();
}());

Immediately-Invoked Object Expression:

var app = {};

({
  init: function() {
    app.widget = this;

    return true;
  }
}.init());

Please take a look: http://jsperf.com/iife-vs-iioe

Code is not just about performance. You have to check for maintainability, scalability and readability as well.

With regards to the way code is written, I'd favor the first method because it's the module pattern and is a common convention. The second one is too full of brackets and the object notation, though flexible, is nasty especially when you forget the commas.

For extendability, I suggest you build a base set of helper functions to facilitate module loading and phase handling. Here are potential events that you might want to hook on events:

when the DOM is ready (DomContentLoaded)
when everything is loaded (window.onload)
when the page is unloading (window.onunload and onbeforeunload)
when the widget is loaded into the library
when the widget is unloaded/removed
when the widget is first run (init)
and so on...

Also, I suggest the following style of extension since:

It creates a single local scope for your widget to operate and share. You have the so called "private sandbox".
Objects are provided as an interface to attach stuff like public methods/properties, handlers and stuff. Whatever attached will be collected by your framework, sorted and executed accordingly.
It's simple and readable!

Here's a sample of a widget:

//assumung app is your framework namespace
app.addWidget('WeatherWidget',function(handles,publicStuff){

  //your local scope
  var foo = 'bar';

  function baz(){console.log('bam');}

  //handles and public are your interfaces
  //in this example, handles will be used to collect your event handlers

  handles.init = function(){/*run on init*/};
  handles.documentReady = function{/*run on DOM ready*/};

  publicStuff.getBar = function(){return foo;}

});


Giving money to the bank Lets say we need to write an algorithm for giving money to the bank. We need to know interest rate, money which we put every year on our account, and amount of money we want to get. F.e int. rate is 1%, money we put every year is 1000 (or whatever) and we want to get 10000 (not more). It'd look like this:

MoneyOnAcc=1000*1.01
MoneyOnAcc=(MoneyOnAcc+1000)*1.01
Until MoneyOnAcc reaches 10000 (we want to print amount what was before
10000 - 9462.21)

and so on. Here is my code in Pascal.

program whilel;

var interestRate:real;
    moneyEveryYear:real;
    accMoneyOut:real;
    totalAmount:real;
    accMoneyControl:real;
begin
readln(moneyEveryYear);
readln(interestRate);
readln(totalAmount);

while accMoneyOut<totalAmount do begin
    accMoneyControl:=(accMoneyOut+moneyEveryYear)*((interestRate/100)+1);

    if accMoneyControl>totalAmount then break;

    accMoneyOut:=(accMoneyOut+moneyEveryYear)*((interestRate/100)+1);

end;
writeln(accMoneyOut:6:2);
readln();
end.

How can I improve this algorithm? Maybe some better logic will help.

Care about indentation. Pascal is usually written with 2 space indentation.
Repeat is a posttest loop, and your code needs to run at least once, so why not a repeat..until loop. You can save a begin..end block, as well.
you should explain to your self the inputs, unite measures and all the info you need to properly insert the requested variables.
in pascal, prefixing your own variables is good practice for complex code (not your case). Take a look at this answer. I choose to prefix our global variables with 'G'; Also, functions and procedures goes with CapitalLetters.
Avoid repetition, I choose to use GAccMoneyOut:=GMoneyControl; instead of repeating the code to raise the money.
Care about the user, you should use readkey and not readln at the end. ReadLn increases the chance of getting lost.

Putting all together:

program whilel;

uses Crt;

var
  GInterestRate,
  GMoneyEveryYear,
  GTotalAmount,
  GMoneyControl,
  GAccMoneyOut:real;

begin
  WriteLn('Enter the porcentage interest rate');
  ReadLn(GInterestRate);

  WriteLn('Enter the money you will put every year into the account');
  ReadLn(GMoneyEveryYear);

  WriteLn('Enter the money you want to get');
  ReadLn(GTotalAmount);

  GAccMoneyOut:=0;
  GMoneyControl:=0;
  repeat
    GMoneyControl:=(GAccMoneyOut+GMoneyEveryYear)*((GInterestRate/100)+1);
    if GMoneyControl>GTotalAmount then
      Break
    else
      GAccMoneyOut:=GMoneyControl;
  until GAccMoneyOut=GTotalAmount;

  WriteLn('Result is:');
  WriteLn(GAccMoneyOut:6:2);

  WriteLn('Press any key to exit...');
  Readkey;
end.


Check “five in a row” for a 15x15 chess board (part 2) This is a follow-up from here. I have a further optimization for space complexity by store only direction status data for one row for each directions (refer to variable stats). Any advice on algorithm time complexity improvement, code bugs or code style advice is highly appreciated.

  Problem:

  For a given board, judge who had complete a continuously line (could
  be horizontal, vertical or diagonal) of 5 pieces. Input is a 15×15
  chess board, could be white (1), black (2) or empty (0). Return if
  white win, if black win, if both win or if none of them win.

import random
import pprint
def check_five(matrix):
    directions = [(0,-1),(-1,0),(-1,-1),(-1,1)] # (row, col)
    stats = [[0] * len(directions) for _ in range(len(matrix[0]))]
    WHITE = 1
    BLACK = 2
    EMPTY = 0
    WIN = 5
    BLACK_WIN = False
    WHITE_WIN = False
    # init first row
    for i in range(len(matrix[0])):
        if matrix[0][i] != EMPTY:
            for j in range(len(directions)):
                stats[i][j] = 1
    for row in range(1, len(matrix)):
        for col in range(0, len(matrix[0])):
            for d_i, d_v in enumerate(directions):
                if matrix[row][col] == EMPTY:
                    stats[col][d_i] = 0
                elif len(matrix)>row+d_v[0] >= 0 and len(matrix[0])>col+d_v[1] >= 0 and matrix[row][col] == matrix[row+d_v[0]][col+d_v[1]]:
                    stats[col][d_i] += 1
                else:
                    stats[col][d_i] = 1
                if stats[col][d_i] == WIN:
                    if matrix[row][col] == WHITE:
                        WHITE_WIN = True
                    else:
                        BLACK_WIN = True
                    if WHITE_WIN and BLACK_WIN:
                        return (True, True)

    return (WHITE_WIN, BLACK_WIN)

if __name__ == "__main__":
    matrix = [[random.choice([0,1,2]) for _ in range(15)] for _ in range(15)]
    pprint.pprint(matrix, indent=4)
    print check_five(matrix)

What an interesting little problem!

You don't need to accumulate any stats at all, if you just test each cell for whether it is at the centre of a line of 5 all the same colour. This also means that, for example, you don't need to look at first two and last two columns when searching for a horizontal line, and similarly for rows and diagonals.

Here's a (rather wholesale) modification of your code. All the real work is done in the winning_cell() function, which just tests the four lines that this cell is at the centre of. This leaves the main loop in check_five() very clean.

Although there's a lot of work in winning_cell, on a randomised matrix this runs about 30% faster.

(EDIT: this is in Python 3.6 - I've just noticed you are on 2.7. Apart from the brackets round the print statement I'm not sure what else this would affect.)

import random
import pprint

# Offsets from the current cell to cells that might form five in a row
# with the current cell as centre, calculated here so as to be outside
# the main loop
OFFSETS = (-2,-1,+1,+2)
DIAG_1 = [*zip(OFFSETS,OFFSETS)]
DIAG_2 = [*zip(OFFSETS,reversed(OFFSETS))]

def winning_cell(m, row, col):
    """ True if the given cell is at the centre of a row of 5 """
    # Note that when checking for a horizontal line we don't need to
    # check the first two or last two columns - the same logic applies
    # to rows and diagonals.
    colour = m[row][col]
    # check within row
    if (1 < col < len(m[0]) - 2 and
        all([m[row][col + c] == colour for c in OFFSETS])):
        return True
    # check within column
    if (1 < row < len(m) - 2 and
        all([m[row+r][col] == colour for r in OFFSETS])):
        return True
    #check diagonals
    if (1 < col < len(m[0]) - 2 and
        1 < row < len(m) - 2 and
        (all([m[row + r][col + c] == colour for r, c in DIAG_1]) or
         all([m[row + r][col + c] == colour for r, c in DIAG_2]))):
        return True
    return False

def check_five(matrix):
    WHITE = 1
    BLACK = 2
    EMPTY = 0
    BLACK_WIN = False
    WHITE_WIN = False

    # main loop to check cells
    for row in range(len(matrix)):
        for col in range(len(matrix[0])):
            cell = matrix[row][col]
            if (cell == EMPTY or
                cell == WHITE and WHITE_WIN or
                cell == BLACK and BLACK_WIN):
                pass
            else:
                if (cell == WHITE and winning_cell(matrix, row, col)):
                    WHITE_WIN = True
                if (cell == BLACK and winning_cell(matrix, row, col)):
                    BLACK_WIN = True
                if WHITE_WIN and BLACK_WIN:
                    return (True, True)

    return (WHITE_WIN, BLACK_WIN)

if __name__ == "__main__":
    matrix = [[random.choice([0,1,2]) for _ in range(15)] for _ in range(15)]
    pprint.pprint(matrix, indent=4)
    print (check_five(matrix))


Data structure that is fast to read and modify I'm working on an apllication that needs to process a lot of objects in realtime. I've separated the logic from the UI so that it isn't as much of a mess. The basic idea is that every tick I'm trying to populate a list of objects to process and once they are processed I update the UI accordingly.

I've written this collection class for use in the main processing loop. Here is the reasoning behind some of my decisions:

The objects are processed sequentially in a loop. I only care for fast sequential reading so I've decided to use arrays at the base of it all hoping to get some kind of cache hit benefit.
The maximum amount of objects is known at compile time so I've tried to avoid rapid heap allocations by pooling my data objects. I allocate a maximum amount but only use the amount that is active.
I needed a way to correlate the logical objects with the presentation layer, so I've decided to use identifiers. These identifiers allow me to know what actual UI object the data belongs to.

Through basic stopwatch benchmarking I've arrived to the following solution:

class FixedCapCollectionIntGuids<T> where T : struct
{
  public T[] m_array;
  Dictionary<int, int> m_guidToIDs;
  int[] m_idToGuids;
  int m_capacity;

  public int Size { get; private set; }

  public FixedCapCollectionIntGuids(int capacity)
  {
    Size = 0;
    m_capacity = capacity;
    m_array = new T[capacity];
    m_guidToIDs = new Dictionary<int, int>(capacity);
    m_idToGuids = new int[capacity];
  }

  public T this[int key]
  {
    get
    {
      Debug.Assert(key < Size, "Illegal read");
      return m_array[key];
    }
  }

  public void Add(T value, int guid)
  {
    Debug.Assert(!m_guidToIDs.ContainsKey(guid), "Double insertion");
    m_array[Size] = value;
    m_guidToIDs[guid] = Size;
    m_idToGuids[Size] = guid;
    ++Size;
  }

  public void Remove(int guid)
  {
    Debug.Assert(m_guidToIDs.ContainsKey(guid), "Not found");
    int i = m_guidToIDs[guid];
    int lasti = Size - 1;
    int lastGuid = m_idToGuids[lasti];
    m_array[i] = m_array[lasti];
    m_guidToIDs.Remove(guid);
    m_guidToIDs[lastGuid] = i;
    m_idToGuids[i] = lastGuid;
    --Size;
  }
}

Please try to ignore the dumb naming, this is WIP so I didn't put much thought into what the best names for variables and classes are at this point.

My benchmarks show that this approach appears to be faster than just having a dictionary for reading(presumably because an array is cache friendlier), but slower for adding\removing(presumably because I'm maintaining 3 separate collections instead of one). I'm somewhat fine with this because addition and removal is going to be less frequent than iteration, but I'm still wondering whether I've solved the problem in a stupid way or not. Can it be done better?

This looks like a good (i.e. not stupid) implementation if your main requirements is efficient sequential access to the data. Addition and removal will necessarily be slower than for a Dictionary because you are wrapping a dictionary, but the time complexity is of the other work is constant, so it isn't something I would worry about too much if your goal is fast read/modification.

Encapsulation

Generally the encapsulation is fine, but there is one exception: m_array should not be publically settable. It should be readonly or a getter-only {get; } property. Being a reference type, I can't think of any downside to making it a property. Being publically settable means that anyone can change it to null or an array of the wrong size and break the data-structure, which is obviously bad. The other array and dictionary should also be readonly.

I do wonder why you provide an indexer as well as the array, but do not provide a setter for the indexer. The indexer will also create confusion by copying structs you want to mutate. You should probably look at whether you can hide m_array altogether, and do all the work via the indexer: this would produce a much tighter API if you can 'get away' with it. You may be interested to look into ArraySegment as a possible means of facilitating a somewhat tighter API if that indexer is deficient.

Exception Handling

I don't know you use-case, but I'd reconsier using Debug.Assert for input sanitisation. It would seem much better would be to exploit the TryGetValue method of Dictionary'2 in Remove and TryAdd in Add to efficiency perform the check and throw under any runtime conditions.

Your exception messages could also be clearer. "NotFound": what is not found? If you throw an exception instead of asserting, you can throw an ArgumentException and make it completely clear what was not found and where the caller went wrong. "IllegalRead" means nothing at all. "Double Insertion" is pretty cryptic.

You could also do with checking that Size is less than Capacity when adding, as this should also throw an exception, rather than relying on the order of operations to throw with an IndexOutOfRange exception before doing any damanger (if m_guidToIDs[guid] was set first, this would linger despite the operation failing, so the code is extremely brittle).

A check in the constructor that capacity is non-negative would also be nice, just to cover as much of the API surface as possible with clear and useful exceptions (it will still fail if it tries to allocate a negative length array, but an ArgumentOutOfRangeException would be so much nicer.

Typical C♯

The code is mostly nice-to-read C#, though there are some bits that could be nicer:

The m_ member prefix is typical in C++, but much less so in C#. m_array should have a name consistent with being a public member: it should be in ProperCamelCase.
Generally it is advised that the accessibility of any member of type is made explicit e.g. public class FixedCapCollectionIntGuids<T>, private int m_capactiy). This avoids any confusion (particularly when people are used to using different languagse with different defaults), and makes it completely clear that this is the intention and you didn't just forget to qualify the accessibility.
The BCL uses Count rather than Size for everything: it's what people using C# will expect. I'd also argue it is clearer, because Size has can relate to size in memory. Conformity with the BCL also be sensible if you wanted to implement IReadOnlyList<T> (for which you would need to provide in addition an enumerator but nothing more), which could be useful.
It's also typical to have key, value in the BCL add methods, so you might want to consider having the same in Add.
Usually we use 4 spaces for indentation.

Other Comments

While I think I see why you would except this to only be used with structs, I don't see why you need limit this general purpose data structor to only structs. If you don't have a reason, I would remove the generic constraint (it has no semantic/performance impact for when structs are used). The only change this would warrant would be clearing removed values (so that you don't hold references to objects that are not in the structure). This is just a suggestion, and 'that would increase the API surface and costs of maintance/testing' would be a good reason to not do this.
public T this[int key] should have parameter guid for consistency.
FixedCapCollectionIntGuids is an odd name; I will only say that shorting Cap to Capacity is not a good idea: it only obfuscates the meaning.
For a 'tricky' API like this one (where it's very easy to mis-use m_array), you need to provide inline documentation (///) for all public parts of the API. This should detail the usage and assumptions, and explain things like "this is not thread safe at all", and that m_array may be larger than Size and you should not access anything beyond index Size - 1. I find that the act of writing documentation makes me aware of the edge cases I need to handle, makes it possible to maintain code without having to guess at the exact intention of a method/class, and gives consumers the information and confidence they need to use your APIs as intended.
Remove is a bit untidy looking: I would try to break it up a bit with empty lines, to separate the bit where you remove the old values, move the 'last' value into the inner position, and finally resize the structure. There is also a bug in Remove: it will go wrong if you try to remove the last element: writing the code to detect this should automatically force you to organise it so that my previous comment is addressed.


Pool of curl handles I am trying to write a simple reusable pool of curl handles. I found this wonderful implementation of the blocking queue.

The pool itself looks like this:

class curl_pool
{
private:
    int        m_size;
    queue<int> m_queue;
    CURL     **m_handles;
public:
    class connection
    {
        friend class curl_pool;
    private:
        curl_pool &m_pool;
        int        m_index;
        connection(curl_pool &pool, int index) : m_pool(pool), m_index(index) {}
    public:
        connection(connection&& that) : m_pool(that.m_pool), m_index(that.m_index) {}
        CURL *handle() const {
            return m_pool.m_handles[m_index];
        }
        void release() {
            m_pool.m_queue.push(m_index);
        }
        virtual ~connection() {
            release();
        }
    };
    connection open() {
        return connection(*this, m_queue.pop());
    }
    curl_pool(const std::string &root_cert, int size) : m_size(size) {
        curl_global_init(CURL_GLOBAL_DEFAULT);
        m_handles = new CURL*[m_size];
        for (int i = 0; i < m_size; i++) {
            CURL *curl = curl_easy_init();
            curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);
            curl_easy_setopt(curl, CURLOPT_CAINFO, root_cert.c_str());
            curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L);
            curl_easy_setopt(curl, CURLOPT_TCP_KEEPIDLE, 120L);
            m_handles[i] = curl;
            m_queue.push(i);
        }
    }
    virtual ~curl_pool() {
        for (int i = 0; i < m_size; i++) {
            curl_easy_cleanup(m_handles[i]);
        }
        delete m_handles;
        curl_global_cleanup();
    }
};

This is the intended use:

{
    curl_pool::connection c = pool.open();
    CURL *curl = c.handle();
    curl_easy_setopt(curl, CURLOPT_URL, "https://www.***.com/");
    CURLcode res = curl_easy_perform(curl);
    stream << "Status code: " << res;
}

I am not a C++ person at all, so I am certain there are a lot of things to improve.

Please stop using new and delete.

This is like reading old C code. Modern C++ rarely uses new and practically never uses delete (unless you are building stuff at a very low level). You should be using containers or smart pointers (always question the use of new and if you see a delete you are not using the facilities of the language).

curl_pool(const std::string &root_cert, int size) : m_size(size) {
    m_handles = new CURL*[m_size];
}

virtual ~curl_pool() {
    delete m_handles;
      // Also note this should have been `delete [] m_handles;`
}

This breaks separation of concerns. You are doing memory management and business logic in the same object.

Also because you don't follow the rule of three. Your code is broken and will likely cause crashes.

The best way to fix this is to change m_handles into a vector.

std::vector<CURL*>     m_handles;

Let the vector handle the memory management. You can handle the business logic. PS. You still need to implement the rule of three.

Your connection is dangerious:

    void release() {
        m_pool.m_queue.push(m_index);
    }

What happens if I call release multiple times? I should not be able to break the code from the outside of the interface. Once you have released the handle you should make sure that a subsequent release does nothing and getting a CURL* should return nullptr.

Note I: Calling release() once will break the code. As the object will subsequently be destroyed and call release again.

Note II: Because your object is copyable. Passing it as a parameter will also break the code. As a copy is made and both the original and the new version will eventually be destroyed.

Looks like this should be a movable but not copyable object.


Can reverse it the input with less code than these? Java So we ask to the user to enter 10 number and then we reverse it.

package *;
import java.util.Scanner;
public class * {
   // n = limit of inputs
   private static final int n = 10;
   static Scanner kb = new Scanner(System.in);
   public static void main(String[] args) {
     //nkb = number from keyboard
     int [] nkb = new int[n];
     // in = number of length
     int in = nkb.length;
     //for input user
     for (int i = 0; i < n; i ++){
       System.out.println("Writte a number: ");
       nkb[i] = kb.nextInt();
     }//end for
     for (int i = 0; i < n; i ++){
       System.out.println("In these position "+i+" now we have these number: "+nkb[in-1-i]);
     }//end for reverse
   }//end method
}//end class

It's possible to make it with less code?
I check already and they told me that we can use List<Integer> and call Collections.reverse()... but how?

Searching on the site maybe this can help.

for(int i = 0; i < validData.length / 2; i++) {
  int temp = validData[i];
  validData[i] = validData[validData.length - i - 1];
  validData[validData.length - i - 1] = temp;
}

To reverse an int array


JavaScript/HTML web application - user interface logic I am writing a JavaScript/HTML driven web application. For the user interface, I am not completely sure that my JavaScript is 'OK'. Mainly, I switch between "modes" by assigning a placeholder variable to a new function, which changes what the onclick event does for items in a table (either open, edit, or move them).

Am I breaking any code styles? Is my script hard to follow or understand? Is it unconventional for me to assign bookmarkEventMapping to another function later on in my code, using it as a placeholder? What can I do differently? Can you recommend any material that I could read to write clearer JavaScript?

(Please note that this is a WIP)

var bookmarkEventMapping = function() {};//placeholder
/*
    ...
    more JavaScript, which creates &lt;li&gt; elements
    that all have onclick events, example:
        element.addEventListener(
            "click",
            function(e) { bookmarkEventMapping(e); }
        );
    ...
*/
function initModeMenu() {//called in &lt;body&gt; onload event
    var modeNames = ["normal","edit","move"];//the order in HTML menu
    for(var i = 0; i &lt; 3; i++) {,
        var node = document.getElementById("modeMenu").children[i];
        bookmarkActions[ modeNames[i] ].node = node;
        node.dataset["action"] = modeNames[i];
        node.addEventListener(
            "click",
            function() {
                bookmarkActions.setCurrentMode( this.dataset["action"] );
            }
        );
    }
    bookmarkActions.setCurrentMode("normal");
}
var bookmarkActions = {
    "currentMode":"normal",
    "setCurrentMode":function(modeName) {
        var prevMode = bookmarkActions.currentMode;
        bookmarkActions[ prevMode ].exitMode();
        bookmarkActions[ prevMode ].node.classList.remove("selected");
        bookmarkEventMapping = bookmarkActions[ modeName ].eventMapping;
        bookmarkActions[ modeName ].node.classList.add("selected");
        bookmarkActions.currentMode = modeName;
    },
    "normal":{
        "initMode":function() {
        },
        "exitMode":function() {
        },
        "eventMapping":function(ev) {
        }
    },
    "edit":{
        "initMode":function() {
        },
        "exitMode":function() {
        },
        "eventMapping":function(ev) {
            if(ev.type == "click")
            {
                bookmarkActions.edit.selectBookmark( ev.target );
            }
        },
        "selectBookmark":function(itemNode) {
            itemNode.dataset['bookmarkItemId'];
            itemNode.classList.add("editing");//.remove later
        }
    },
    "move":{
        "initMode":function() {
        },
        "exitMode":function() {
        },
        "eventMapping":function(ev) {
        }
    }
};

Below is a relevant snippet of HTML from the

<menu id="modeMenu">

Normal

Edit

Move

</menu>

Readability - the code reads fine.  The modes concept is explained well just by reading the code, so I wouldn't worry about that.

Code style - Javascript programmers are used to passing functions around and probably wouldn't bat an eyelid at the idea of assigning different functions to a variable, but it makes more sense to me to always have the same function as the event handler, and call the action for the current mode from within it:

var bookmarkEventMapping = function(e) {
    bookmarkActions[bookmarkActions.currentMode].eventMapping(e);
};

A couple of other things I would do, out of personal preference more than anything:

store the list of modes separately, not in the bookmarkActions object
refer to the modes using references to the actual objects, as opposed to using the object property names.

If you did store the mode list separately, you could also make things cleaner by making that list the only place where information about the modes is stored and using it to generate the html, instead of having to maintain the html and the code in initModeMenu separately.