Helper class to change property of string when serializing for Json.Net Is there any way to simplify the following C# helper class to numerate a string for Json.Net?

internal static string CreateContact(string title)
{
    var createContact = new {Title = title};
    string output = JsonConvert.SerializeObject(createContact);
    return output;
}

[JsonProperty(PropertyName = "9")]
public string Title
{
    get
    {
        return _title;
    }
    set
    {
        _title = value;
        _title = GetTitleId(Title).ToString();
    }
}

public static int? GetTitleId(string title)
{
    //remove "."
    var titleRemoveDots = title.Replace(".", "");
    var titleLowerCase = titleRemoveDots.Trim().ToLower();
    switch(titleLowerCase)
    {
        case "dir":
            return 1;
        case "dr": case "doctor":
            return 4;
        case "mag": case "magister":
            return 5;
        case "ing":
            return 6;
        case "dipling": case "dipl":
            return 7;
        case "prof": case "professor":
            return 8;
        case "dkfm":
            return 9;
        case "prok":
            return 12;
        default:
            return null;
    }
}

Just a quick point, regarding readability; the switch block would be better off written like this:

//remove "."
var titleRemoveDots = title.Replace(".", "");
var titleLowerCase = titleRemoveDots.Trim().ToLower();
switch(titleLowerCase)
{
    case "dir":
        return 1;
    case "dr":
    case "doctor":
        return 4;
    case "mag":
    case "magister":
        return 5;
    case "ing":
        return 6;
    case "dipling":
    case "dipl":
        return 7;
    case "prof":
    case "professor":
        return 8;
    case "dkfm":
        return 9;
    case "prok":
        return 12;
    default:
        return null;
}

The absence of break; between cases (/ the presence of return in all cases) can make it harder to refactor the switch block later. I think it would be better to separate the concept of figuring out the return value and that of returning a value:

int? result;

//remove "."
var titleRemoveDots = title.Replace(".", "");
var titleLowerCase = titleRemoveDots.Trim().ToLower();
switch(titleLowerCase)
{
    case "dir":
        result = 1;
        break;
    case "dr":
    case "doctor":
        result = 4;
        break;
    case "mag":
    case "magister":
        result = 5;
        break;
    case "ing":
        result = 6;
        break;
    case "dipling":
    case "dipl":
        result = 7;
        break;
    case "prof":
    case "professor":
        result = 8;
        break;
    case "dkfm":
        result = 9;
        break;
    case "prok":
        result = 12;
        break;
    default:
        break;
}

return result;

Now, call me a switch-hater, I'd probably end up with something like this:

public enum PersonTitle
{
    Director = 1,
    Doctor = 4,
    Magister = 5,
    Ingineer = 6,
    Dipling = 7,
    Professor = 8,
    Dkfm = 9, // todo: give readable name
    Prok = 12 // todo: give readable name
}

private static _titleIds = new Dictionary<string, PersonTitle> {
                             { "dir", PersonTitle.Director },
                             { "dr", PersonTitle.Doctor },
                             { "doctor", PersonTitle.Doctor },
                             { "mag", PersonTitle.Magister },
                             { "magister", PersonTitle.Magister },
                             { "ing", PersonTitle.Ingineer },
                             { "dipl", PersonTitle.Dipling },
                             { "dipling", PersonTitle.Dipling },
                             { "prof", PersonTitle.Professor },
                             { "professor", PersonTitle.Professor },
                             { "dkfm", PersonTitle.Dkfm },
                             { "prok", PersonTitle.Prok }
                         };

And then GetTitleId could look like this:

public static int? GetTitleId(string title)
{
    var cleanTitle = title.Replace(".", string.Empty).Trim().ToLower();
    PersonTitle result;

    if (_titleIds.TryGetValue(cleanTitle, out result))
    {
        return (int)result;
    }
    else
    {
        return null;
    }
}

One issue is that Title is actually a string representation of the int value, which can be allowed to be null - I might be mistaken, but it looks like the setter for Title would throw a NullReferenceException if/when that is the case, because it's calling ToString() on null:

[JsonProperty(PropertyName = "9")]
public string Title
{
    get
    {
        return _title;
    }
    set
    {
        _title = value;
        _title = GetTitleId(Title).ToString();
    }
}

I'd expect a null-check there:

 set
 {
     _title = value;
     _title = (GetTitleId(Title) ?? string.Empty).ToString();
 }


Importing a report through the enabler4excel add-in This connects to Salesforce to import a report through the enabler4excel add-in. The report overwrites what is currently there on the sheet (2016 Incidents Dashboard). The report contains about 55,000 rows and 10 columns each. I also have 8 columns of formulas (nothing complex) off to the right. The next section autofills the Excel formulas from the last row on the right to match that of the report on the left.

This works, but the only problem is that it seems awfully slow: about 2 minutes. How can I do this so it doesn't take forever?

Sub ButtonClick1()

Dim addin As Office.COMAddIn
Dim automationObject As Object
Dim error As String
Dim error1 As String
Dim Result As Boolean
Dim ReportResult As String
Dim LastRowLeft As String
Dim LastRowRight As String
Dim sheetname As String

sheetname = ActiveSheet.Name
For Each addin In Application.COMAddIns
If addin.Description = "Enabler4Excel" Then
Set automationObject = addin.Object
End If
Next addin

Result = automationObject.LogIn("myemail.org", "mypassword",              "https://login.salesforce.com", error)
If Result = False Then
MsgBox error
End
End If

ReportResult = automationObject.RunReport("00Od0000004yulX", error1)

Dim wb As Excel.Workbook
Set wb = ThisWorkbook
Set ws = wb.Sheets("2016 Incidents Dashboard")

With ws.QueryTables.Add(Connection:="TEXT;" & ReportResult,           Destination:=ws.Range("A1"))
.TextFileParseType = xlDelimited
.TextFileCommaDelimiter = True
.RefreshStyle = xlOverwriteCells
.Refresh
End With

Result1 = automationObject.LogOut()

Worksheets("2016 Incidents Dashboard").Activate
LastRowLeft = ws.Range("A" & ws.Rows.Count).End(xlUp).Row
LastRowRight = ws.Range("M" & ws.Rows.Count).End(xlUp).Row
Range("M" & LastRowRight & ":U" & LastRowRight).Select
Selection.AutoFill Destination:=Range("M" & LastRowRight & ":U" & LastRowLeft)

ActiveWorkbook.RefreshAll
 Worksheets(sheetname).Activate
End Sub

Readability & Maintainability

Use indentation to make control-structures easier to understand and read.
Some variables are all lower-case, some are camelCase, some are PascalCase. OCD is unhappy :( Use a consistent naming convention. I'd recommend camelCase. Either way, choose one and stick to it
ButtonClick1 is a rather non-descriptive name
these "random" spaces before the third argument look strange. I assume they're not there in your own code, right?

Compartmentalization
You should be able to make your code into neat little compartments. Those are also known as "methods", "functions", "procedures"  or a handful of other names. All these separate paragraphs that have been mentioned in a comment already can almost be methods. Consider:
Set automationObject = FindEnablerAddin()
' [...]
Private Function FindEnablerAddin() As Object
    For Each addin In Application.COMAddIns
        If addin.Description = "Enabler4Excel" Then
            Set FindEnablerAddin = addin.Object
            Exit Function
        End If
    Next addin
    'FindEnablerAddin should be Nothing
End Function

This can be applied in other places as well. Note that I have used Exit Function to break the loop over all COMAddIns early. This may or may not help with performance noticeably. It usually avoids work, so you should do it.
Performance & Nitpicks

You can usually significantly increase performance by turning of Events, Calculation and ScreenUpdating in Excel. Don't forget to turn them back on, even if your code falls into an error state.
Working on the Workbook directly is slow. Then again this looks like you don't have much of a choice here
Instead of If Result = False Then you can use If Not Result Then. This is cleaner and avoids the potential for confusion about assignment and boolean comparisons


Given an array of Strings, return the number of sets of anagrams Please review my code:

public class Anagram {

    public static void main(String args[]) {
        String[] arr = { "abc", "cbc", "bcc", "dog", "god", "mary", "army",
                "rty" };
        HashMap<Integer, List<String>> mapList = new HashMap<Integer, List<String>>();

        for (int i = 0; i < arr.length; i++) {
            int hashcode = gethashcode(arr[i]);

            if (mapList != null && mapList.get(hashcode) == null) {
                List<String> list = new ArrayList<String>();
                mapList.put(hashcode, list);
                list.add(arr[i]);
            } else {
                List<String> list = mapList.get(hashcode);
                list.add(arr[i]);
            }
        }

        printMap(mapList);

        System.out.println("Count=" + mapList.size());
    }

    private static void printMap(HashMap<Integer, List<String>> mapList) {

        if (mapList != null && mapList.size() > 0) {
            for (Integer key : mapList.keySet()) {
                List<String> list = mapList.get(key);

                System.out.print("[");
                if (list != null && list.size() > 0) {
                    for (int k = 0; k < list.size(); k++) {
                        System.out.print(list.get(k) + " ");
                    }
                }
                System.out.print("]");
            }
        }

    }

    private static int gethashcode(String str) {
        int hashcode = 0;
        char ch[] = str.toCharArray();

        for (int i = 0; i < ch.length; i++) {
            if (hashcode != 0) {
                hashcode = hashcode + String.valueOf(ch[i]).hashCode();
            } else {
                hashcode = String.valueOf(ch[i]).hashCode();
            }
        }
        return hashcode;
    }
}

Output:

[mary army ][abc ][rty ][cbc bcc ][dog god ]    Count=5

Bug

I fed "aad" and "abc" to your program, and it considered the two strings anagrams of each other.  The problem is that you are trying to use a hashcode to determine whether two strings are anagrams of each other.  However:

Your hashcode is not very good because it simply adds the characters of the string together.  Therefore "aad" and "abc" were considered equivalent.
No matter what, a 32 bit hashcode is going to have some problems because there are more than 2^32 different anagram possibilities if you use long enough strings.  Even a 64 bit hashcode would not be enough.

I suggest that you think about creating a sorted string (i.e. alphabetized) as your hash key instead of a numerical hashcode.

Simplification

This code:

       if (mapList != null && mapList.get(hashcode) == null) {
            List<String> list = new ArrayList<String>();
            mapList.put(hashcode, list);
            list.add(arr[i]);
        } else {
            List<String> list = mapList.get(hashcode);
            list.add(arr[i]);
        }

could be simplified to:

        List<String> list = mapList.get(hashcode);

        if (list == null) {
            list = new ArrayList<String>();
            mapList.put(hashcode, list);
        }
        list.add(arr[i]);


switchMap with array of Observables I'm trying Rxjs 5 to simplify some Node.js nested callbacks. I need to read a directory (fs.readdir), then read stats of each file (fs.stats) and parse them if they were modified since last sync.

The following code works but I find it a bit odd and not "the rxjs way" because of the first switchMap which is too big!

const fs = require('fs');
const path = require('path');
const { Observable } = require('rxjs');

const lastSync = new Date(2017, 01, 01);
const pathToFolder = '/any/path/';

Observable.bindNodeCallback(fs.readdir)(pathToFolder)
  .switchMap((files) => {
    const array = files.map((fileName) => {
      return Observable.zip(
        Observable.of(fileName),
        Observable.bindNodeCallback(fs.stat)(path.join(pathToFolder, fileName))
      );
    });
    return Observable.concat(...array);
  })
  .filter(([fileName, stats]) => stats.mtime.getTime() > lastSync.getTime())
  .subscribe(([fileName, stats]) => parseFile(fileName));

function parseFile(fileName) { /* ... */ }

How can I improve it?

I'd suggest that the switchMap operator be replaced with mergeMap. Only a single array of files (or an error) is going to be emitted from the bound callback, so it will never be necessary to switch; a merge is all that's required.

And the Observable.zip call could be simplified. You could use a map operator to pair the file name and the stats.

Observable.bindNodeCallback(fs.readdir)(pathToFolder)
  .mergeMap((files) => {
    const array = files.map((fileName) => Observable
      .bindNodeCallback(fs.stat)(path.join(pathToFolder, fileName))
      .map((stats) => [fileName, stats])
    );
    return Observable.concat(...array);
  })
  .filter(([fileName, stats]) => stats.mtime.getTime() > lastSync.getTime())
  .subscribe(([fileName, stats]) => parseFile(fileName));

It's possible to further simplify the composed observable by taking advantage of the fact that the operators in the concact and merge families also support arrays. So, concatAll can be used to flatten the array, emitting the file names from the readdir callback. And those file names can then be mapped to stats with a concatMap operator:

Observable.bindNodeCallback(fs.readdir)(pathToFolder)
  .concatAll()
  .concatMap((fileName) => Observable
    .bindNodeCallback(fs.stat)(path.join(pathToFolder, fileName))
    .map((stats) => [fileName, stats])
  )
  .filter(([fileName, stats]) => stats.mtime.getTime() > lastSync.getTime())
  .subscribe(([fileName, stats]) => parseFile(fileName));


Determine local maximum from given points How could I make this code more effective?  I'm just experimenting and learning the different ways of how Python runs.

list=[]
original=mkPoint3D(x,y)
z1=mkPoint3D(x-d,y)
z2=mkPoint3D(x+d,y)
z3=mkPoint3D(x,y-d)
z4=mkPoint3D(x,y+d)
list.append(original)
list.append(z1)
list.append(z2)
list.append(z3)
list.append(z4)

if(maxPoint(list).z==original.z):
    print("The local maximum is at " + "( " + str(original.x) + " , "  + str(original.y) + " )" + " = " + str(original.z))
elif maxPoint(list).z==z1.z:
    hillClimb(z1.x,z1.y,d)
elif maxPoint(list).z==z2.z:
    hillClimb(z2.x,z2.y,d)
elif maxPoint(list).z==z3.z:
    hillClimb(z3.x,z3.y,d)
elif maxPoint(list).z==z4.z:
    hillClimb(z4.x,z4.y,d)

Don't use "list" as a variable name, since it is a python function.
Don't initialize it a list and append to it, if you can
just create it and populate it with one line.
Don't call a function more than once, if you don't need to.
Use a loop with a break statement, instead of all the elif statements

original=mkPoint3D(x,y)
z1=mkPoint3D(x-d,y)
z2=mkPoint3D(x+d,y)
z3=mkPoint3D(x,y-d)
z4=mkPoint3D(x,y+d)

point_list = [original, z1, z2, z3, z4]

maxz = maxPoint(point_list).z

if maxz == original.z:
    print (" The local ...")

else:
    for next_point in point_list[1:]:
        if maxz == next_point.z:
            matching_point = next_point
            break

    hillClimb(matching_point.x, matching_point.y, d)


Enforce exclusive order of four processes using semaphores I am on the learning curve of C and its capabilities. Below I have tried to create 4 individual programs where the execution is always in order (program 0,1,2,3).

About the program
Program #0 is always executed (called) first, it also creates semid's
Program #1 can ONLY be executed if Program #0 has ran
Program #2 can ONLY be executed if Program #1 has ran
Program #3 can ONLY be executed if Program #2 has ran

What Do I Need?
Since, I am a beginner I thought an expert's knowledge on creating a better design or code structuring would help me.

Program Call
./prog0 && ./prog1 && ./prog2 && ./prog3

Program #0

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/ipc.h>
#include <sys/sem.h>

int
main ( )
{
    int semid;
    key_t key;
    union semun arg;

    key = ftok("program0.c", 'a');
    semid = semget(key, 3, 0666 | IPC_CREAT);
    if ( semid < 0 )
    {
        perror("Could Not Created The Requested Semaphore");
        exit(1);
    }
    arg.val = 1;
    semctl(semid, 0, SETVAL, arg);
    arg.val = 0;
    semctl(semid, 1, SETVAL, arg);
    semctl(semid, 2, SETVAL, arg);
    printf("Executing process #0.\n");
    return 0;
}

Program #1

int
main ( )
{
    int semid, key;
    union semun arg;
    key = ftok("program0.c", 'a');
    semid = semget(key, 3, 0666 | IPC_CREAT);
    if ( semid < 0 )
    {
        perror("Could Not Created The Requested Semaphore");
        exit(1);
    }
    if ( semctl(semid, 0, GETVAL, 0) != 1 )
    {
        semctl(semid, 0, IPC_RMID);
        exit(1);
    }
    printf("Executing process #1.\n");
    arg.val = 0;
    semctl(semid, 0, SETVAL, arg);
    arg.val++;
    semctl(semid, 1, SETVAL, arg);
    return 0;
}

Program #2

int
main ( )
{
    int semid, key;
    union semun arg;
    key = ftok("program0.c", 'a');
    semid = semget(key, 3, 0666 | IPC_CREAT);
    if ( semid < 0 )
    {
        perror("Could Not Created The Requested Semaphore");
        exit(1);
    }
    if ( semctl(semid, 1, GETVAL, 0) != 1 )
    {
        semctl(semid, 0, IPC_RMID);
        exit(1);
    }
    printf("Executing process #2.\n");
    arg.val = 0;
    semctl(semid, 1, SETVAL, arg);
    arg.val++;
    semctl(semid, 2, SETVAL, arg);
    return 0;
}

Program #3

int
main ( )
{
    int semid, key;
    key = ftok("program0.c", 'a');
    semid = semget(key, 3, 0666 | IPC_CREAT);
    if ( semid < 0 )
    {
        perror("Could Not Created The Requested Semaphore");
        exit(1);
    }
    if ( semctl(semid, 2, GETVAL, 0) != 1 )
    {
        semctl(semid, 0, IPC_RMID);
        exit(1);
    }
    printf("Executing process #3.\n");
    semctl(semid, 0, IPC_RMID);
    return 0;
}

Programs exit instead of wait

Right now, your programs check their semaphore, but if they don't find the value they are expecting, they exit instead of wait.  I think that you should modify your programs to wait until the previous program signals them.  You would need to make the following modifications:

In program #0, initialize each semaphore to the value 1.
In each other program, use semop() to wait for its semaphore to reach value 0.  For example, program #1 waits for semaphore #0's value to become 0, using semop().
Modify each program to actually take a non-zero amount of time to do its work.  I suggest a for loop like this:

for (int i=0;i<10;i++) {
    printf("Executing process #1, iteration #%d.\n", i);
    sleep(1);
}

After each program's work loop, it releases its semaphore using semop().  This causes the semaphore value to transition from 1 to 0, which allows the next program to run.  For example, program #1 releases semaphore #1, causing program #2 to be able to run.
Program #3 cleans up instead of releasing a semaphore.


PHP - How can I refactor this store function (which handles different database operation for user types) in order to reduce duplication? I'm simply using Slim PHP MVC framework in my API project. PDO::FETCH_ASSOC is used in the database calls. So no data objects. The router handles requests and pass to the controller. controller calls the necessary model to do database operations and response is delivered.

This is the scenario that I want to handle different user types. Each user_type has user_type based specific db transaction to be done. I want to make this more reusable and remove code duplication.

Sorry if there's any wrong thing here, and for my English.

My Route

$this->post('/users/preferences', \UserPreferenceController::class . ':store')

UserPreferenceController => Controller

public function store($request, $response, $args){
    $parsedBody = $request->getParsedBody();
    $user_id = $parsedBody['user_id'];
    $preferences = $parsedBody['preferences'];

    $user = $this->User->getUserInfo($user_id);

    if ($user["type_id"] == 'Admin') {
        $preferenceAdded = $this->User->AddAdminUserPreferences($user_id, $preferences); // DB transaction operation
    } else if ($user["type_id"] == 'Customer') {
        $preferenceAdded = $this->User->AddCustomerUserPreferences($user_id, $preferences); // DB transaction Operation
    } else if ($user["type_id"] == 'Sales') {
        $preferenceAdded = $this->User->AddSalesUserPreferences($user_id, $preferences); // DB transaction Operation
    }

    // return if the transaction is rollbacked
    if(!$preferenceAdded){
        return $response->withJSON([
            "error" => true,
            "message" => "cannot add preferences for user"
            "data" => $user_id
        ]);
    }

    if ($user["type_id"] == 'Admin') {
        $staisticsAdded = $this->User->AddAdminUserPreferencesStatistics($user_id, $preferences); // DB transaction operation
    } else {
        $staisticsAdded = $this->User->AddOtherUserPreferencesStatistics($user_id, $preferences); // DB transaction operation
    }

    // return if the transaction is rollbacked
    if(!$staisticsAdded ){
        return $response->withJSON([
            "error" => true,
            "message" => "cannot add statistics for user"
            "data" => $user_id
        ]);
    }

    return $response->withJSON([
        "error" => false,
        "message" => "operation completed successfully"
        "data" => null
    ]);
}

User.php => User model

public function getUserInfo($user_id)
{
    $sql = "SELECT id, name, email, type_id FROM users WHERE id= :id";
    $stmt = $this->db->prepare($sql);
    $result = $stmt->execute(['id' => $user_id]);
    return $stmt->fetch()[0];
}

public function AddAdminUserPreferences($user_id, $preferences)
{
    $this->db->beginTransaction();

    try {
        // $sql1 execute (common function for every user_type)
        // rollback if $sql1 fails

        // $sql2 execute (a user_type specific function)
        // rollback if $sql2 fails

        $this->db->commit();
        return true;
    } catch(\PDOException $e){
        $this->db->rollBack();
        return false;
    }
}

My Container

$container = $app->getContainer();

$container['UserPreferenceController'] = function ($c) {
    return new App\Controllers\UserPreferenceController($c);
};

$container['User'] = function ($c) {
    return new App\Models\User($c);
};

I think it's a very good case to employ OOP, namely polymorphism and inheritance.

First, we've got to create a dedicated class to handle the user preferences.

A common ancestor to hold all the common code

abstract class UserPreferences
{
    protected $userId;
    protected $db;

    public function __construct($userId, $db) {
        $this->userId = $userId;
        $this->db = $db;
    }
    protected function addGeneric($preferences) {
        $sql = "INSERT INTO preferences (...) VALUES (?,?,?)";
        $this->db->prepare($sql)->execute($preferences);
    }

    abstract protected function addSpecific($preferences);

    public function add($preferences)
    {
        $this->db->beginTransaction();

        try {
            $this->addGeneric($preferences);
            $this->addSpecific($preferences);
            $this->db->commit();
        } catch(Throwable $e){
            $this->db->rollBack();
            throw $e;
        }
    }
}

and then implementations for different types

class AdminPreferences extends UserPreferences
{
    protected function addSpecific($preferences) {
        $sql = "INSERT INTO admin_preferences (...) VALUES (?,?,?)";
        $this->db->prepare($sql)->execute($preferences);
    }
}

class SalesPreferences extends UserPreferences
{
    protected function addSpecific($preferences) {
        $sql = "INSERT INTO sales_preferences (...) VALUES (?,?,?)";
        $this->db->prepare($sql)->execute($preferences);
    }
}

...and so on.

Then in the User class create an instance of the Preferences class

class User
{
    public $preferences;
    public function __construct($db, $type) {
        $this->userId = $userId;
        $this->db = $db;
        $this->preferences = $this->setPreferences($db, $type);
    }
    protected function setPreferences($db, $type) {
        switch($type) {
            case 'Admin':
                $this->preferences = new AdminPreferences($db);
                break;
            case 'Sales':
                $this->preferences = new SalesPreferences($db);
                break;
        }
    }
}

the same goes for the statistics

and finally in your controller simply

$preferences = $parsedBody['preferences'];
$this->User->preferences->add($preferences);
$this->User->preferencesStatistics->add($preferences);

return $response->withJSON([
    "error" => false,
    "message" => "operation completed successfully"
    "data" => null
]);

Note that checking $preferenceAdded manually is not the way to go. There should be a dedicated error handler to do the job.


Guessing a unique 4 random digits number I've created a simple game, in which the user needs to guess 4 digits number between 0-9, generated randomly using Random() Each of the 4 digits are different from each other, with no repeated digits.  The user has 5 attempts to guess, and each failed (almost correct) guess will tell the user in which part they got it correct.

Do you think this is too hard, or too easy for the user to guess?  I also need some review of this code.  Perhaps some of it can be simplified?

import java.util.Arrays;
import java.util.Random;
import java.util.Scanner;

public class Game {

    public static void main(String[] args) {
        System.out.println("The computer has generate a unique 4 digit number.\n"
                + "You can try to guess the 4 digits number in 5 attempts.\n");
        System.out.println("_______________________________________________________\n");
        int[] random=numberGenerator();
        int maxTry=5;
        int indexMatch=0;
        int match=0;
        while(maxTry>0 && indexMatch!=4){
            int[] guess=getGuess();
            indexMatch=0;
            match=0;
            for(int i=0;i<guess.length;i++){
                if(guess[i]==random[i]){
                    indexMatch++;
                }
                else if(guess[i]==random[0] || guess[i]==random[1] || guess[i]==random[2] || guess[i]==random[3]){
                    match++;
                }
            }
            if(indexMatch==4){
                System.out.print("Well done! Your guess is Correct! The number is: ");
                for(int i=0;i<guess.length;i++){
                    System.out.print(guess[i]);
                }
            }
            else{
                maxTry--;
                if(maxTry>1){
                    System.out.println("You have guess "+indexMatch+" correct number in correct position,"+
                    " and "+match+" correct number in incorrect position. \n"+maxTry+" attempt remaining.");
                }
                else if(maxTry==1){
                    System.out.println("You have guess "+indexMatch+" correct number in correct position,"+
                    " and "+match+" correct number in incorrect position. \nLast attempt!. Good luck");
                }
                else{
                    System.out.println("Sorry, you failed to guess the number in 5 attempts.");
                    System.out.print("The number is: ");
                    for(int i=0;i<random.length;i++){
                        System.out.print(random[i]);
                }
            }
        }
    }
}

public static int[] getGuess(){
    Scanner keyboard = new Scanner(System.in);
    System.out.println("Please enter your guess: ");
    String input = keyboard.nextLine();
        if(input.length()!=4 || input.replaceAll("\\D","").length()!=4){
            System.out.println("Invalid number. You must enter 4 digits between 0-9 only.");
            return getGuess();
    }
    int[] guess = new int[4];
    for (int i = 0; i < 4; i++) {
        guess[i] = Integer.parseInt(String.valueOf(input.charAt(i)));
    }
    return guess;
}

public static int[] numberGenerator() {
    Random randy = new Random();
    int[] randArray = {10,10,10,10};

    for(int i=0;i<randArray.length;i++){
        int temp = randy.nextInt(9);
        while(temp == randArray[0] || temp == randArray[1] || temp == randArray[2] || temp == randArray[3]){
            temp=randy.nextInt(9);
        }
        randArray[i]=temp;
    }
    return randArray;
}
}

As this part of the question has been skipped so far, I'll take it:

Do you think this is too hard?

I think it is. Similarly to your game, in classic Mastermind, the player has to guess a combination of 4 non-unique coloured pegs, and each time is given a mark for the number of correct position and colour combinations.

64 = 1,296 possible combinations

However, with your game (to borrow the nomenclature), there are 10 possible pegs, and as they're unique the combinations are:

10 x 9 x 8 x 7 = 5,040 possible combinations

Which is an increase of 288.8%, furthermore you're only giving the user 5 attempts to guess, as opposed to Mastermind's 6, 8 or 12. The reason Mastermind plays with a minimum of six, is that you get to test every combination, e.g:

However, when you increase the number of possibilities to 10, and reduce the guesses to 5, you can no longer do this, and the chances of never guessing a correct number increase dramatically, e.g.:

Therefore, I think you need to either increase the number of guesses to 10 (at the least), or chose a smaller set of things to guess, like A-F, 1-6, or something GUI based like Mastermind.


Share data between controllers with service I have to share data between controllers. In this example I'm using an aside. This has the exact same functionality as a modal from Angular Bootstrap.

I put the aside in my service so I can close it from another controller.

The problem is that I have to share the complete scope / object in the service so i can share the data between the controllers. In my real example, there are many lists in this object so it seems like it's a bit of overkill if I put the complete object in my service.

Is there a better way to do this?

I have to change the value from the scope on my page (= input field) but also in the aside. But the aside must have a different controller. Or can I use the same controller (MainController) for the aside? If I add the MainController to the aside, it instantiate the MainController again and the binding is lost. (See code example below)

Plunker

AsideService.initAside($aside.open({
    placement: 'right',
    templateUrl: 'PropertiesSidebar.html',
    size: 'lg',
    controller: 'MainController'
  }));

I think you're on the right track. Having a model object owned by a service and referenced from multiple scopes is good practice. Having more references to the same object in this was isn't a cost you should worry about.

However the example could be improved with clearer layout and more meaningful naming. I would suggest something like the following style.

angular.module('asideApp', ['ui.bootstrap', 'ngAside'])

  .service('AsideService', function ($timeout) {
    var model = {
      aside: null,
      text: null
    };

    function initAside(aside) { shared.aside = aside; }

    return {
      model: model,
      initAside: initAside
    };
  })

  .controller('MainController', function ($scope, $aside, AsideService) {
    $scope.shared = AsideService.model;

    $scope.openAside = function () {
      AsideService.initAside($aside.open({
          placement: 'right',
          templateUrl: 'PropertiesSidebar.html',
          size: 'lg',
          controller: 'AsideController'
        }));
    };
  })

  .controller('AsideController', function ($scope, AsideService) {
    $scope.shared = AsideService.model;
  });


Finding patterns in a string I'm trying to find patterns of different length in a given string. How can I improve it? Are there problems?

function find(pattern,str) {
   var  arr = [];
   for (i= 0;i<str.length;i++) {
      if(pattern== str.slice(i,pattern.length+i)) {
       arr.push([i,pattern.length-1+i]);
     };
  };
   if(!arr.length) { return false;} else { return arr;};
};

find('abfd','abfdffdabfdfaffab');

Here are two different ways of doing this, one using repeated calls to .indexOf() and one using repeated calls with a regex to .exec().  The regex solution requires the pattern argument to be a regex compatible string or an actual RegExp object with the g flag on it.

The .indexOf() option should be much faster than what you have since it avoids repeated .slice() operations and .indexOf() as a library function should be faster than your own variation.  The regex option is a lot more flexible since you can search for phrases that match a regular expression.

function find1(pattern,str) {
    var  arr = [];
    var i = 0;
    while ((i = str.indexOf(pattern, i)) > -1) {
        // got a match starting at i
        arr.push([i, i + pattern.length - 1]);
        i++;
    }
    return arr.length ? arr : false;
};

function find2(pattern,str) {
    var  arr = [], matches;
    if (typeof pattern === "string") {
     pattern = new RegExp(pattern, "g");
    } else if (!(pattern instanceof RegExp)) {
        throw new Error("pattern must be string or RegExp object");
    }

    while (matches = pattern.exec(str)) {
        arr.push([matches.index, matches.index + matches[0].length - 1]);
    }
    return arr.length ? arr : false;
};

document.getElementById("run1").addEventListener("click", function(e) {
    var results = find1('abfd','abfdffdabfdfaffab');
    log(results);
});

document.getElementById("run2").addEventListener("click", function(e) {
    var results = find2('abfd','abfdffdabfdfaffab');
    log(results);
});

function log(x) {
    var d = document.createElement("div");
    d.innerHTML = JSON.stringify(x) + "
";
    document.body.appendChild(d);
}
<button id="run1">Run .indexOf()</button>
<button id="run2">Run regex</button>


Pagination algorithm based on scores I need to divide scoreResult into pages (which is already ordered by score column from highest to lowest), and each page cannot contain too many records (e.g. 12 is a limitation in my example), and each page cannot contain duplicate id1, and each page needs to be ordered by score from highest to lowest.

Here is my working code; I am seeking advice, especially of smarter and more efficient solutions.

scoreResult = [
#id1,id2,score",
"1,28,300.1",
"4,5,209.1",
"20,7,208.1",
"23,8,207.1",
"16,10,206.1",
"1,16,205.1",
"1,31,204.6",
"6,29,204.1",
"7,20,203.1",
"8,21,202.1",
"2,18,201.1",
"2,30,200.1",
"15,27,109.1",
"10,13,108.1",
"11,26,107.1",
"12,9,106.1",
"13,1,105.1",
"22,17,104.1",
"1,2,103.1",
"28,24,102.1",
"18,14,11.1",
"6,25,10.1",
"19,15,9.1",
"3,19,8.1",
"3,11,7.1",
"27,12,6.1",
"1,3,5.1",
"25,4,4.1",
"5,6,3.1",
"29,22,2.1",
"30,23,1.1"
]

PageLimit = 12

def pageResult():

    #store final resutls
    pages = {}
    # store dictionary to see if page exists
    # key = host_id+#+page_id, value = item
    pageDict = {}
    # key: page ID, value: # of items
    pageCount = {}
    for item in scoreResult:
        page= 0
        host_id=item.split(',')[0]

        while pageDict.has_key(str(host_id)+'#'+str(page)) or (pageCount.has_key(page) and pageCount[page] >= PageLimit):
            #print page
            page += 1
        pageDict[str(host_id)+'#'+str(page)] = True
        if pages.has_key(page):
            pages[page].append(item)
        else:
            pages[page]=[]
            pages[page].append(item)

        if pageCount.has_key(page):
            pageCount[page] += 1
        else:
            pageCount[page] = 1

    for k,v in pages.items():
        print k
        print v

if __name__=="__main__":
    pageResult()

If you look at your code you have a lot of if pageCount.has_key(page): else:.
This can be replaced with a defaultdict.
pages defaults to a list, as if pages[page] does not exist you manually default it to a list.
And page_count defaults to zero.

As a simple example the following is how you could use default-dicts:

from collections import defaultdict

pages = defaultdict(list)
pages[0].append('Page 1')

I think changing page_dict to a set would be better, as they index the same way, but using a set will use less memory.

I would use a tuple as page_dict's key, rather than a string.
This is as (a, b) is easier to read than str(a) + "#" + str(b).

Python also has a style guide called PEP8, which basically means that your variables should be snake_case, and PageLimit should be UPPER_SNAKE_CASE as it's a constant.

Using the above should get you:

def page_result(score_result):
    pages = defaultdict(list)
    page_dict = set()
    page_count = defaultdict(int)
    for item in score_result:
        page = 0
        host_id = item.split(',')[0]

        while (host_id, page) in page_dict or page_count[page] >= PAGE_LIMIT:
            page += 1

        pages[page].append(item)
        page_dict.add((host_id, page))
        page_count[page] += 1

    for k, v in pages.items():
        print k
        print v

You could make another default-dict to store the latest page for a host_id.

This limits the amount of times the while loop runs. This dramatically reduces the amount of time it takes on larger data-sets. But has little diffrence on small data sets, and can perform slower!

I use the same data as given in the question, but timesed by 100.
scoreResult *= 100.

This gave me the following times, over 1000 tries:

Original: 225.99s
My original: 53.14s
My updatet: 2.97s
Mathias': 58.83s

This shows that there may be little speed-up from using a dictionary rather than len.
I also removed the prints.

def page_result(score_result):
    pages = defaultdict(list)
    page_dict = set()
    page_count = defaultdict(int)
    host_page = defaultdict(int)
    for item in score_result:
        host_id = item.split(',')[0]
        page = host_page[host_id]

        while page_count[page] >= PAGE_LIMIT:
            page += 1

        pages[page].append(item)
        page_dict.add((host_id, page))
        page_count[page] += 1
        host_page[host_id] = page + 1

    for k, v in pages.items():
        print k
        print v

I would also recommend:

Moving the print part of the function into it's own function.
Read the data from a file. Saving it as a JSON file and using Python's JSON library is a simple way to do this.


Sort three input values by order This is a program to sort three inputted integers in order, from least to greatest. I would like to have it reviewed.

#include <iostream>

void Sort(int &a, int &b, int &c){
    if(a>b){
        int tmp = a;
        a = b;
        b = tmp;
    }
    if(a>c){
        int tmp = a;
        a=c;
        c = tmp;
    }
    if(b>c){
        int tmp = b;
        b=c;
        c=tmp;
    }
    return;
}

int main(){
    std::cout << "Enter three integers: " << std::endl;
    int num1;
    int num2;
    int num3;
    std::cin >> num1 >> num2 >> num3;

    int output1 = num1;
    int output2 = num2;
    int output3 = num3;

    Sort(output1,output2,output3);

    std::cout << num1 << " " << num2 << " " << num3 << " "
         << " in sorted order: ";
    std::cout << output1 << " " << output2 << " " << output3 << std::endl;

    return 0;
}

Things you should change:

Sort() is explicitly returning at the end (return;). The return statement is implicit once you reach the end of a void function, so should not appear.
Pay attention to the spacing between the = sign. In some places you have b=c; while in others you have a = b; Be consistent with the spacing. I suggest the latter, as it seems more readable to me.
You don't have to return 0 from main. For main(), the return zero is implicit if not added.

This is also a great opportunity for you to learn about C++ templates. I will suggest you a base implementation that you can study, test, and better understand the awesomeness of this language feature:

template<typename T>
void swap_if_greater(T& a, T& b)
{
    if (a > b)
    {
        T tmp(a);
        a = b;
        b = tmp;
    }
}

template<typename T>
void sort(T& a, T& b, T& c)
{
    swap_if_greater(a, b);
    swap_if_greater(a, c);
    swap_if_greater(b, c);
}

Just as in the example provided by @EngieOP, I've added a swap function to reuse more code.
I've also better separated common code by moving the conditional inside the swap function (swap_if_greater()), since it is common to all inputs.

sort() is now a template function, meaning is can operate on any type, including int, float, double, char, ... It is a generic function, just as many of the functions in the standard C++ library. This concept promotes great code reuse and low redundancy of functionality.


Implementation of cstring wrapper class I want to have a thin wrapper (low overhead) around c-style string. Also, with some additional features like == and < operators for map indexing. I have come up with the following class.

Can somebody comment on it? From what I've gathered this class would take only as much memory as a pointer, so much less than std::string.

I also heard that this class has more overhead (execution-wise) than std::string and indeed that is the case for the constructor (char *), copy constructor and move constructor. Can somebody explain why?

inline char * dupStr(const char * s){
    if(!s){return nullptr;}
    size_t l = strlen(s) + 1;
    char * d = new char[l];
    memcpy(d,s,l);
    return d;
}

class CString_{
    private:
        const char *s_;

    public:
        CString_(const char *s):s_(dupStr(s)){}
        CString_(const std::string &s):CString_(s.c_str()){}
        CString_(const CString_ &that):s_(dupStr(that.s_)){}
        CString_(CString_ &&that):s_(that.s_){
            that.s_ = nullptr;
        }

        CString_ & operator= (const CString_ & that) = delete;
        CString_ & operator= (CString_ && that) = delete;

        ~CString_(){ delete[] s_;}

        const char * get() const {return s_;}

        bool operator<(const CString_ &that) const{
            return strcmp(s_,that.s_) < 0;
        }

        bool operator==(const CString_ &that) const{
            return strcmp(s_,that.s_) == 0;
        }
};

Answers to Questions

  I want to have a thin wrapper (low overhead) around c-style string.

I am not sure this class is 'low-overhead' compared to std::string or even a C-String. Because it still has all the overhead of creating dynamic space and copying.

Low-overhead: To me this means the cost of creating the object. If you use this as you measurement then you have not achieved your goal. The cost is about the same as both a std::string and C-String.

Now. It has memory management abilities of std::string but has a smaller footprint. It is the same size as a C-String but the advantage of memory management.

So it has some extra advantages (just not low overhead).

  Also, with some additional features like == and < operators for map indexing.

Which is useful; like it.

  Can somebody comment on it?

Not a problem.

  From what I've gathered this class would take only as much memory as a pointer.

Probably.
But you can always test this by using sizeof() to find out for sure.

  so much less than std::string.

It's probably about one third the size of a std::string (you can't make a definitive statement because the standard does not specify how to implement it).

Now a third sounds like a lot. But if a pointer is 4 bytes then a std::string would probably be 12 bytes. Not a great saving. Especially since the std::string has several other advantages (small string optimization: if the string is less than 12 bytes there is no dynamic memory allocation).

  I also heard that this class has more overhead (execution-wise) than std::string and indeed that is the case for the constructor (char *), copy constructor and move constructor. Can somebody explain why?

I don't believe that. Your code is pretty optimal in those terms.

Review of Code

Not sure why this is a standalone function? Why not make it a private member of the class.

inline char * dupStr(const char * s){
    if(!s){return nullptr;}           // Sure. Why save the space
                                      //       Do this on multiple lines.
                                      //       The point is to make the code
                                      //       readable.
    size_t l = strlen(s) + 1;
    char * d = new char[l];
    memcpy(d,s,l);
    return d;
}

The move constructor. You should mark the move constructor as noexcept:

        CString_(CString_ &&that) noexcept
            : s_(that.s_)
        {
            that.s_ = nullptr;
        }

This is because the standard containers require this to do an optimal operations. If you don't specify this then they will fall back to using the copy constructor (note: only mark it as noexcept if it really is).

Sure you can do a get:

        const char * get() const {return s_;}

But it is sometimes also useful to define the cast operator:

        const char * operator() const {return get();}

This is useful so you can use CString_ in any location that you could use a C-String without having to change the code.

Low-Overhead.

If I was thinking low-overhead. Then I would make the class take ownership of the string object. That way there is no dynamic allocation or copying. But you get the advantages of RAII (so automatic destruction).

In addition I would get store the size. One of the largest cost of a C-String is repeatedly calculating the size of the string (we switched some old code from using C-String to std::string (a big job)). But we got a 40% speed increase.

The problem is how the C-String was created (malloc() or new). We can make the assumption that the user will only use string that come from C libraries. The problems are that you can't check and make that guarantee (one of the reasons std::string makes a copy).

 class CStringOwner
 {
      char*        data;
      std::size_t  size;
      public:
          CStringOwner(std::nullptr_t)
              : data(nullptr)
              , size(0)
          {}
          explicit CStringOwner(char* data)
              : data(data)
              , size(data ? strlen(data) : 0) // make sure null ptrs are correctly handled.
          {}

          // Copy.
          CStringOwner(CStringOwner const& copy)
              : data(reinterpret_cast<char*>(malloc(copy.size+1)))
              , size(copy.size)
          {
              if (data == nullptr) {
                  throw std::bad_alloc("CStringOwner::CStringOwner: Copy: Failed on Malloc");
              }
              std::copy(copy.data, copy.data+copy.size+1, data);
          }
          CStringOwner& operator=(CStringOwner copy)
          {
              copy.swap(*this);
              return *this;
          }

          // Move
          CStringOwner(CStringOwner&& move) noexcept
              : data(nullptr)
              , size(0)
          {
              move.swap(*this);
          }
          CStringOwner& operator=(CStringOwner&& move) noexcept
          {
              move.swap(*this);
              return *this;
          }

          // Destroy
          ~CStringOwner()
          {
              free(data);
          }
          void swap(CStringOwner& other) noexcept
          {
              using std::swap;
              swap(data, other.data);
              swap(size, other.size);
          }

          // Other shit goes here.
  };


Spotify API data to vk.com user's status Just a simple script that puts a user's currently playing track(on Spotify) to his account's status on vk.com. And printing info about the track in the console.

import requests
import os
import json

URL_TRACK = 'https://api.spotify.com/v1/me/player/currently-playing'
URL_STATUS = "https://api.vk.com/method/status.set"
VK_TOKEN = os.environ.get('VK_TOKEN')
SP_TOKEN = os.environ.get('SP_TOKEN')

def set_status():
    params = {
              'user_id': 8573490,
              'v': 5.92,
              'access_token': VK_TOKEN,
              'text': current_track()
    }

    status = requests.get(url=URL_STATUS, params=params)

def track_data():
    headers = {
               'Accept': 'application/json',
               'Content-Type': 'application/json',
               'Authorization': f'Bearer {SP_TOKEN}',
    }

    return requests.get(url=URL_TRACK, headers=headers)

def track_is_playing():
    return track_data().status_code == 200

def current_track():
    data = track_data().json()
    artist_path = data["item"]['artists'][0]['name']
    track_path = data["item"]['name']

    return(f'{artist_path} - {track_path}')

def run_script():
    if track_is_playing():
        set_status()
        print(current_track())
    else:
        print('Not playing')

if __name__ == "__main__":
    run_script()

Rather than os.environ.get you can do getenv.

In track_data, don't pass Content-Type, because you aren't passing any body with your request.

Otherwise, it's quite reasonable. You may want to expand error checking and reporting, particularly for track_data, since you aren't currently checking the status. raise_for_status is often a good thing to do.


Method that compares the scores of two players I tackled this problem today and was wondering how I could possibly improve this code.
The objective of the method is to return an array with the respective scores of each player. The method takes three values from 2 users. a0,a1,a2 are all scores from player a. b0,b1,b2 are all scores of player b.

A player is awarded a point if their score of the same category is bigger than the score of the player.
For example to determine who got a better first score we must compare a0 with b0 and so on forth.

Here is my code:

static int[] solve(int a0, int a1, int a2, int b0, int b1, int b2){
        int p1 = 0;
        int p2 = 0;
        if(a0 > b0){
            p1++;
        }else if(a0 < b0){
            p2++;
        }

        if(a1 > b1){
            p1++;
        }else if(a1 < b1){
            p2++;
        }

        if(a2 > b2){
            p1++;
        }else if(a2 < b2){
            p2++;
        }

        return new int[]{p1,p2};

    }

As Timothy mentioned, your naming of the variables are not so good. I would recommend you have a look at Java Naming Conventions. It will be helpful for writing easily readable codes.

Secondly, try using private for your method. I know it is not a big program, but you will get used to it.

Thirdly, the beauty of the code is in shortness and effectiveness. And here I took Timothy's code and wrote it with for loop:

int[] scoresOfPlayerA = new int[]{a0,a1,a2};
int[] scoresOfPlayerB = new int[]{b0,b1,b2};

for (int i = 0; i < scoresOfPlayerA.length; i++){
    if (scoresOfPlayerA[i] > scoresOfPlayerB[i]){
        finalScoreOfA++;
    } else if (scoresOfPlayerA[i] < scoresOfPlayerB[i]) {
        finalScoreOfB++;
    }
}

return new int[]{finalScoreOfA, finalScoreOfB};

Good luck with your future codes.


Text-to-Morse code converter I created a morse converter from a .txt file.

I'm learning java and I created a simple morse converter. Convert only A to Z and 0 to 9 chars. Space char remain for split words in morse-code.
At the beginning I used a big switch for any letters or number but it was no good.

public static String toMorse(byte[] bytes) {
    String text = new String(bytes);
    String morseText = "";
    text = text.toLowerCase();
    String[] morseChar
            = {".-",
                "-...",
                "-.-.",
                "-..",
                ".",
                "..-.",
                "--.",
                "....",
                "..",
                ".---",
                "-.-",
                ".-..",
                "--",
                "-.",
                "---",
                ".--.",
                "--.-",
                ".-.",
                "...",
                "-",
                "..-",
                "...-",
                ".--",
                "-..-",
                "-.--",
                "--.."};
    String[] morseNumber
            = {".----",
                "..---",
                "...--",
                "....-",
                ".....",
                "-....",
                "--...",
                "---..",
                "----.",
                "-----"};

    for (int i = 0; i < text.length(); i++) {
        if (text.charAt(i) >= 'a' && text.charAt(i) <= 'z') {
            morseText = morseText + morseChar[text.charAt(i) - 'a'];
        } else if (text.charAt(i) >= '0' && text.charAt(i) <= '9') {
            morseText = morseText + morseNumber[text.charAt(i) - '0'];
        } else if (text.charAt(i) == ' ') {
            morseText = morseText + text.charAt(i);
        }
    }
    return morseText;
}

One more review, where I'd like to point out two Java pitfalls you should avoid in professional code.

byte[] array to represent text

Your public static String toMorse(byte[] bytes) { ... } method signature gets the input as byte array and only internally creates a String out of that array.

That's dangerous, as that String(byte[] bytes) constructor is documented as (emphasis mine):

  public String(byte[] bytes)

  Constructs a new String by decoding the specified array of bytes using
  the platform's default charset. [...]

That means that the same byte array will produce different results when running under different operating systems, as even nowadays there are still many different character encodings in use all over the world and all over the various operating systems.

A computational method like your morse converter should have a String argument instead of a byte[] array, then it's run-anywhere reproducible. If necessary, have your file I/O methods handle character encodings.

Rule of thumb: to pass around things that represent text, use String.

Repeated String appending

In your loop, you build the output string by repeatedly doing lines like

morseText = morseText + morseChar[text.charAt(i) - 'a'];

That's ok if you know the text is rather short, but it will become quite slow for strings with thousands of characters.

Why? Every time the machine executes such a line, it creates a completely new morseText string, copying all the characters from the old morseText, plus the additional morse snippet. So, the first morse snippets get copied over and over and over again. To avoid that, use a single StringBuilder, and in every loop iteration, append() the morse snippet to that StringBuilder, and only after the loop, convert the StringBuilder to the String you want to return.


Merge Two Sorted Lists by splicing together the nodes of the first two lists I solved this problem:

  Merge two sorted linked lists and return it as a new list. The new list should be made by splicing together the nodes of the first two lists.

How can I improve its execution time and also, if there is any better way of doing it?

public ListNode mergeTwoLists(ListNode listNode1, ListNode listNode2) {
    ListNode headNode = new ListNode(0);
    ListNode node = headNode;
    while (null != listNode1 && null != listNode2) {
        if (listNode1.val < listNode2.val) {
            node.next = listNode1;
            listNode1 = listNode1.next;
        } else {
            node.next = listNode2;
            listNode2 = listNode2.next;
        }
        node = node.next;
    }
    if(null != listNode1) {
        node.next = listNode1;
    }
    if(null != listNode2) {
        node.next = listNode2;
    }
    return headNode.next;
}

LGTM.

Still a couple of notes:

A null in a conditional context evaluates to false. It is safe to omit an explicit comparison to null, along the lines of

    while (listNode1 && listNode2) {

Ditto for if (listNode1) and if (listNode2).
Mandatory stability loss notice. The if (listNode1.val < listNode2.val) test loses stability: when the values compare equal, an element from the second list is merged first.

The stability of merge is not required by the problem statement, and doesn't matter for integers. Just be aware.


C++ template Partial Specialization Member Function; Am I doing right? I'm trying to improve my C++ template metaprogramming skills. From my understanding, I can't partially specialize a function(either member function or not). So, I need to define another class that can be used as template class, which enables us partially specialize template argument.
So, In order to check my understanding, I've wrote a simple code. It uses general template class and partially specialized template class to deal with various types. (I've tested this code using gcc 4.8 / clang 3.4 / MSVC 2015) It would give a result as follows:

Generic
Specialization for std::string
Specialization for bool
Specialization for integral type

Am I doing write using class templates? I need a review for this. Any comment would be helpful.
// a.hpp
#ifndef A_HPP
#define A_HPP
#include <iostream>
#include <string>
#include <type_traits>

// declaration & implementation for bar_impl::bar
template <typename T, typename T1, typename T2, typename T3>
struct bar_impl
{
    static void bar(const T& input)
     {
        (void)input;
        std::cout << "Generic" << std::endl;
    }
};

template <typename T>
struct bar_impl<T, std::false_type, std::true_type, std::false_type>
{
    static void bar(const T& input)
    {
        (void)input;
        std::cout << "Specialization for integral type" << std::endl;
    }
};

template <typename T>
struct bar_impl<T, std::false_type, std::true_type, std::true_type>
{
    static void bar(const T& input)
    {
        (void)input;
        std::cout << "Specialization for bool" << std::endl;
    }
};

template <typename T, typename T1, typename T2>
struct bar_impl<T, std::true_type, T1, T2>
{
    static void bar(const T& input)
    {
        (void)input;
        std::cout << "Specialization for std::string" << std::endl;
    }
};

struct Foo
{
    template <typename T>
    void bar(const T& input);
};

template <typename T>
void Foo::bar(const T& input)
{
    bar_impl<
        T,
        typename std::is_same<T, std::string>::type,
        typename std::is_integral<T>::type,
        typename std::is_same<T, bool>::type
    >::bar(input);
}

#endif // A_HPP

// b.cpp
#include "a.hpp"

int main()
{
  Foo foo;

  foo.bar(Foo());
  foo.bar(std::string());
  foo.bar(bool());
  foo.bar(int());
  return 0;
}

What you've written does work, and you seem to understand all the concepts (e.g., "you can't partially specialize function templates"), and your coding style (indentation and whatnot) seems fine.

Well, one coding-style improvement: You need those (void)input casts to shut up compiler warnings about unused parameters... in C. But in C++ you don't need those casts; what you do instead is, if you're not going to use a parameter, just don't give it a name.

static void bar(const T&) { ... }

However, your code doesn't really match any common TMP patterns — which is probably just because you're noodling around, learning, instead of trying to solve a specific problem.

If you just wanted to produce the same output with less code, you would do this:

struct Foo
{
    template<class T> auto bar(const T&) -> std::enable_if_t<!std::is_integral<T>::value>
    {
        std::cout << "Generic" << std::endl;
    }

    template<class T> auto bar(const T&) -> std::enable_if_t<std::is_integral<T>::value>
    {
        std::cout << "Specialization for integral type" << std::endl;
    }

    void bar(bool)
    {
        std::cout << "Specialization for bool" << std::endl;
    }

    void bar(const std::string&)
    {
        std::cout << "Specialization for std::string" << std::endl;
    }
};

int main()
{
    Foo foo;
    foo.bar(Foo());
    foo.bar(std::string());
    foo.bar(bool());
    foo.bar(int());
    return 0;
}

That is, your code is equivalent to providing two overloads of foo.bar, plus a couple of mutually exclusive templates (one for integral types and one for other types). The specific overloads for bool and std::string will take precedence over the templates.

Alternatively, you could use tag dispatch, as follows. (But I don't recommend it in this case because it turns out to be surprisingly subtle. Replace the second instance of std::true_type with ... and watch that overload silently drop out. I'm not actually sure why.)

struct Foo
{
    template<class T> void bar_impl(const T&, ...) {
        std::cout << "Generic" << std::endl;
    }

    template<class T> void bar_impl(const T&, std::true_type) {
        std::cout << "Specialization for integral type" << std::endl;
    }

    void bar_impl(const std::string&, ...) {
        std::cout << "Specialization for std::string" << std::endl;
    }

    void bar_impl(const bool&, std::true_type) {
        std::cout << "Specialization for bool" << std::endl;
    }

    template<class T> void bar(const T& input)
    {
        bar_impl(input, std::is_integral<T>{});
    }
};

Maybe the best approach would be to use two non-interacting levels of tag dispatch: first, is T integral? and second, what is T exactly?

template<bool IsIntegral> struct BarImpl {
    template<class T> static void bar(const T&) {
        std::cout << "Generic" << std::endl;
    }
    static void bar(const std::string&) {
        std::cout << "Specialization for std::string" << std::endl;
    }
};

template<> struct BarImpl<true> {
    template<class T> static void bar(const T&) {
        std::cout << "Specialization for integral type" << std::endl;
    }
    static void bar(bool) {
        std::cout << "Specialization for bool" << std::endl;
    }
};

struct Foo {
    template<class T> void bar(const T& input) {
        BarImpl<std::is_integral<T>::value>::bar(input);
    }
};


Update the value of a user based on the click of a checkbox In an admin view on an intranet I maintain, there is a table of users present that allows admins to validate or delete users. Right now, the way the user is validated (or how they have their validation revoked) is by clicking a checkbox (i.e. a check in the checkbox signifies the user is validated, but no check means the user is not validated).

And right now I have an if/else statement that looks to see if the box has just been checked (or unchecked) and then sends the data to the server to achieve the desired outcome.

The code is not DRY at all, because the only difference in the code from the if/else statement is the state of the checkbox. How can I make the following code more DRY?

const chkBox = document.querySelectorAll("input[type=checkbox]");

for (box of chkBox) {
    box.addEventListener("change", updateUser, false);
}

function updateUser() {

    const messageBox = document.getElementById("message");

    if (this.checked) {

    // Getting the values to post to the server and the initialization requirements
        const postingValues = JSON.stringify({id:this.value, validated:true});
        const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

    // Making the fetch call and returning the message to the user
        fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
            response.text().then(function(value) {
                messageBox.innerHTML = value;
            });
        });
    } else {

    // Getting the values to post to the server and the initialization requirements
        const postingValues = JSON.stringify({id:this.value, validated:false});
        const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

    // Making the fetch call and returning the message to the user
        fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
            response.text().then(function(value) {
                messageBox.innerHTML = value;
            });
        });
    }
}

So chalk this one up to me not giving it enough thought (although I've had this in the back of my mind for about two weeks and today just decided to use my Saturday to actually sit down and refactor the entire application):
The answer is that I put the state of the checkbox in a variable and I don't need to check for it at all, I can just pass it through into my postingValues object. The code now looks like this:

const chkBox = document.querySelectorAll("input[type=checkbox]");

for (box of chkBox) {
    box.addEventListener("change", updateUser, false);
}

function updateUser() {

// Getting the state of the checkbox and the DOM element to display the message
    const messageBox = document.getElementById("message");
    const validated = this.checked;

// Getting the values to post to the server and the initialization requirements
    const postingValues = JSON.stringify({id:this.value, validated:validated});
    const fetchInit = {method: "POST", headers: {"Content-type": "application/json"}, body: postingValues};

// Making the fetch call and returning the message to the user
    fetch("index.php?action=updateLogin", fetchInit).then(function(response) {
        response.text().then(function(value) {
            messageBox.innerHTML = value;
        });
    });
}


Building a string representing 50+ ActiveDirectory options I have a switch statement used to handle options that a user selected (stored in an array). The array is iterated through, and for each element (enum object) in the array, an appropriate action is performed. However, the enum has over 50 items, so the switch statement is very long.

The items are all necessary and related, so they cannot be moved or removed. They also each call a method or get a property of an object, so no dictionaries. Are there any other options to make this shorter?

public string GetUserAttributes(UserPrincipal user,
    ActiveDirectoryUserAttribute[] attributes)
{
    var userAsDirectoryEntry
        = (DirectoryEntry)user.GetUnderlyingObject();
    var userAttributes = string.Empty;
    foreach (ActiveDirectoryUserAttribute attribute in attributes)
    {
        switch (attribute)
        {
            case ActiveDirectoryUserAttribute.AccountExpirationDate:
                userAttributes += AttributeToString(
                    user.AccountExpirationDate.ToString());
                break;
            case ActiveDirectoryUserAttribute.AccountLockoutTime:
                userAttributes += AttributeToString(
                    user.AccountLockoutTime.ToString());
                break;
            case ActiveDirectoryUserAttribute.Assistant:
                userAttributes += AttributeToString(
                    userAsDirectoryEntry.Properties[AttributeAssistant]
                    .Value.ToString());
                break;
            case ActiveDirectoryUserAttribute.BadLogonCount:
                userAttributes += AttributeToString(
                    user.BadLogonCount.ToString());
                break;
            ...
        }
    }
    return userAttributes;
}

You can put the switch-cases into a dictionary to reduce the boilerplate case and break. The foreach and the string concatenation part can also be reduced with string.Concat(...) or string.Join(delimiter, ...):

public string GetUserAttributes(UserPrincipal user, ActiveDirectoryUserAttribute[] attributes)
{
    var userAsDirectoryEntry = (DirectoryEntry)user.GetUnderlyingObject();

    var getters = new Dictionary<ActiveDirectoryUserAttribute, Func<object>>();
    getters[ActiveDirectoryUserAttribute.AccountExpirationDate] = () => user.AccountExpirationDate;
    getters[ActiveDirectoryUserAttribute.AccountLockoutTime] = () => user.AccountLockoutTime;
    getters[ActiveDirectoryUserAttribute.Assistant] = () => userAsDirectoryEntry.Properties[AttributeAssistant].Value;
    getters[ActiveDirectoryUserAttribute.BadLogonCount] = () => user.BadLogonCount;
    // ...

    // just concat them directly, like you are doing now
    return string.Concat(attributes.Select(attr => getters[attr]()));

    // or, you can separate them with a comma, or anything :
    return string.Join(", ", attributes
        .Select(attr => getters[attr]()));
}

You can also use the collection initializer for dictionary to cut down some letters :

var getters = new Dictionary<ActiveDirectoryUserAttribute, Func<object>>
{
    // C# 6
    [ActiveDirectoryUserAttribute.AccountExpirationDate] = () => user.AccountExpirationDate,
    [ActiveDirectoryUserAttribute.AccountLockoutTime] = () => user.AccountLockoutTime,

    // older version
    { ActiveDirectoryUserAttribute.Assistant, () => userAsDirectoryEntry.Properties[AttributeAssistant].Value },
    { ActiveDirectoryUserAttribute.BadLogonCount, () => user.BadLogonCount },

    // ...
};

Personally I prefer the C# 6 collection initializer the most, second by the method shown in first snippet, and lastly the older initializer.


How to improve this session cache to search faster I have this code which will store user session credentials:

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

public class SessionCache
{

    /**
     * Default Constructor
     */
    public SessionCache()
    {
    }

    /**
     * Map which which will store Connections Objects by connection_id
     */
    private static final Map<String, ActiveConnections> cache = new LinkedHashMap<>();

    /**
     * Object which will store connection credentials
     */
    public class ActiveConnections
    {

        private int one;
        private int two;
        private int three;

        public ActiveConnections(int one, int two, int three)
        {
            this.one = one;
            this.two = two;
            this.three = three;
        }

        public int getOne()
        {
            return one;
        }

        public void setOne(int one)
        {
            this.one = one;
        }

        public int getTwo()
        {
            return two;
        }

        public void setTwo(int two)
        {
            this.two = two;
        }

        public int getThree()
        {
            return three;
        }

        public void setThree(int three)
        {
            this.three = three;
        }

    }

    /**
     * Get Connection credentials by connection_id
     *
     * @param connection_id
     * @return
     */
    public Object getCache(String connection_id)
    {
        return cache.get(connection_id);
    }

    /**
     * Insert new Connection Object into the cache
     *
     * @param connection_id
     * @param one
     * @param two
     * @param three
     */
    public void addCache(String connection_id, int one, int two, int three)
    {
        /**
         * @ You may want to check if entry already exists for connection depends on logic in your application.
         * @ Otherwise this will replace any previous entry for connection_id
         */
        if (!cache.containsKey(connection_id))
        {
            cache.put(connection_id, new ActiveConnections(one, two, three));
        }

    }

    /**
     * Remove Connection from the cache based on connection_id
     *
     * @param connection_id
     */
    public void removeCache(String connection_id)
    {
        cache.remove(connection_id);
    }

    /**
     * Get the number of all connections
     *
     * @return
     */
    public int countCacheSize()
    {
        return cache.size();
    }

    /**
     * Clear all cache
     */
    public void flushCache()
    {
        cache.clear();
    }

    public List<String> sendAllKeys()
    {
        List<String> list = new ArrayList<>(cache.keySet());
        return list;
    }

    public Object sendKeyValue(int key)
    {
        Object mapKey = cache.keySet().toArray()[key];
        return mapKey;
    }

}

Can you help me to improve this code?
I would like to make a while cycle which searches for a value into the Map every 5 seconds for example expired session. Could you please help me to improve the code and make it lightweight as much as possible.

First you should rename your instance members in ActiveConnections class - having methods and variables named getOne(), getTwo(), etc, are horrible because at first you'd expect them to return the int value of one, two, three (e.g., returning 1, 2, 3) but in reality they most likely won't.
You should either put a lock on your cache or use a ConcurrentMap or you will sooner or later experience race condition errors.
Your send methods should be renamed to get because you are not sending them anywhere, you are returning them.
You should consider setting a memory/element limit on your cache; ex. it should only hold 101 cached items and when this limit is reached you apply a Least Recently Used policy and remove the X amount of LRU cache items.
Your getCache(..) and addCache(..) should be renamed because you are not getting nor adding a cache, but rather a cache item.
Your getCache(..) should have ActiveConnection as the return type

If you want a kind of Sliding Expiration policy where you remove items that are older than 5 seconds there are a few things to keep in mind:

5 seconds are a very short period of time; and
You need a way to map a timestamp to a cache item. A very simple way of doing this is making a nested class, e.g., CacheItem, which only holds a timestamp and a value and your map will consist of a <string key, CacheItem value>

edit

Here is an example of a very simple cache class I wrote in C# (just for reference). For the test cases I've ran it through it works fine, but you might want to check into existing cache libraries.

public class SimpleCache
{
    public const int MAX_CACHE_SIZE = 101;
    public long CacheExpirationTicks { get; set; }
    private ConcurrentDictionary<string, CacheItem> _cache = new ConcurrentDictionary<string, CacheItem>();

    public SimpleCache() { CacheExpirationTicks = 30000; /*default*/ }

    public void Add(string key, object value)
    {
        //if the cache limit is reached we apply a LRU policy on the cache and remove half of the cache ( MAX_CACHE_SIZE / 2 ) least recently used
        if (_cache.Count == MAX_CACHE_SIZE)
        {
            foreach (var cacheItem in _cache
                .OrderByDescending(x => x.Value.LastAccessed)
                .Skip(MAX_CACHE_SIZE / 2))
            {
                CacheItem ignored;
                _cache.TryRemove(cacheItem.Key, out ignored);
            }

        }
        _cache.TryAdd(key, new CacheItem() { Value = value, LastAccessed = DateTime.Now.Ticks });
    }

    public object this[string key]
    {
        get
        {
            if (_cache.ContainsKey(key))
            {
                if ((_cache[key].LastAccessed + CacheExpirationTicks) > DateTime.Now.Ticks)
                {
                    return _cache[key].Value;
                }
                CacheItem ignored;
                _cache.TryRemove(key, out ignored);
            }
            return null;
        }
    }

    internal sealed class CacheItem
    {
        //despite of it's name, LastAccess will *AND SHOULD* only be set once. The cache will validate its item in the get and add function and remove them by itself
        public long LastAccessed { get; set; }
        public object Value { get; set; }
    }
}

And you would implement it like this

public class Program
{
    public static SimpleCache simpleCache = new SimpleCache();

    static void Main(string[] args)
    {
        DataTable dt = GetDataTableFromCSVFile("myCSV.csv");
    }

    private static DataTable GetDataTableFromCSVFile(string csv_file_path)
    {
        object obj;
        if ((obj = simpleCache[csv_file_path]) != null)
        {
            return (DataTable)obj;
        }
        else
        {
            var csvData = ExpensiveOperation();
            var key = csv_file_path;

            simpleCache.Add(key, csvData);

            return csvData;
        }
    }
}


Array is Twinoid Or Not A twinoid is defined to be an array that has exactly two even values that are adjacent to one another. For example {3, 3, 2, 6, 7} is a twinoid array because it has exactly two even values (2 and 6) and they are adjacent to one another. The following arrays are not twinoid arrays.

{3, 3, 2, 6, 6, 7} because it has three even values.
{3, 3, 2, 7, 6, 7} because the even values are not adjacent to one another.
{3, 8, 5, 7, 3} because it has only one even value.

I wrote Twinoid to check that given array is twinoid or not.

public class Twinoid {

public static void main(String args[]) {
    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 6, 7}));
}

public static boolean isTwinoid(int[] a) {
    boolean status = false;
    int i;
    int count = 0;
    for (i = 0; i < a.length; i++) {
        if (a[i] % 2 == 0) {
            count++;
        }
    }
    if (count == 2) {
        int j;
        for (j = 0; j < a.length; j++) {
            if (a[j] % 2 == 0) {
                System.out.println(a[j]);
                break;
            }
        }
        int nextValue = a[j + 1];
        System.out.println(nextValue);
        if (nextValue % 2 == 0) {
            status = true;
        }
    } else {
        status = false;
    }
    return status;
}
}

You don't need two passes.

    boolean status = false;
    int i;
    int count = 0;
    for (i = 0; i < a.length; i++) {
        if (a[i] % 2 == 0) {
            count++;
        }
    }
    if (count == 2) {
        int j;
        for (j = 0; j < a.length; j++) {
            if (a[j] % 2 == 0) {
                System.out.println(a[j]);
                break;
            }
        }
        int nextValue = a[j + 1];
        System.out.println(nextValue);
        if (nextValue % 2 == 0) {
            status = true;
        }
    } else {
        status = false;
    }
    return status;

You can replace this with just

    int i;
    for (i = 0; i < a.length - 1; i++) {
        if (a[i] % 2 == 0) {
            i++;
            if (a[i] % 2 == 0) {
                i++;
                break;
            } else {
                return false;
            }
        }
    }

    if (i == a.length && a[i-1] % 2 != 0) {
        return false;
    }

    for (; i < a.length; i++) {
        if (a[i] % 2 == 0) {
            return false;
        }
    }

    return true;

or even shorter

    for (int i = 0; i < a.length - 1; i++) {
        if (a[i] % 2 == 0) {
            i++;
            if (a[i] % 2 == 0) {
                i++;
                for (; i < a.length; i++) {
                    if (a[i] % 2 == 0) {
                        return false;
                    }
                }

                return true;
            } else {
                return false;
            }
        }
    }

    return false;

Either of these only requires a single scan.

If they find no even numbers in the first a.length - 1 elements, they fall through and return false.

If they find an even number but the next number is odd, they return false.

If they find two even numbers and then find a third, they return false.

If they find two even numbers and every remaining number is odd, they return true.

This saves both status and count as variables.

But the shortest version that I could write was

    int evensCount = 0;
    for (int number : numbers) {
        if (number % 2 == 0) {
            evensCount++;
        } else if (evensCount > 0 && evensCount != 2) {
            return false;
        }
    }

    return evensCount == 2;

Note that I renamed a to numbers in this case.  I prefer that kind of name for collections and arrays, and since I only use it once, there was less advantage to terseness.

This version only checks if numbers are even in one place.  All the other versions had to do this in multiple places.  I believe that makes this code DRYer as well as shorter.

This iterates through the array, counting the even numbers.  For each odd number, it checks if it has seen any even numbers.  If it has, it checks if it has seen exactly two.  If not, it can return false.  If it has seen exactly two consecutive even numbers, it keeps going.  If it goes all the way through and finds exactly two even numbers, it can return true.  Otherwise, it returns false.

So if it sees one even number followed by an odd number, it will abort and return false.  The evensCount will be equal to 1.

If it sees three or more consecutive even numbers, it will return false once it sees the next odd number.  Or when it is done iterating if the even numbers last until the end.

If it sees two consecutive even numbers and then an odd number, it will search for more even numbers.  If it finds one, it will eventually return false.  If it doesn't, it will return true after scanning the entire array.

You could sometimes speed it up by adding another if that checks if evensCount is greater than two.  However, that will also slow it down in some situations.  And of course it is more code.  If time is critical, it might help.  Or not.  It really depends on the inputs.  I omitted it because it is only sometimes better at all and I'm not sure we need to optimize to that level.

My test cases:

    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 6, 5, 6, 7}));
    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 6, 6, 7}));
    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 7, 6, 7}));
    System.out.println("The result is: " + isTwinoid(new int[]{3, 8, 5, 7, 3}));
    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 6, 7}));
    System.out.println("The result is: " + isTwinoid(new int[]{3, 3, 2, 6}));

Results:

  The result is: false
  The result is: false
  The result is: false
  The result is: false
  The result is: true
  The result is: true


Bell numbers calculation in C This is my implementation of the bell() function to calculate the n-th bell number in C.

Along with it I made the factorial() and binomial() functions.

I have serious doubts about their efficiency, in particular whether my implementation of the factorial() function should be correctly optimized for tail-recursion, and whether it is possible to rewrite the bell() to be more efficient.

Let me know any critics about my code, and if I can rely on this implementation or maybe use different approaches.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h> // For uint64_t
#include <inttypes.h> // For PRIu64

#define factorial(n) fact(n, 1)

static inline uint64_t fact(uint8_t n, uint64_t inc);
static inline uint64_t binomial(uint8_t n, uint8_t k);
static inline uint64_t bell(uint8_t n);

int main(int argc, char **argv) {
    printf("Bell: %" PRIu64 ".\n", bell(21));

    return EXIT_SUCCESS;
}

// Tail recursion?
static inline uint64_t fact(uint8_t n, uint64_t inc) {
    return (n == 0) ? inc : fact(n - 1, inc * (uint64_t)n);
}

static inline uint64_t binomial(uint8_t n, uint8_t k) {
    return factorial(n) / (factorial(k) * factorial(n - k));
}

// can calculate up to bell(21)
static inline uint64_t bell(uint8_t n) {
    if (n == 0) {
        return 1;
    }

    uint8_t i;
    uint64_t sum = 0;

    n--;

    for (i = 0; i <= n; i++) {
        sum += binomial(n, i) * bell(i);
    }

    return sum;
}

Even without memoization, I cut the calculation time by about 40% when I switched from your version of binomial to the following:

static uint64_t permute(uint8_t n, uint8_t k) {
    uint64_t result = 1;

    for (; n > k; --n) {
        result *= n;
    }

    return result;
}

static inline uint64_t binomial(uint8_t n, uint8_t k) {
    if ( n - k > k ) {
        return permute(n, n - k) / permute(k, 1);
    } else {
        return permute(n, k) / permute(n - k, 1);
    }
}

Note that you could use the ternary operator rather than the if in binomial.

static inline uint64_t binomial(uint8_t n, uint8_t k) {
    return ( n - k > k )
        ? permute(n, n - k) / permute(k, 1)
        : permute(n, k) / permute(n - k, 1);
}

I find the if easier to follow in a complex statement like that.

The reason this works is that \$ \binom{n}{k} = \frac{n!}{k!(n-k)!}\$ can also be written as

$$ \frac{n * (n-1) * ... * (k+1)*k!}{k!(n-k)!} = \frac{n * (n-1) * ... * (k+1)}{(n-k)!} $$

and replacing factorial(k) with permute(k, 1) works because

$$ \frac{n!}{1!} = n! $$

It's unclear if memoization of the factorials would help more.  It may depend on how many values you need to calculate.


Monad to sample without replacement I created a monad in Haskell that lets you sample without replacement from user-defined urns, and then at the end gives you a list of all possible outcomes. It looks like it's similar to the list monad (except that one only ever samples with replacement), and to the ST monad. Here's the interface I want to present:

data Draw s a
data Urn s a
instance MonadPlus (Draw s)
newUrn :: [a] -> Draw s (Urn s a)
drawFrom :: Urn s a -> Draw s a
drawList :: [a] -> Draw s a -- so that you can still sample with replacement, like in the list monad
runDraw :: (forall s. Draw s a) -> [a]

Here's an example of how I want to use it:

runDraw $ do
  l <- newUrn [1,2,3,3]
  x <- drawFrom l
  y <- drawFrom l
  return (x, y)

-- produces [(1,2),(1,3),(1,3),(2,1),(2,3),(2,3),(3,1),(3,2),(3,3),(3,1),(3,2),(3,3)]

And here's what I came up with to implement that:

{-# LANGUAGE MagicHash, RankNTypes, RoleAnnotations #-}

module Draw (Draw, Urn, newUrn, drawFrom, drawList, runDraw) where

import Control.Applicative (Alternative(..))
import Control.Monad (MonadPlus, ap, liftM)
import Data.List (genericSplitAt)
import GHC.Exts (Any, unsafeCoerce#)
import Numeric.Natural (Natural)

newtype Draw s a = Draw { unDraw :: (Natural, [Any]) -> [(a, (Natural, [Any]))] }
type role Draw nominal representational

newtype Urn s a = Urn Natural
type role Urn nominal representational

instance Functor (Draw s) where
  fmap = liftM

instance Applicative (Draw s) where
  pure = drawList . pure
  (<*>) = ap

instance Alternative (Draw s) where
  empty = drawList empty
  Draw m1 <|> Draw m2 = Draw $ \s -> m1 s <|> m2 s

instance Monad (Draw s) where
  Draw m >>= f = Draw $ \s -> m s >>= uncurry (unDraw . f)

instance MonadPlus (Draw s)

drawList :: [a] -> Draw s a
drawList xs = Draw $ \s -> flip (,) s <$> xs

runDraw :: (forall s. Draw s a) -> [a]
runDraw (Draw f) = map fst (f (0, []))

newUrn :: [a] -> Draw s (Urn s a)
newUrn xs = Draw $ \(n, us) -> pure (Urn n, (n + 1, us ++ [toAny xs]))

drawFrom :: Urn s a -> Draw s a
drawFrom (Urn i) = Draw go where
  go :: (Natural, [Any]) -> [(a, (Natural, [Any]))]
  go (n, us) = map (\(x, remainingContents) -> (x, (n, before ++ toAny remainingContents : after))) (removeEach (fromAny urnContents)) where
    (before, urnContents:after) = genericSplitAt i us

fromAny :: Any -> [a]
fromAny = unsafeCoerce#

toAny :: [a] -> Any
toAny = unsafeCoerce#

removeEach :: [a] -> [(a, [a])]
removeEach [] = []
removeEach (x:xs) = (x, xs):map (fmap (x:)) (removeEach xs)

This seems to work, at least with the example I posted above.

Here's my concerns:

I'm doing a lot of unsafeCoerce#, which is obviously not very safe
(before, urnContents:after) = genericSplitAt i us is an incomplete pattern match, which may be able to fail at runtime
I'm building the list of urns with xs ++ [x], which is quadratically slow
I'm not confident that this satisfies all of the typeclass laws, in particular the monad law of associativity I now realize that my type is isomorphic to StateT (Natural, [Any]) [], with equivalent instances, so I'm no longer concerned about this.
I'm not sure if the way I'm handling the urns is correct, or if it's somehow possible to use an urn where it doesn't belong and thus break type safety

This is an interesting problem, and you've written an interesting solution.

With respect to your inefficient urn "store" -- the mapping of immutable urn references (Urn Natural) to mutable urn contents -- it might be worth considering that because of the nature of your monad, most monadic computations involving urns will scale exponentially in the number of urns anyway, so big-O performance of urn list building and lookups is essentially irrelevant.  You can start thinking about it when people want to use your monad for 100000-urn problems; or you could probably port everything over to a Data.Map Int or Data.IntMap in a few minutes).

The bigger problem, as you've noted, is that because this all has to run in a specific monotyped monad, unless you want to pre-declare the set of urns and their element types as used in a particular computation, you need an ugly, unsafe generic type like [Any] to represent your set of urns.

One method of dealing with this would be to represent the mutable contents of an urn by a set of always-integer indices while packaging the actual elements as part of the immutable Urn reference.  That is, the Urn references you pass around can be represented as:

data Urn s a = Urn { tag :: Key
                   , labels :: Int -> a }
type role Urn nominal representational

with monotyped mutable state:

data UrnState = UrnState { nextTag :: Key
                         , urns :: IntMap [Int] }

So urns urnState ! tag1 is the set of integer indices still in play for that urn, and the actual elements are available by looking up those indices in the labels urnRef map.

SPOILERS

A complete code example, which seems to work on your test case is:

{-# LANGUAGE DeriveFunctor, RoleAnnotations, RankNTypes #-}
import Data.List
import Control.Monad
import qualified Data.IntMap as IntMap
import Data.IntMap (Key, IntMap, (!))

data Urn s a = Urn { tag :: Key
                   , labels :: Int -> a }
type role Urn nominal representational

data UrnState = UrnState { nextTag :: Key
                         , urns :: IntMap [Int] }

newtype Draw s a = Draw { unDraw :: UrnState -> [(a, UrnState)] } deriving (Functor)
type role Draw nominal representational
instance Applicative (Draw s) where
  pure x = Draw (\s -> [(x,s)])
  (<*>) = ap
instance Monad (Draw s) where
  Draw d >>= f = Draw $ \s -> do  -- list monad
    (a', s') <- d s
    unDraw (f a') s'

evalDraw :: (forall s. Draw s a) -> [a]
evalDraw (Draw d) = map fst $ d $ UrnState 0 IntMap.empty

newUrn :: [a] -> Draw s (Urn s a)
newUrn xs = Draw $ \(UrnState nxttag urs) ->
  let -- list of labels keyed by indexes [0..n-1]
      lbls = IntMap.fromAscList (zip [0..] xs)
      -- new urn has tag "nxttag" and the immutable labelling function
      u = Urn nxttag (lbls !)
      -- add urn to state
      urs' = IntMap.insert nxttag (IntMap.keys lbls) urs
  in  [(u, UrnState (nxttag+1) urs')]

draws :: [a] -> [(a,[a])]
draws xs = zipWith3 go (inits xs) xs (tail (tails xs))
  where go l a r = (a, l++r)

drawFrom :: Urn s a -> Draw s a
drawFrom (Urn tg lbls) = Draw $ \(UrnState nxttag urs) ->
  case urs ! tg of
    [] -> fail "empty urn"
    xs -> do  -- list monad
      (a, xs') <- draws xs
      return $ (lbls a, UrnState nxttag $ IntMap.insert tg xs' urs)

main :: IO ()
main = print $ evalDraw $ do
  l <- newUrn [1,2,3,3]
  x <- drawFrom l
  y <- drawFrom l
  return (x, y)


Input Columns with Awk I wrote a small script to manipulate fields of a file based on the input of a user. Is there something I could've done better, or perhaps a better refactor? I do know however that I need a way to validate for empty fields.

view_column() {
  for i in "$file_name"; do
    cat "$i" | awk -v col="$1" '{ print $col }'
  done
}

print_menu() {
  menu=( 'First' 'Last' 'Email' 'Exit' )
  for (( i=0; i<${#menu[@]}; i++ )); do
    printf "%2d. %s\n" "$((i+1))" "${menu[$i]}"
  done
}

main() {
  while true; do
    clear
    print_menu
    read -p 'Enter a choice: ' choice
    if (( "$choice" == 4 )); then
      break
    fi
    view_column "$choice"
    sleep 1.5
  done
}

That's very elegant looking code. bash has a builtin select statement to present a menu to the user and get a response --
the advantage of select is that the user is locked into it until a valid response is obtained. Plus, it seems that printing the menu and reading the response should be together.

My code below is not as beautiful as yours, but it allows you to localize all knowledge about the file structure into a single line: local fields=(...)

main() {
  local PS3='Enter a choice: '
  local fields=(First Last Email)

  # reverse the fields array: map field names to column numbers
  local -A column
  for (( i=0; i < ${#fields[@]}; i++)); do
    column[${fields[i]}]=$((i+1))
  done

  while true; do
    clear
    select choice in "${fields[@]}" Exit; do
      # invalid response
      [[ -z $choice ]] && continue

      # exit
      [[ $choice = "Exit" ]] && return

      # do something interesting
      view_column "${column[$choice]}"
      sleep 1.5
      break
    done
  done
}


How to simplify these delegate functions? I'm looking for a way to simplify this code, because I could develop more overloads for TryThis  I made the string and int both of class Nullable so that in each overloaded function, the catch block could return the same value.

The problem is I need, if possible, no overloads of TryThis.  The function overloads are both identical, except for the type of delegate they are passed.  Is there some kind of variable that would encompass any delegate that can be executed?

class Program
{
    delegate int MyIntReturn();
    delegate string MyStringReturn();
    static private MyIntReturn ReadInt = () => {return int.Parse(Console.ReadLine()); };
    static private MyStringReturn ReadString = () => { return Console.ReadLine(); };

    static private Nullable<int> TryThis(MyIntReturn MyAction)
    {
        try
        {
            return MyAction();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
            return null;
        }
    }
    private static Nullable<string> TryThis(MyStringReturn MyAction)
    {
        try
        {
            return MyAction();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.ToString());
            return null;
        }
    }

}

You can use the generic delegate Func<>:

static private T TryThis<T>(Func<T> MyAction) {
    try {
        return MyAction();
    } catch (Exception ex) {
        Console.WriteLine(ex.ToString());
        return default(T);
    }
}


Creating a Student class for a project To recover from yesterday's disaster, I re-read the SOLID principles and refactored my code. Hopefully I did a better job than yesterday. I wanted to keep my Student class immutable, but that meant I would have to change the class every time a new Student type was introduced, that violated the S in SOLID. Here is what I came up with.

public interface StudentStatus {
    Collection<String> retrieveDocuments();
    StudentType retrieveStatus();
}

public final class Student {

   private final String ID;
   private final String firstname;
   private final String lastname;
   private final StudentStatus status;

   public Student(String iD, String firstname, String lastname, StudentStatus status) {
       ID = iD;
       this.firstname = firstname;
       this.lastname = lastname;
       this.status = status;
   }

   public Collection<String> retrieveDocuments() {
       return status.retrieveDocuments();
   }

   public StudentType retrieveStatus() {
      return status.retrieveStatus();
   }

My requirements are:

Domestic students don't require documentation
International student do require documentation (passports, etc..)

Domestic Implementation:

    public final class Domestic implements StudentStatus {

       private final StudentType type;
       private final Collection<String> documents;

       public Domestic() {
           this.type = StudentType.Domestic;
           this.documents = unmodifiableList(new ArrayList<String>(documents);
       }

       @Override
       public Collection<String> retrieveDocuments() {
           return unmodifiableList(new ArrayList<String>(documents);
       }

       @Override
       public StudentType retrieveStatus() {
           return type;
       }
   }

International Implementation:

    public final class International implements StudentStatus {

       private final StudentType type;
       private Collection<String> documents;

       public International(Collection<String> documents) {

           this.type = StudentType.International;
           this.documents = Collections.unmodifiableList(new ArrayList<String>(documents));
       }

       @Override
       public Collection<String> retrieveDocuments() {
           return Collections.unmodifiableList(new ArrayList<String>(documents));
       }

       @Override
       public StudentType retrieveStatus() {
          return type;
       }
    }

If there is any new type of status, it can be added my implementing StudentStatus, no need to modify the Student class.

Use:

        List<String> documents = new ArrayList<String>();
        documents.add("Passport");
        documents.add("Drivers License");

        StudentStatus domestic = new Domestic();
        StudentStatus international = new International(documents);

        List<Student> students = new ArrayList<Student>();
        students.add(new Student("123456789","Susan","Ceesharp", domestic));
        students.add(new Student("987654321","Bill","Finalclass", international));

        for(Student display : students) {
            System.out.println(display.getID() + " " + display.retrieveStatus());
        }

I'm aware none of my classes have validation, I kept these out to keep my code clear and concise.

Goal:
Keep the Student immutable without violating SOLID.

As a minor note you could change this.documents = unmodifiableList(new ArrayList<String>(documents); to this.documents = Collections.emptyList();