What would be proper way to store data from file and validate data from file what is proper way to save all lines from text file to objects. I have .txt file something like this

0001Marcus Aurelius          20021122160   21311
0002William  Shakespeare     19940822332   11092
0003Albert Camus             20010715180   01232

From this file I know position of each data that is written in file, and all data are formatted.

Line number is from 0 to 3
Book author is from 4 to 30
Publish date is from 31 to 37
Page num. is from 38 to 43
Book code is from 44 to 49

I made class Data which holds information about start, end position, value, error.

Then I made class Line that holds list of type Data, and list that holds all error founded from some line. After load data from line to object Data I loop through lineError and add errors from all line to list, because I need to save errors from each line to database.

My question is this proper way to save data from file to object and after processing same data saving to database, advice for some better approach?
Is better approach to create custum attribute which holds position of every record?
For this I can't use third party library.

public class Data
{
    public int startPosition = 0;
    public int endPosition = 0;
    public object value = null;
    public string fieldName = "";
    public Error error = null;

    public Data(int start, int end, string name)
    {
        this.startPosition = start;
        this.endPosition = end;
        this.fieldName = name;
    }

    public void SetValueFromLine(string line)
    {
        string valueFromLine = line.Substring(this.startPosition,
                               this.endPosition - this.startPosition);
        // if else statment that checks validity of data (lenght, empty
         value)
         this.value = valueFromLine;
     }

 }

 public class Line
 {
     public List<Data> lineData = new List<Data>();
     public List<Error> lineError = new List<Error>();

     public Line()
     {
         AddObjectDataToList();
     }

     public void AddObjectDataToList()
     {
         lineData.Add(new Data(0, 3, "lineNumber"));
         lineData.Add(new Data(4, 30, "bookAuthor"));
         lineData.Add(new Data(31, 37, "publishData"));
         lineData.Add(new Data(38, 43, "pageNumber"));
         lineData.Add(new Data(44, 49, "bookCode"));
     }

     public void LoadLineDataToObjects(string line)
     {
         foreach(Data s in lineData)
         {
             s.SetValueFromLine(line);
         }
      }

      public void GetAllErrorFromData()
      {
           foreach (Data s in lineData)
           {
               if(s.error != null)
               {
                    lineError.Add(s.error);
               }

           }
      }

   }

   public class File
   {
     public string fileName;
     public List<Line> lines = new List<Line>();
   }

from what I understood, you need a Model class, since you don't have one for the current data. I assume the file that holds the information is saved by other third-party application, and you're working on a middle-ware application in which would take that file and insert the data into the database.

If possible, try to find another approach than reading the file and try parse each line of it to convert them into an object model. If the other end can process CVS, a table view from the database if any, or an API that would retrieve the information you need, these would be a much safer approaches.

If there is no other way than reading the file and parsing each line. You can still do your idea, but hence, you must know that you need the replace both (the other end application, and this one) with an application that process the data better (combining both application processes into one).

The class you need :

// Book Model
public class Book
{
    // position start index: 0
    //  Length: 4
    public int LineNumber { get; set; }

    // position start index: 4
    //  Length: 25
    public string Author { get; set; }

    // position start index: 29
    //  Length: 8
    public DateTime Publish { get; set; }

    // position start index: 37
    //  Length: 5
    public int PageNumber { get; set; }

    // position start index: 43
    //  Length: 5
    public string Code { get; set; }

    public Book(string line)
    {
        // normal validation
        if (string.IsNullOrEmpty(line)) { throw new ArgumentNullException(nameof(line)); }

        // business validation
        if (line.Length != 48) { throw new InvalidOperationException(nameof(line)); }

        LineNumber = int.TryParse(ParseValue(line, 0, 4), out int lineResult) ? lineResult : -1;

        Author = ParseValue(line, 4, 25);

        Publish = DateTime.TryParse(ParseValue(line, 29, 8), out DateTime publishResult) ? publishResult : DateTime.MinValue;

        PageNumber = int.TryParse(ParseValue(line, 37, 5), out int pageResult) ? pageResult : -1;

        Code = ParseValue(line, 43, 5);
    }

    private string ParseValue(string line, int startIndex, int endIndex)
    {
        // get
        var value = line.Substring(startIndex, endIndex).Trim();

        // check if is it a white space return null
        // we need to discard whitespace initialization.
        // this could be a business requirment where all data should have real data
        // if this is a requirment, then throw an exception instead of null
        if (value.Length == 0) { return null; }

        return value;
    }
}

Then, you stream the file and create a Book from each line or implement GetBooks method example :

public IEnumerable<Book> GetBooks(string filePath)
{
    if(string.IsNullOrEmpty(filePath)) { throw new ArgumentNullException(nameof(filePath)); }

    using (var fs = File.Open(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
    using (var bs = new BufferedStream(fs))
    using (var sr = new StreamReader(bs))
    {
        string line;
        while ((line = sr.ReadLine()) != null)
        {
            yield return new Book(line);
        }
    }
}

then you could do this :

var books = GetBooks(@"filepath");

foreach(var book in books)
{
    // do something with each book object.
}

or you can use LINQ if you need as well.

UPDATE

Since you're looking for using Attribute you need to keep in mind, using attributes would put more work on your hands, as you would define them, and use Reflection to access its values, which would add more processing time to your code performance.

To create a new custom attribute you can do this :

/// <summary>
/// Implement StringPosition custom attribute
/// To define the start position and the length of the extracted string
/// </summary>
[AttributeUsage(AttributeTargets.Property)]
public class StringPositionAttribute : Attribute
{
    public int StartPosition { get; set; }

    public int Length { get; set; }

    public StringPositionAttribute(int startPosition, int endPosition)
    {
        StartPosition = startPosition;
        Length = endPosition;
    }
}

Now, you can use [StringPosition(startIndex, length)] attribute on the class properties.

Example :

// Book Model
public class Book
{
    [StringPosition(0, 4)]
    public int LineNumber { get; set; }

    [StringPosition(4, 25)]
    public string Author { get; set; }

    [StringPosition(29, 8)]
    public DateTime Publish { get; set; }

    [StringPosition(37, 5)]
    public int PageNumber { get; set; }

    [StringPosition(43, 5)]
    public string Code { get; set; }
}

now, by doing this, we can create a method where it loop over all of these properties and extract the targeted string based on the values of their attributes, then save the results for each.

private void ParseValues(string line)
{
    foreach(var property in this.GetType().GetProperties())
    {
        // Get property attribute
        var attribute = (StringPositionAttribute)property.GetCustomAttribute(typeof(StringPositionAttribute));

        // Get the start index value from the attribute
        var startIndex = attribute.StartPosition;

        // Get the length value from the attribute
        var length = attribute.Length;

        // parse the values to get the string
        var value = ParseValue(line, startIndex, length);

        // set the value of the property for this instance
        property.SetValue(this, Convert.ChangeType(value, property.PropertyType), null);
    }
}

now use this method in the constructor :

public Book(string line)
{
    // normal validation
    if (string.IsNullOrEmpty(line)) { throw new ArgumentNullException(nameof(line)); }

    // business validation
    if (line.Length != 48) { throw new InvalidOperationException(nameof(line)); }

    // do the parsing
    ParseValues(line);
}

this is a quick examples to give you a boost start, the attribute can be useful in many cases, such as validation, or messages ..etc. But I believe your work does not need that much of work.

I hope this would give you the boost you need.


LINQ queries to fetch first and last name I have these two queries and they work just fine.  I use the values they retrieve to populate a title for a view in my application.  It seems though that I should be able to accomplish this with one query and I do not know how.

Dim firstName = (From row In db.ItemDetails
    Where row.Unique_Location_ID = locationID And row.Item_Type_Identifier = 1 And _
    row.Category_Attribute_Identifier = 2
    Select CStr(row.Value)).FirstOrDefault()

Dim lastName = (From row In db.ItemDetails
    Where row.Unique_Location_ID = locationID And row.Item_Type_Identifier = 1 _
    And row.Category_Attribute_Identifier = 3
    Select CStr(row.Value)).FirstOrDefault()

ViewBag.assignedTo = ("Employee this item is currently assigned to: " + firstName + " " + lastName + ".")

Is there a better way to do this?

You can achieve this by using the group by clause to create an aggregated list constrained by Unique_Location_ID and Item_Type_Identifier. To make the query more readable, store the names in local variables using the let clause.

Dim name As String = (
    From row In (From r In db.ItemDetails Where r.Unique_Location_ID = locationID And r.Item_Type_Identifier = 1 Select r)
    Group row By row.Unique_Location_ID Into rows = Group
    Let firstName = (From row In rows Where row.Category_Attribute_Identifier = 2 Select CStr(row.Value)).FirstOrDefault()
    Let lastName = (From row In rows Where row.Category_Attribute_Identifier = 3 Select CStr(row.Value)).FirstOrDefault()
    Select String.Concat(firstName, " ", lastName).Trim()
).FirstOrDefault()

I also advice you to not use underscores in public members as suggested in the .net general naming conventions. So, if possible, rename the members to UniqueLocationID, ItemTypeIdentifier and CategoryAttributeIdentifier.


Hash Tables: Ice Cream Parlor solution in javascript I tried solving the Hash Tables: Ice Cream Parlor question in javascript:

'use strict';

process.stdin.resume();
process.stdin.setEncoding('utf-8');

let inputString = '';
let currentLine = 0;

process.stdin.on('data', inputStdin => {
    inputString += inputStdin;
});

process.stdin.on('end', function() {
    inputString = inputString.replace(/\s*$/, '')
        .split('\n')
        .map(str => str.replace(/\s*$/, ''));

    main();
});

function readLine() {
    return inputString[currentLine++];
}

// Complete the whatFlavors function below.
function whatFlavors(cost, money) {

// Solution starts here
  var dict1={};

  var len=cost.length;

  for(var i=0; i<len; i++) {

    if (cost[i]<money) {
      if(dict1[cost[i]] !== undefined)
      {
       if(Array.isArray(dict1[cost[i]]))
       {
         dict1[cost[i]].push(i);
       }
       else
       {
         dict1[cost[i]] = [dict1[cost[i]]];
         dict1[cost[i]].push(i);
       }
     }
     else
       dict1[cost[i]] = i;
    }
  }

  cost.sort(function(a,b) {return a-b});

  for(var j=0; j<len; j++){
    var left=0;
    var right=len-1;
    if(cost[j]<money){
      var ser=money-cost[j];
      while(left<right){
        var mid=Math.floor((left+right)/2);

        if(ser===cost[mid]){
          var val1=cost[j];
          var val2=cost[mid];
          break;
        }
        if(ser<cost[mid]){
          right = mid-1;
        }
        else{
          left = mid+1;
        }
      }
      if(val1 !== undefined && val2 !== undefined ){
        break;
      }
    }
  }

  var index1;
  var index2;
  if (val1===val2) {
     index1 = dict1[val1][0];
     index2 = dict1[val2][1];
  }
  else{
     index1 = dict1[val1];
     index2 = dict1[val2];
  }

  if (index2 > index1) {
    console.log(index1+1, index2+1);
  }
  else{
    console.log(index2+1, index1+1);
  }
  // Solution ends here
}

function main() {
    const t = parseInt(readLine(), 10);

    for (let tItr = 0; tItr < t; tItr++) {
        const money = parseInt(readLine(), 10);

        const n = parseInt(readLine(), 10);

        const cost = readLine().split(' ').map(costTemp => parseInt(costTemp, 10));

        whatFlavors(cost, money);
    }
}

It works for all inputs except when it is very large, for example when the input is very large for eg:
30733 39289
12352 19413
448 3955
74 75
12316 34744
2916 4669
1941 6571
2871 17443
34132 42603
1753 9623
7217 8111
3411 17665
3190 16653
1923 14237
6307 22944
10874 22052
967 21913
7562 7948
11038 36319
586 8260
338 1426
17083 37691
11944 15889
10347 13601
643 1653
18754 19595
9561 22822
22521 26308
114 1965
338 412
8423 9497
6371 33551
1292 3705
5634 9563
14043 14669
12566 39425
1149 2638
12664 12939
10217 29104
it simply throws Wrong Answer, is there a better way to write the same script?

The error was because of a silly mistake, in the binary search, it should be while(left<=right):

'use strict';

process.stdin.resume();
process.stdin.setEncoding('utf-8');

let inputString = '';
let currentLine = 0;

process.stdin.on('data', inputStdin => {
    inputString += inputStdin;
});

process.stdin.on('end', function() {
    inputString = inputString.replace(/\s*$/, '')
        .split('\n')
        .map(str => str.replace(/\s*$/, ''));

    main();
});

function readLine() {
    return inputString[currentLine++];
}

// Complete the whatFlavors function below.
function whatFlavors(cost, money) {
  // Solution starts here
  var dict1={};

  var len=cost.length;

  for(var i=0; i<len; i++) {

    if (cost[i]<money) {
      if(dict1[cost[i]] !== undefined)
      {
       if(Array.isArray(dict1[cost[i]]))
       {
         dict1[cost[i]].push(i);
       }
       else
       {
         dict1[cost[i]] = [dict1[cost[i]]];
         dict1[cost[i]].push(i);
       }
     }
     else
       dict1[cost[i]] = i;
    }
  }

  cost.sort(function(a,b) {return a-b});

  for(var j=0; j<len; j++){
    var left=0;
    var right=len-1;
    if(cost[j]<money){
      var ser=money-cost[j];
      while(left<=right){
        var mid=Math.floor((left+right)/2);

        if(ser===cost[mid]){
          var val1=cost[j];
          var val2=cost[mid];
          break;
        }
        if(ser<cost[mid]){
          right = mid-1;
        }
        else{
          left = mid+1;
        }
      }
      if(val1 !== undefined && val2 !== undefined ){
        break;
      }
    }
  }

  var index1;
  var index2;
  if (val1===val2) {
     index1 = dict1[val1][0];
     index2 = dict1[val2][1];
  }
  else{
     index1 = dict1[val1];
     index2 = dict1[val2];
  }

  if (index2 > index1) {
    console.log(index1+1, index2+1);
  }
  else{
    console.log(index2+1, index1+1);
  }
  // Solution ends here
}

function main() {
    const t = parseInt(readLine(), 10);

    for (let tItr = 0; tItr < t; tItr++) {
        const money = parseInt(readLine(), 10);

        const n = parseInt(readLine(), 10);

        const cost = readLine().split(' ').map(costTemp => parseInt(costTemp, 10));

        whatFlavors(cost, money);
    }
}


Simple Console Maze Game I am fairly new to C++ and programmed a maze game as an exercise. However, looking at it I can see it can be improved a lot. These are my requests if you don't mind:

I do not want to use the library <conio.h>.
I want to use another method instead of system("CLS").
This code is old; I think it has many unsafe codes in it and I cannot get used to C++11, C++17 etc. If you can help me update this game to the newer variants of C++, I would be more than happy.
In main function I just want to use function calling, so an Update() method would be nice.
OOP is highly appreciated.
Please do not hesitate to use advanced C++ like Lambda Expressions, auto type declarations etc. (see number 3). I don't know them yet, but I will try to understand.
Criticize like there is no tomorrow!

//maze game in console

#include <iostream>
#include <conio.h>
#include <stdlib.h>

#define KEY_UP 72
#define KEY_DOWN 80
#define KEY_LEFT 75
#define KEY_RIGHT 77

// S: Start
// #: Wall
// .: Space
// F: Finish
// P: Player

const char maze[5][5]={{'#','S','#','#','#'},
                       {'#','.','.','#','#'},
                       {'#','#','.','#','#'},
                       {'#','.','.','.','#'},
                       {'#','#','#','F','#'}};

void Clear(){
    system("CLS");//clear console
}
void InputHandle(int move, int &pX, int &pY){
    switch(move=getch()){
            case KEY_UP:
            if( pY-1 >= 0 ){
                if(maze[pY-1][pX] != '#'){//if it is not the limit or it is not the wall(#) move player
                pY--;
            }
        }
        break;

        case KEY_DOWN:
        if( pY+1 <= 4 ){
            if(maze[pY+1][pX] != '#'){
                pY++;
            }
        }
        break;

        case KEY_RIGHT:
        if( pX+1 <= 4 ){
            if(maze[pY][pX+1] != '#'){
                pX++;
            }
        }
        break;

        case KEY_LEFT:
            if( pX-1 >= 0 ){
            if(maze[pY][pX-1] != '#'){
                pX--;
            }
        }
}
}
void Display(int &pX, int &pY){
for(int i=0;i<5;i++){
    for(int j=0;j<5;j++){
        if(j==pX && i==pY){//instead of the maze char, put player's 'P'
            std::cout<<"P ";
            continue;
        }
        std::cout<<maze[i][j]<<" ";
    }
    std::cout<<"\n";
}
}
bool CheckWinCondition(int &pX, int &pY){
if(pX==3 && pY ==4){
    return true;
}
return false;
}

int main(){

//movement
int playerPosX=1;
int playerPosY=0;
int movement;

while(!CheckWinCondition(playerPosX,playerPosY)){//if player is not in the finish, loop
    Display(playerPosX, playerPosY);//Show current maze
    InputHandle(movement, playerPosX, playerPosY);//take input
    Clear();//Clear the screen
}
std::cout<<"Congrats, you finished the maze!";

}

Here are some things that may help you improve your code.

Use consistent formatting

The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.

Use more whitespace

Lines like this one:

for(int i=0;i<5;i++){

are easier for most humans to read and understand with more whitespace like this:

for (int i = 0; i < 5; i++) {

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause").  The first is that it is not portable to other operating systems which you may or may not care about now.  The second is that it's a security hole, which you absolutely must care about.  Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything.  First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system.  Then rewrite the contents of those functions to do what you want using C++.  For example, if your terminal supports ANSI Escape sequences, you could use this:

void Clear()
{
    std::cout << "\x1b[2J";
}

Minimize the scope of variables

The variable movement is only used within InputHandle.  This means that instead of a parameter, it could and should be a local variable within the member functions rather than a passed parameter.

Only update when needed

The current code updates the screen as fast as possible whether or not there's any reason to do so.  This causes an annoying flicker on my computer and needlessly impedes the quick response of the program to user input.  I'd suggest instead modifying the InputHandle routine to return a bool which is true only if the player's position has changed.

Prefer std::array to raw C-style arrays

Modern C++ provides std::array as an enhancement of a raw C array because it has knowledge of its own size, which helps with bounds checking.

Eliminate "magic numbers"

This code has a number of "magic numbers," that is, unnamed constants such as 4, 5, etc.  Generally it's better to avoid that and give such constants meaningful names.  That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "5" and then trying to determine if this particular 5 is relevant to the desired change or if it is some other constant that happens to have the same value.

Simplify your code

The current code contains this function:

bool CheckWinCondition(int &pX, int &pY)
{
    if (pX == 3 && pY == 4) {
        return true;
    }
    return false;
}

This can be simplified:

bool CheckWinCondition(int &pX, int &pY)
{
    return pX == 3 && pY == 4;
}

Use a portable library

You are right to want to eliminate the use of conio.h because it is neither standard nor portable.  For example, your constants for keys don't work on my machine at all.  One possible alternative would be to use the ncurses library.  See Ncurses Snake game for inspiration on how to use the ncurses library in C++.

Use better names

The function CheckWinCondition and variable maze are good, descriptive names, but InputHandle and pX are not as good.  I'd probably change InputHandle to handleUserInput and pX to simply x.  Also, CheckWinCondition could be named isPlayerAtGoal to make it easy at a glance to understand what true and false might mean.

Don't use macros for constants

Instead of having KEY_UP and friends as old-style #define values, a better, more C++ approach is to use constexpr instead. See ES.32 and Con.5.

Use an object

The Maze would make a rather obvious object.  It could include the maze structure, the player's current position and the goal's location.  Methods (member functions) could include all of the current free-standing functions and the main could be reduced to this:

int main() {
    Maze maze;
    maze();
}

Further hints

Here's a rewritten display() function written using ncurses as a member function:

void Maze::display() const
{
    clear();
    for (int row = 0; row < height; row++) {
        for (int col = 0; col < width; col++) {
            addch(tokenAt(row, col));
            addch(' ');
        }
        addch('\n');
    }
}

The clear() and addch() functions are from ncurses and tokenAt() looks like this:

char Maze::tokenAt(int row, int col) const {
    return (col == x && row == y) ? 'P' : maze[row][col];
}

Here's the member function that actually runs the maze:

void Maze::operator()() {
    initscr();
    noecho();
    nodelay(stdscr, TRUE);
    keypad(stdscr, TRUE);
    display();
    while (!isPlayerAtGoal()) {
        if (handleUserInput()) {
            display();
        }
    }
    endwin();
    std::cout << "Congrats, you finished the maze!\n";
}


Java Scanner String input I am doing Java exercises from HackerRank.com and bit confused about this problem.

Here is my code that pass the test case.

import java.io.*;
import java.util.*;

public class Solution {

    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);

     // String input = in.nextLine();

        int counter = 1;
        while (in.hasNext()) {
            System.out.println(counter + " " + in.nextLine());
            counter++;
        }
    }
}

We are suppose to print number of lines of input from Scanner, until line of input contains a empty String. The line of inputs are automatically generated by the HackerRank website. For test case, three lines of input will be generated.

  Test case input

    Hello World

    I am a file

    Read me until end-of-file.

  Expected output

    1 Hello World

    2 I am a file

    3 Read me until end-of-file.

However, whenever I uncomment out the

String input = in.nextLine();

My output only shows first 2 lines instead of all 3. I thought I need that line of code in order to type a line of input. For example,

Scanner sc = new Scanner(System.in);
System.out.println("Enter your name.");
String name = sc.nextLine();

Also when I write

String input = in.next();

instead of nextLine() code, my output shows all 3 lines but first word "Hello" is omitted.

  in.next(); output

    1   World

    2 I am a file

    3 Read me until end-of-file.

What I want to know is why I don't need String input = in.nextLine(); to pass the test case.

The nextLine method is included in your print, so when you include it at the start then the first line ("Hello World") is stored in input and it is not touched again. Then, when the nextLine in the println method runs, it checks for the next line's content ("I am a file"), and prints "1 I am a file" because it is the first iteration of the loop. It then works successfully in getting and printing the rest of the lines of input.

The next method gets the next word in this context, so using it would cause "Hello" to be stored in input and, as input is never used again, forgotten. nextLine then gets the rest of the line's content and continues successfully for other input lines.

Neither methods are required to start with because the call to nextLine within println runs successfully for each iteration.


Remove unwanted characters from a string From what I've seen in other posts, if I actually know the unwanted characters, then I can do string.replace(). But in my case, any characters can appear in my input string, and I only want to keep the characters that I want (without messing up the order of course).

private string RemoveUnwantedChar(string input)
{
    string correctString = "";

    for (int i = 0; i < input.Length; i++)
    {
        if (char.IsDigit(input[i]) || input[i] == '.' || input[i] == '-' || input[i] == 'n'
                || input[i] == 'u' || input[i] == 'm' || input[i] == 'k' || input[i] == 'M'
                || input[i] == 'G' || input[i] == 'H' || input[i] == 'z' || input[i] == 'V'
                || input[i] == 's' || input[i] == '%')
            correctString += input[i];
    }
    return correctString;
}

Characters that I want:

0123456789
numkMGHzVs%-

How can I tidy this code to be neater and more readable?

By having a const string which contains all of your wanted chars, you could do either a simple call to Contains() or check if IndexOf() will return a value > -1.
using string concatenation in a loop is mostly a bad idea. Use a StringBuilder instead.
omitting braces {} although they are optional for single lined if statements is a bad idea because it makes your code error prone.

Implementing the mentioned points will lead to

private const string allowedCharacters = "numkMGHzVs%-.";
private string RemoveUnwantedChar(string input)
{

    StringBuilder builder = new StringBuilder(input.Length);

    for (int i = 0; i < input.Length; i++)
    {
        if (char.IsDigit(input[i]) || allowedCharacters.Contains(input[i]))
        {
            builder.Append(input[i]);
        }
    }
    return builder.ToString();
}

@Caricorc made a good suggestion in the comments

  In my opinion allowedCharacters should be an argument to the function to allow reusability.

So by passing the allowedCharacters as an optional parameter with an additional check with IsNullOrEmpty().

If performance is an issue, you could also pass a HashSet<char> to the method or have an overloaded method like so

private string RemoveUnwantedChar(string input, string allowedCharacters = "0123456789numkMGHzVs%-.")
{

    if (string.IsNullOrEmpty(allowedCharacters)) { return input; }

    return RemoveUnwantedChar(input, new HashSet<char>(allowedCharacters));

}
private string RemoveUnwantedChar(string input, HashSet<char> allowedCharacters)
{
    if (allowedCharacters.Count == 0) { return input; }

    StringBuilder builder = new StringBuilder(input.Length);

    for (int i = 0; i < input.Length; i++)
    {
        if (allowedCharacters.Contains(input[i]))
        {
            builder.Append(input[i]);
        }
    }
    return builder.ToString();
}

you can reuse it somewhere else.


CSS list-style with Font-Awesome toolkit I will design a list with a Font-Awesome design arrows.
My code already works fine, but I will improve it.

ul.long_arrow_right_red,ul.long_arrow_right_gold     {list-style: none;}
ul.long_arrow_right_gold li:before {content:"\f178"; font-family: FontAwesome;display: block;float: left;width: 1.5em;color:gold;}
ul.long_arrow_right_red  li:before {content:"\f178"; font-family: FontAwesome;display: block;float: left;width: 1.5em;color:red;}
<link rel="stylesheet" type="text/css" media="screen" href="https://maxcdn.bootstrapcdn.com/font-awesome/4.5.0/css/font-awesome.min.css" />

Title

Lorem

Epsum

Title

Questions:

How to use an extra class which define only the color for the li icon? The text inside shouldn't change the color.

Existing other css code, which I use for text coloring:

.red{color:red;}
.gold{color:gold;}

I think it would be better not do have duplicate following statements:

font-family: FontAwesome;display: block;float: left;width: 1.5em;

How to combine them with a different content: definition?
If you have other improvement ideas, please tell me them.

Properly formatted CSS can make your styles vastly more readable, helping you find bugs as well as making it easier to work with.

This:

ul.long_arrow_right_red,ul.long_arrow_right_gold     {list-style: none;}
ul.long_arrow_right_gold li:before {content:"\f178"; font-family: FontAwesome;display: block;float: left;width: 1.5em;color:gold;}
ul.long_arrow_right_red  li:before {content:"\f178"; font-family: FontAwesome;display: block;float: left;width: 1.5em;color:red;}

Should instead be something like this:

ul.long_arrow_right_red,
ul.long_arrow_right_gold {
    list-style: none;
}

ul.long_arrow_right_gold li:before {
    content: "\f178";
    font-family: FontAwesome;
    display: block;
    float: left;
    width: 1.5em;
    color: gold;
}

ul.long_arrow_right_red li:before {
    content: "\f178";
    font-family: FontAwesome;
    display: block;
    float: left;
    width: 1.5em;
    color: red;
}

If you want to have the code "minified" then do that before you deploy it, source code should aim for readability and maintainability, rather than length / character count.


Counting e-mails in a mailbox for each hour Pardon if this is silly, but I'm pretty new to coding. I have this code, that works, but I feel like it could be tighter than this, I just can't seem to get it to work any other way. The idea is to read a .txt file find incoming e-mail strings and organize the data by frequency of hour sent.

Here is an example line that I'm looking for in the text:

From email@emailaddress.com Sat Jan 5 09:14:16 2008

Here is my code:

fname = input("Enter file:")
if len(fname) <1 : fname = "filename.txt"
fh = open(fname)
time = list()
hours = list()
hr = dict()

for line in fh:
        if not line.startswith("From "): continue
        words = line.split()
        time.append(words[5])

for i in time:
        i.split(":")
        hours.append(i[:2])

for h in hours:
        hr[h]=hr.get(h, 0)+1

l = list()
for k,v in hr.items():
        l.append((k,v))
l.sort()
for k, v in l:
        print (k,v)

Formatting

Please follow PEP8, the Python style guide.
Most noticeably,
use 4 spaces to indent.

Working with files

The recommended way to work with files is using with ... as, like this:

with open(fname) as fh:
    for line in fh:
        if not line.startswith("From "):
            continue
        words = line.split()
        time.append(words[5])

This way, you don't have to remember to close the filehandle when you're done using it.

Naming

Use plural names for collections.
time is really odd for a list of time strings.
And I suppose that's part of the reason for naming the loop variable i here:

for i in time:
    i.split(":")
    hours.append(i[:2])

When it would have been so much more natural as:

for time in times:
    time.split(":")
    hours.append(time[:2])

Pointless statements

i.split here is completely pointless. It splits the string, but then the result is never used:

for i in time:
    i.split(":")
    hours.append(i[:2])

This is the same thing:

for i in time:
    hours.append(i[:2])

Use list comprehensions

The hours list can be created simply as:

hours = [time[:2] for time in times]

Use collections.Counter

Instead of this:

for h in hours:
        hr[h]=hr.get(h, 0)+1

You could create hr a lot simpler:

from collections import Counter

hr = Counter(hours)

(Of course, the import statement should be at the top of the script.)

Sorting

Instead of this:

l = list()
for k, v in hr.items():
    l.append((k, v))
l.sort()
for k, v in l:
    print(k, v)

You could use the builtin sorted, and greatly simplify:

for hour, count in sorted(hr.items()):
    print(hour, count)

Notice that I renamed the loop variables. How much easier to read that!

Use functions

Rather than just a sequence of statements,
it would be better to decompose your logic to functions.

Suggested implementation

With the above suggestions applied (+ PEP8, do read that!), and then some more:

from collections import Counter

DEFAULT_FILENAME = 'filename.txt'

def input_filename():
    filename = input("Enter file:")
    if not filename:
        filename = DEFAULT_FILENAME
    return filename

def read_times(filename):
    with open(filename) as fh:
        for line in fh:
            if line.startswith("From "):
                yield line.split()[5][:2]

def main():
    filename = input_filename()

    hour_counts = Counter(read_times(filename))

    for hour, count in sorted(hour_counts.items()):
        print(hour, count)

if __name__ == '__main__':
    main()


Fading in while scrolling - is this an elegant solution? I have posted such question at StackOverflow but the single answer I got there was to ask here.
My task is to get elements fading in while the user is scrolling. Here is my link - http://layot.prestatrend.com/ I have made it with the help of such jQuery code I've gathered from two sources:

<script type="text/javascript">
 $(document).ready( function() {

 tiles = $('.ajax_block_product').fadeTo(0,0);

 $(window).scroll(function(d,h) {
    tiles.each(function(i) {
        a = $(this).offset().top + $(this).height();
        b = $(window).scrollTop() + $(window).height();
        if (a < b) $(this).fadeTo(500,1);
    });
 });

 function inWindow(s){
  var scrollTop = $(window).scrollTop();
  var windowHeight = $(window).height();
  var currentEls = $(s);
  var result = [];
  currentEls.each(function(){
    var el = $(this);
    var offset = el.offset();
    if(scrollTop <= offset.top && (el.height() + offset.top) < (scrollTop + windowHeight))
      result.push(this);
  });
  return $(result);
}

inWindow('.ajax_block_product').fadeTo(0,1);

});
</script>

Is this an elegant solution or is there a way to make the code shorter and more elegant?

The onScroll event is generally very performance hungry, since it is called quite often, while scrolling. Looping through all "ajax_block_product" elements on every singel call, even if most of them might be outside of the viewport, makes it even worse. But you can use some specialities of JS to optimize it. As a short intro: In JS you can set an array element at a defined index, leaving the ones in between undefined while having the length of the array set to the highest index defined.

a[100] = 'one';
a[1000] = 'two';
for (var i = 0; i < a.length; i++) {
    console.log('iterate'); // called 1000 times
}
for (var elm in a) {
    console.log('iterate'); // called two times
}

Now if you loop through the array with a typical for(var i = 0; i < a.length; i++) you will have 1000 iterations BUT since an array is also an object in JS a for(elm in a) would only iterate 2 times. We can now use this to store all Y-positions of "ajax_block_product" elements as indexes of an array, than determine the dimension of the current viewport and cut out only those element who are inside that range.

$(document).ready(function () {
    var tiles = $('.ajax_block_product'),
        winHeight = $(window).height(),
        posArr = [];

    tiles.each(function () {
        var pos = $(this).offset().top;
        posArr[pos] = $(this);
    });

    $(window).scroll((function fadeIn() {
        var sTop = $(window).scrollTop(),
            range = posArr.slice(sTop, sTop + winHeight);
        range.forEach(function (elm) {
            elm.css('opacity', 1);
        });
        return fadeIn;
    }()));
});

In this example I've used CSS3: transition: opacity .5s ease-in; for the fade in. You need to check out how to do this with jQuery's fadeIn, if needed...

Last but not least, the onScroll event callback is somehow special. It a self-executing, self-referencing function. It is called once, when it's defined, to initially show all elements already in the viewport and then returns a reference to itself for later onScroll event calls.

Hope this could help you, even if it's some time ago, you asked this question...


Implementation of Multiparental Sorting Crossover I implemented the Multiparental Sorting Crossover as described here in section 4.2. The (compiled) program can be run as follows:

$ java MPSX input.txt

where input.txt looks like this (the first line is the mask followed by three parents):

3 1 2 1 1 1 1 2 2

8 4 7 3 6 2 5 1 9

9 1 2 3 4 5 6 7 8

4 7 9 3 6 2 5 1 8

As for output, it merely prints the correct result as a string.

import edu.princeton.cs.algs4.In;
import java.io.InputStream;

public class MPSX {

    public static void main(String[] args) {

        String filename = args[0];
        In in = new In(filename);

        String[] Input = in.readAllLines();

        String result = solve(Input);

        System.out.println(result);

    }

    private static void swap(int[] parent_x, int[] parent_y, int index) {

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

            if (parent_y[i] == parent_x[index]) {

                int tmp = parent_y[index];
                parent_y[index] = parent_y[i];
                parent_y[i] = tmp;

            }

        }

    }

    private static String solve(String[] Input) {

        String result = "";

        String[] parent1str = Input[1].split(" ");
        String[] parent2str = Input[2].split(" ");
        String[] parent3str = Input[3].split(" ");

        int[] parent1 = new int[parent1str.length];
        int[] parent2 = new int[parent2str.length];
        int[] parent3 = new int[parent3str.length];

        for (int i = 0; i < parent1str.length; i++) {
            parent1[i] = Integer.parseInt(parent1str[i]);
            parent2[i] = Integer.parseInt(parent2str[i]);
            parent3[i] = Integer.parseInt(parent3str[i]);
        }

        String mask = Input[0].replaceAll("\\s","");

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

            int mask_element = Character.getNumericValue(mask.charAt(i));;

            if (mask_element == 1) {
                result += parent1[i];
                swap(parent1, parent2,i);
                swap(parent1, parent3,i);
            }

            else if (mask_element == 2) {
                result += parent2[i];
                swap(parent2, parent1,i);
                swap(parent2, parent3,i);
            }

            else if (mask_element == 3) {
                result += parent3[i];
                swap(parent3, parent1,i);
                swap(parent3, parent2,i);
            }

        }

        return result;
    }

}

It is effectively the first program I wrote in Java so I know there is a lot of room for improvement and refactorization.

In particular, I am sure there is a better (more concise) way of processing the input text file. I thought of creating an array of arrays of Strings instead of 3 separate arrays but I wasn't sure how to do it.

Parsing a line of space separated integers is repeated 4 times you can replace those with the method below:

private static int[] parseLineOfInts(String s) {
    String[] tokens = s.split(" ");
    int[] arr = new int[tokens.length];
    for (int j = 0; j < tokens.length; j++)
       arr[j] = Integer.parseInt(tokens[j]);
    return arr;
}

You can use arrays of arrays to hold 2D data. Parsing the parents becomes as below:

int nParents = 3;
int[][] parents = new int[nParents][];
for (int i = 0; i < nParents; i++) {
    parents[i] = parseLineOfInts(Input[i + 1]);
}

Note parent1 is now parent[0] and so on.

You can use parseLineOfInts method to calculate mask_elements as below:

int[] maskElements = parseLineOfInts(Input[0]);

parameters and local variables should be camelcase: first word starts with lowercase, subsequent words start with upper case:

Input should be input
parent_x should be parentX
parent_y should be parentY

Separate parsing and actual algorithm. Separation of concerns improves readability, testability, many other ...abilities. As a first step you can extract not string manipulation related code into a new function such that its signature becomes as below:

int[] solve(int[] mask, int[][] parents)


LINQ queries improvement - repetition of query I have a DAL class full of LINQ queries which I would like to improve as I found that I'm repeating myself couple of times.

As you can see the part below is repeated couple of times and I was wondering if it is possible to reduce the number of repetitions (passing the query somehow?) and/or improve the queries overall.

  .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)

    private DashboardEntities db;

    public AnswerSelectLabelRepository(DashboardEntities _db)
    {
        this.db = _db;
    }

    public AnswerSelectLabel GetById(string idAnswerSelectLabel)
    {
        return db.AnswerSelectLabel.Find(idAnswerSelectLabel);
    }

    public IEnumerable<AnswerSelectLabel> GetAll()
    {
        return db.AnswerSelectLabel
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .OrderBy(x => x.DisplayOrder)
            .ThenBy(x => x.Value);
    }

    public IEnumerable<AnswerSelectLabel> GetByVariable(string idVariable)
    {
        return GetAll().Where(x => x.idVariable == idVariable);
    }

    public IEnumerable<AnswerSelectLabel> GetByVariable(IEnumerable<Respondent> respondents, string idVariable)
    {
        return respondents.SelectMany(x => x.AnswerSelect)
            .Select(x => x.AnswerSelectLabel)
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .Where(x => x.idVariable == idVariable);
    }

    public IEnumerable<AnswerSelectLabel> GetByQuestion(string idQuestion)
    {
        return GetAll().Where(x => x.Variable.idQuestion == idQuestion);
    }

    public IEnumerable<AnswerSelectLabel> GetByQuestion(IEnumerable<Respondent> respondents, string idQuestion)
    {
        return respondents.SelectMany(x => x.AnswerSelect)
            .Select(x => x.AnswerSelectLabel)
            .Where(x => x.Active == true && x.Variable.Active == true && x.Variable.Question.Active == true)
            .Where(x => x.Variable.idQuestion == idQuestion);
    }

The first thing (and this is very minor) is that the expression a == true and simply a are identical. Thus you don't need all those == true's.

The simplest, most obvious way is to simply extract a private method for your filter:

private boolean isActive(AnswerSelectLabel answer)
{
  return answer.Active
      && answer.Variable.Active
      && answer.Variable.Question.Active
}

public IEnumerable<AnswerSelectLabel> GetAll()
{
  return db.AnswerSelectLabel
      .Where(isActive) //you'll pardon me if this is syntax error, I've been in java land too long now, where this would be this::isActive
      .OrderBy(x => x.DisplayOrder)
      .ThenBy(x => x.Value);
}

public IEnumerable<AnswerSelectLabel> GetByVariable(IEnumerable<Respondent> respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
      .Select(x => x.AnswerSelectLabel)
      .Where(isActive)
      .Where(x => x.idVariable == idVariable);
}

and so on.

Slightly more sophisticated (and almost certainly overkill), is to extract some kind of interface on your answer, variable, and Question, classes. Something like

interface IActivatable
{
  public Active { get; }
}

If you make this change, you can start to play games with exactly what you're asking to be active:

public IEnumerable<AnswerSelectLabel> GetByVariable(IEnumerable<Respondent> respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
    .Select(x => x.AnswerSelectLabel)
    .Where(x => new IActivatable[]{x, x.Answer, x.Answer.Question}.all(x => x.Active))
    .Where(x => x.idVariable == idVariable);
}

which, again, we can extract a method for. But this time we're parameterized on the actual things we want to check, which is nice because it means we can pick-and-choose what we want to be active. In your case it looks like this is always answer, question, and variable, but if that's not the case this is likely helpful:

public IEnumerable<AnswerSelectLabel> GetByVariable(IEnumerable<Respondent> respondents, string idVariable)
{
  return respondents.SelectMany(x => x.AnswerSelect)
    .Select(x => x.AnswerSelectLabel)
    .Where(x => isActive(x, x.Answer, x.Answer.Question))
    .Where(x => x.idVariable == idVariable);
}

private boolean isActive(params IActivateable activatables){
  return activatables.all(x => x.Active);
}

Of course, if this isn't the case, this is just useless speculative code, so don't do it.

If I could make a couple more notes:

using strings as a catch-all for ID's has gotten me in trouble, so now I push to create specialized ID classes (where a simple GUID wont do). Don't let this burn you too!
GetByQuestion(IEnumerable<Respondent> respondents, string idQuestion) and GetByVariable(IEnumerable<Respondent> respondents, string idVariable) are suffering from a sort of linq-feature-envy. They don't use your db field at all, so why are they on this class? Do these methods exist so that I can do something like this:

var respondents = dal.GetAllRespondents();
respondents = respondents.Where(callerSpecificFilter);
var asnwers = GetByVariable(respondents, "1234");

If this is the case, and you can afford the potential performance hit, you can remove the respondents parameter and take a Func<Respondent, Boolean> filter instead. Then simply apply the filter yourself.


How to declutter dependencies? I'm running into a problem with my code in a ASP.NET MVC application. Maybe it's not a problem and I'm just being paranoid, but it seems like a problem to me (might be because I've just read the book Clean Code).

I am using the repository-pattern with the unit of work-pattern to abstract the data access. In between my controllers and the repositories I have the service-layer which handles my business logic.

Now my problem is: whenever I have a bigger controller with more functionality it gets cluttered up with service declarations. Here is a little example of what one of my controllers might look like:

public class ForumController
{
    private readonly ForumAnswerService _answerService;
    private readonly ForumThreadService _threadService;
    private readonly ReputationService _repService;
    private readonly IUnitOfWork _unitOfWork;

    public ForumController()
    {
        _unitOfWork = new EFContextUnitOfWork(new MyContext());
        _threadService = new ForumThreadService(new Repository<ForumThread>(_unitOfWork));
        _answerService = new ForumAnswerService(new Repository<ForumAnswer>(_unitOfWork));
        _repService = new ReputationService(new ReputationRepository(_unitOfWork));
    }

    /* more stuff which uses the services above.. */

    protected override void Dispose(bool disposing)
    {
        if (_unitOfWork != null)
        {
            _unitOfWork.Dispose();
        }
        base.Dispose(disposing);
    }
}

This does not seem so bad right now but I had controllers with even more services and with growing functionality I think the list of services might grow even bigger and it does not seem clean to me.

Here are some ideas I have to tackle the problem:

Only initialize services when I actually need them (i.e. in the methods)

that will reduce the number of class variables but I think it might clutter up the methods and make them big and ugly

Combine multiple services

maybe I just have too many small services and need to combine them, for example combine ForumThreadService and ForumAnswerService to a single ForumService
I don't like that though because of SRP and stuff
that would just move the clutter to the services because I would have to pass multiple repositories to the service

Is that maybe a case where a DI framework would come in handy? Never used one and I only did constructor injection so I don't know..
Nope, that's it. Only got those 3 ideas..

I hope my problem is clear and you guys can point me into the right direction.

Yes, #3 is the one - this is exactly what Dependency Injection/Inversion of Control containers are meant for - removing the assembly part of the services from the main code.

Using an external DI framework will also give you additional benefits, such as stubbing out services while unit testing, allow future extensions to services without changing your main code, etc.

A good start to which framework to use you can find here: How do the major C# DI/IoC frameworks compare?


jQuery animations with only one callback I'm aware of doing that 'magic' inside $.when(). How should I achieve same effect writing code more readable ? I read something about a $.Defferer() that I think should help but I don't know how to use it.

<!DOCTYPE html>

 <meta charset="UTF-8">
 <title>Document</title>

Element 1

Element 2

Element 3

What I want is to run multiple animations and at the end of them to show a message, to do something. I know that I can use $(".elm").animate() and all elm divs will be animated by I use that .each() to set the delay between them (maybe there's another posibility using animation step or progress handlers), but I can't get it to do something at the end of all animations. What I ask is if there's any other implementation that will to same thing and be more code readeable.

I'd say your approach is correct. Since your delay is less than the duration, you can't use a series approach, where one completed animation starts the next. So your setup is probably the simplest you can do.

To make it more readable, the simplest thing would be to split the code up a bit:

$(function () { // same as $(document).read(...)
  // declare some "constants"
  var animationDelay = 800,
      animationDuration = 1000;

  // setup all the animations
  var queue = $(".elm").each(function (i) {
    $(this).delay(i * animationDelay).animate({opacity: 0}, {duration: animationDuration});
  });

  // set up the callback for when they finish
  $.when(queue).then(function () {
    // all animations done
  });
});


Eliminating columns from a matrix where the entries in two rows are equal Is there a way to reduce the computation time in this program that eliminates the columns of a matrix A if for a given column the element of line 4 is equal to the element in row 5:

k=1;
for i=1:4000
   if A(4,i) ~= A(5,i)
      B(:,k)=A(:,i);
      k=k+1;
   end
end

In general, you speed up Matlab programs by computing whole vectors or matrices at once, rather than writing loops. In this case, you can generate a "logical" vector that contains 1 in each column where the corresponding column in A has different values in rows 4 and 5, and 0 in the other columns:

selected_columns = A(4, :) ~= A(5, :);

Then you want to collect all the rows of A, in the columns where the two rows are different:

B = A(:, selected_columns);

I tested the idea in Octave, since I don't actually have Matlab:

octave-3.4.0:7> A = [1, 1, 2, 2, 3, 4, 5;1, 2, 3, 2, 1, 4, 5]
A =

   1   1   2   2   3   4   5
   1   2   3   2   1   4   5

octave-3.4.0:8> selected_columns = A(1, :) ~= A(2, :)
selected_columns =

   0   1   1   0   1   0   0

octave-3.4.0:9> B = A(:, selected_columns)
B =

   1   2   3
   2   3   1

This isn't exactly equivalent to your original code, but it's probably closer to what you meant anyway. Specifically, if A has more than 4000 columns, my code will include the extra ones, while yours will drop them. My code also doesn't care about the initial value of B, while yours can include extra columns from B that weren't in A. Similarly, your code can fail if A is narrower than 4000 columns, or if B is too narrow to hold all the unequal columns from A.


Retrieve data from two different rows in a single SQL query? I've a table with the following data

and I need to find the number of minutes between two jobs (Say A and C).
The following query works but wondering, if there is a simpler way to achieve the same.
  DECLARE @StartTime datetime
    Declare @EndTime datetime
    set @StartTime = (SELECT start_time from table where jobname = 'A' )
    set @EndTime = (SELECT end_time from table where  jobname = 'C' )
    select datediff(minute,@StartTime, @EndTime) numberOfMinutes

You can use Sql Server's window functions LEAD and LAG to get access to two rows in one SELECT statement.
Here's just one way to get the results you're after:
DECLARE @jobs TABLE
(
    Job char(1) primary key,
    Start_time datetime,
    End_time datetime
);

INSERT @jobs VALUES
('A', '2020/01/10 8:00', '2020/01/10 8:15'),
('B', '2020/01/10 8:15', '2020/01/10 8:17'),
('C', '2020/01/10 8:17', '2020/01/10 8:19'),
('D', '2020/01/10 8:19', '2020/01/10 8:53');

SELECT Job,
    Start_time,
    [Other job],
    [Other job's end time],
    DATEDIFF(mi, Start_time, [Other job's end time]) [Diff (min)]
FROM
(
    SELECT
        Job,
        Start_time,
        LEAD(Job, 2) OVER (ORDER BY Start_time) [Other job],
        LEAD(End_time, 2) OVER (ORDER BY Start_time) [Other job's end time]
    FROM @jobs
) AS data
WHERE [Other job] IS NOT NULL

Which shows:
Job  Start_time              Other job Other job's end time    Diff (min)
---- ----------------------- --------- ----------------------- -----------
A    2020-01-10 08:00:00.000 C         2020-01-10 08:19:00.000 19
B    2020-01-10 08:15:00.000 D         2020-01-10 08:53:00.000 38

You can output the difference between two specific jobs by filtering appropriately:
SELECT Job,
    Start_time,
    [Other job],
    [Other job's end time],
    DATEDIFF(mi, Start_time, [Other job's end time]) [Diff (min)]
FROM
(
    SELECT
        Job,
        Start_time,
        LEAD(Job) OVER (ORDER BY Start_time) [Other job],
        LEAD(End_time) OVER (ORDER BY Start_time) [Other job's end time]
    FROM @jobs
    WHERE Job IN ('A', 'C')
) AS data
WHERE Job = 'A'

Which shows:
Job  Start_time              Other job Other job's end time    Diff (min)
---- ----------------------- --------- ----------------------- -----------
A    2020-01-10 08:00:00.000 C         2020-01-10 08:19:00.000 19


Refactoring fixed static adding items to new List I am not sure how to approach this it fixed item adding to the list of object. I have the slot of ads, that have to be prepulated so they could be selected based on front end parameter. So basically few of these ads can be picked on the website and be positioned. How could this list be simplified, because now it is block of repeated code.

private static List<AdItem> GetAdSlotItems()
{
    var s = "";
    return new List<AdItem>()
    {
        new AdItem { Type = AdType.LeftSky, Container = "div-ad-skyscraper-l", Sizes = new List<List<int>> { new List<int>() { 120, 600 }, new List<int>() { 160, 600 } }, Position = new List<Position>() { new Position() { Id = AdType.LeftSky.ToString(), Value = AdPosition.left.ToString() } } },
        new AdItem { Type = AdType.RightSky, Container = "div-ad-skyscraper-r", Sizes = new List<List<int>> { new List<int>() { 120, 600 }, new List<int>() { 160, 600 } }, Position = new List<Position>() { new Position() { Id = AdType.RightSky.ToString(), Value = AdPosition.right.ToString() } } },
        new AdItem { Type = AdType.LeaderBoard, Container = "div-ad-leaderboard", Sizes = new List<List<int>> { new List<int>() { 300, 250 } }, Position = new List<Position>() { new Position() { Id = AdPosition.inline.ToString(), Value = AdPosition.inline.ToString() } } },
        new AdItem { Type = AdType.BillBoard, Container = "div-ad-billboard", Sizes = new List<List<int>> { new List<int>() { 728, 90 }, new List<int>() {970, 250 }, new List<int>() { 300, 50 } }, Position = new List<Position>() },
        new AdItem { Type = AdType.MPU1, Container = "div-ad-rectangle-e1", Sizes =  new List<List<int>> { new List<int>() {300, 250 }, new List<int>() { 300, 600 } }, Position = new List<Position>() { new Position() { Id = AdType.MPU1.ToString(), Value = AdPosition.mpu1.ToString() } } },
        new AdItem { Type = AdType.MPU1, Container = "div-ad-rectangle-e2", Sizes =  new List<List<int>> { new List<int>() {300, 250 }, new List<int>() { 300, 600 } }, Position = new List<Position>() { new Position() { Id = AdType.MPU2.ToString(), Value = AdPosition.mpu2.ToString() } } },
        new AdItem { Type = AdType.MPU1, Container = "div-ad-rectangle-e3", Sizes =  new List<List<int>> { new List<int>() {300, 250 }, new List<int>() { 300, 600 } }, Position = new List<Position>() { new Position() { Id = AdType.MPU3.ToString(), Value = AdPosition.mpu3.ToString() } } },
        new AdItem { Type = AdType.WallPaper, Container = "div-ad-wallpaper", Sizes = new List<List<int>> { new List<int>() { 1, 1 } }, Position = new List<Position>() }
    };
}

Well, we can cut the boilerplate down a little bit with some helpers and changes to your original classes like this:

using System.Collections.Generic;

using static AdPosition;
using static AdType;

public class AdItem
{
    public AdType Type { get; set; }

    public string Container { get; set; }

    public SizeList Sizes { get; set; }

    public PositionList Position { get; set; }
}

public sealed class AdItemList : List<AdItem>
{
    public void Add(AdType type, string container, SizeList sizes, PositionList position)
    {
        this.Add(new AdItem { Type = type, Container = container, Sizes = sizes, Position = position });
    }
}

public sealed class SizeList : List<List<int>>
{
    public void Add(params int[] values)
    {
        var intList = new List<int>(values.Length);

        foreach (var value in values)
        {
            intList.Add(value);
        }

        this.Add(intList);
    }
}

public sealed class PositionList : List<Position>
{
    public void Add(string id, string value)
    {
        this.Add(new Position { Id = id, Value = value });
    }
}

then your method shrinks up like so:

private static AdItemList GetAdSlotItems()
{
    var s = "";
    return new AdItemList
    {
         { LeftSky, "div-ad-skyscraper-l", new SizeList { { 120, 600 }, { 160, 600 } }, new PositionList { { LeftSky.ToString(), left.ToString() } } },
         { RightSky, "div-ad-skyscraper-r", new SizeList { { 120, 600 }, { 160, 600 } }, new PositionList { { RightSky.ToString(), right.ToString() } } },
         { LeaderBoard, "div-ad-leaderboard", new SizeList { { 300, 250 } }, new PositionList { { inline.ToString(), inline.ToString() } } },
         { BillBoard, "div-ad-billboard", new SizeList { { 728, 90 }, { 970, 250 }, { 300, 50 } }, new PositionList() },
         { MPU1, "div-ad-rectangle-e1", new SizeList { { 300, 250 }, { 300, 600 } }, new PositionList { { MPU1.ToString(),mpu1.ToString() } } },
         { MPU1, "div-ad-rectangle-e2", new SizeList { { 300, 250 }, { 300, 600 } }, new PositionList { { MPU2.ToString(),mpu2.ToString() } } },
         { MPU1, "div-ad-rectangle-e3", new SizeList { { 300, 250 }, { 300, 600 } }, new PositionList { { MPU3.ToString(),mpu3.ToString() } } },
         { WallPaper, "div-ad-wallpaper", new SizeList { { 1, 1 } }, new PositionList() }
    };
}


Implicit type class for updating maps With the help of SO I got my code working, but I think it still could be improved, in particular I do not like using asInstanceOf. Is there some way how to avoid it?

  trait UpdatedTrait[A, K, T] {
    def upd(k: K, t: T): A
  }

  trait UpdatedFunctor[A, K, T] {
    def upd(a: A, k: K, t: T): A
  }

  implicit class UpdatedMap[M[KT, TT] <: Map[KT, TT], K, T](a: M[K, T])(implicit f: UpdatedFunctor[M[K, T], K, T]) {
    def upd(k: K, t: T) = {
      f.upd(a, k, t)
    }
  }

  implicit def mapUpdatedFunctor[M[KT, TT] <: Map[KT, TT], K, T] = new UpdatedFunctor[M[K, T], K, T] {
    override def upd(a: M[K, T], k: K, t: T): M[K, T] = {
      a.updated(k, t).asInstanceOf[M[K, T]]
    }
  }

  val m = Map("A" -> "1")
  val sm = collection.immutable.SortedMap("A" -> "1")

  val mt = m.upd("A", "2")
  val smt: collection.immutable.SortedMap[String, String] = sm.upd("A", "2")

  println(mt)
  println(smt)

Using CanBuildFrom will help you here. What you need is an implicit CanBuildFrom[M[K, T], (K, T), M[K, T]] in scope, which will be provided by the standard library if it is possible to construct instances of the correct types.

import scala.collection.generic.CanBuildFrom

implicit def mapUpdatedFunctor[M[KT, TT] <: Map[KT, TT], K, T]
  (implicit cbf: CanBuildFrom[M[K, T], (K, T), M[K, T]]) = {
  new UpdatedFunctor[M[K, T], K, T] {
    override def upd(a: M[K, T], k: K, t: T): M[K, T] = {
      val builder = cbf()
      builder ++= a.updated(k, t)
      builder.result()
    }
  }
}

scala> m.upd("A", "2")
res1: scala.collection.immutable.Map[String,String] = Map(A -> 2)

scala> sm.upd("A", "2")
res3: scala.collection.immutable.SortedMap[String,String] = Map(A -> 2)

I actually answered a similar question recently using Maps and updated on SO.


How to transform true and false to 1 and 0? I use the following code:

  var newValue = ($(event.target).is(":checked")) ? 1 : 0;
  var oldValue = (!$(event.target).is(":checked")) ? 1 : 0;
  sendMessage(this.name, oldValue, newValue);

Is there any better approach?

sendMessage(this.name, +!event.target.checked, +event.target.checked)

Not your sending sendMessage(text, !bool, bool) which is silly just send one bool.

Also $(thing).is(":checked") is stupid (jQuery ಠ_ಠ)


Finding max depth of nested parentheses The following code finds the max depth of nested parentheses in the given string - for example, for {}{}{{}} it outputs 2. If the given string doesn't contain correct sequence of parentheses (like {{}), it outputs "NIE". User enters the length of the string and the string.

My concerns: the algorithm is somewhat slow, and I don't think it's really the best way to solve this problem.

#include <iostream>
#include <string>
#include <algorithm>
using namespace std;

int maxDepth(string S)
{
    int current_max = 0;
    int max = 0;
    int n = S.length();

    for (int i = 0; i< n; i++)
    {
        if ((S[i] == '(') || (S[i] == '{') || (S[i] == '['))
        {
            current_max++;

            if (current_max> max)
                max = current_max;
        }
        else if ((S[i] == ')') || (S[i] == '}') || (S[i] == ']'))
        {
            if (current_max>0)
                current_max--;
            else
                return -1;
        }
    }

    if (current_max != 0)
        return -1;

    return max;
}

int main()
{
    int n, i;
    cin >> n;
    string s;
    cin >> s;
    for (i=0; i<n; i++)
    {

        if (count(s.begin(), s.end(), '(')!=count(s.begin(), s.end(), ')'))
        {

            cout << "NIE\n";

            return 0;

        };

        if (count(s.begin(), s.end(), '{')!=count(s.begin(), s.end(), '}'))
        {

            cout << "NIE\n";

            return 0;

        };

            if (count(s.begin(), s.end(), '[')!=count(s.begin(), s.end(), ']'))
        {

            cout << "NIE\n";

            return 0;

        };

    };
    cout << maxDepth(s) << "\n";
    return 0;
}

Soooooooo many iterations

You can check if the nested braces are valid in a single run through the string. Your code currently does 6n+1 iterations (!!) - and I have no idea what n is there for. You have a loop which does not reference i at all, so at the very least you could just do that part once (which would drop you to just 7 iterations), but really you should do the loop zero times. That's why it's slow. Soooo much unnecessary work!

Wrong output

If maxDepth returns -1, you will print -1 instead of "NIE".

A better algorithm

This is a problem that calls for a stack. When you get an open brace, you add it to the stack (updating maximum size as necessary). When you get a close brace, you check the top value of the stack - if it matches, you're good and you pop. If it doesn't match or the stack is empty - it's invalid and you return -1. If at the end of the string the stack isn't empty, it's invalid and you return -1 too.

That's one single pass. It will also handle the case I mentioned in comments of ({)} which your code would incorrectly return 2.

Code specifics

Avoid using namespace std;
Prefer C++11's range based for loops. In maxDepth(), iterate for (char c : S).
When you're comparing against multiple things like you do, when you're not actually differentiating between the different values, it's clearer to just use a switch statement:

for (char c : S) {
    switch (c) {
    case '(':
    case '{':
    case '[':
       ...
       break;
    case ')':
    case '}':
    case ']':
       ...
       break;
    }
}

In maxDepth(), you don't alter the string, so you should take it by reference to const.
Declare variables when you need them. In main(), you loop over i so declare i in the for-loop itself, not up front. This isn't C.


Class for working with users and passwords in a PHP CMS I'm working on CMS in PHP just for fun. Right now I'm trying to implement class for working with users/passwords. I had been reading lot of articles about hashing and as far as I understood crypt() function are the best(most secure) for now.

class Users {
    private static $key, $table, $logtable;

    public static function init($key) {
        self::$key = $key;
        self::$table = DBPREFIX.$key.'s';
        self::$logtable = self::$table.'_access_log';
    }

    public static function isLogged() {
        if(isset($_SESSION[self::$key])) {
            if(isset($_SESSION[self::$key]['uid'])) {
                $uid = intval($_SESSION[self::$key]['uid']);
                $user = DB::select()->from(self::$table)->where('id', '=', $uid)->execute()->object();
                if(count($user) > 0) {
                    return true;
                }
                else {
                    return false;
                }

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

    public static function userExist($login) {
        $user = DB::select()->from(self::$table)->where('login', '=', $login)->execute()->object();
        if(count($user) > 0) {
            return true;
        }
        else {
            return false;
        }
    }

    public static function getUser($login) {
        $user = DB::select()->from(self::$table)->where('login', '=', $login)->execute()->object();
        if(count($user) > 0) {
            return $user[0];
        }
        else {
            return false;
        }
    }

    public static function generateHash($password) {
        $salt = self::generateSalt();
        $hashedPassword = crypt($password, $salt);
        return array('hashed_password' => $hashedPassword, 'salt' => $salt);
    }

    private static function generateSalt() {
        $blowfishPre = '$2y$10$';
        $blowfishEnd = '$';
        $allowedChars ='ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789./';
        $charsLength = strlen($allowedChars);
        $saltLength = 20;
        $salt = "";
        for($i=0; $i < $saltLength; $i++)
        {
            $salt .= $allowedChars[mt_rand(0,$charsLength)];
        }
        $bcrypt_salt = $blowfishPre . $salt . $blowfishEnd;
        return $bcrypt_salt;
    }

    public static function comparePasswords($input, $uid) {
        $user = DB::select()->from(self::$table)->where('id', '=', $uid)->execute()->object();
        if(count($user) > 0) {
            $user = $user[0];
            $password = $user->password;
            return crypt($input, $password) === $password;
        }
        else {
            return false;
        }
    }

    public static function userExit() {
        if(isset($_SESSION[self::$key])) {
            unset($_SESSION[self::$key]);
        }
    }

    public static function changePassword($password, $uid) {
        $newPassword = self::generateHash($password);
        if(isset($newPassword['hashed_password']) && isset($newPassword['salt'])) {
            DB::update(self::$table)->set(array('password' => $newPassword['hashed_password'], 'salt' => $newPassword['salt']))->where('id', '=', $uid)->execute();
        }
    }

    public static function updateTime($uid) {
        DB::update(self::$table)->set(array('last_login' => time()))->where('id', '=', $uid)->execute();
    }

    public static function writeAccessLog($uid, $success) {
        if($success) {
            DB::insert(self::$logtable)->set(array('admin_id' => $uid, 'date' => time(), 'success' => '1'))->execute();
        }
        else if(!$success) {
            DB::insert(self::$logtable)->set(array('admin_id' => $uid, 'date' => time(), 'success' => '0'))->execute();
        }
    }
}

Please, take a look and let me know if I doing something wrong.

New code (optimizations/simplifications, see comments in code for explanations):

class Users {
    private static $key, $table, $logtable;

    public static function init($key) {
        self::$key = $key;
        self::$table = DBPREFIX.$key.'s';
        self::$logtable = self::$table.'_access_log';
    }

    public static function isLogged() {
        if(isset($_SESSION[self::$key]) and isset($_SESSION[self::$key]['uid'])) { # Combined ifs
            $uid = intval($_SESSION[self::$key]['uid']);
            $user = DB::select()->from(self::$table)->where('id', '=', $uid)->execute()->object();
            return count($user) > 0; # Simplified
        }
        return false; # Removed unnecesary elses
    }

    public static function userExist($login) {
        $user = DB::select()->from(self::$table)->where('login', '=', $login)->execute()->object();
        return count($user) > 0; # Simplified
    }

    public static function getUser($login) {
        $user = DB::select()->from(self::$table)->where('login', '=', $login)->execute()->object();
        if(count($user) > 0) {
            return $user[0];
        }
        return false; # Removed unnecessary else
    }

    public static function generateHash($password) {
        $salt = self::generateSalt();
        $hashedPassword = crypt($password, $salt);
        return array('hashed_password' => $hashedPassword, 'salt' => $salt);
    }

    private static function generateSalt($blowfishPre = '$2y$10$', $blowfishEnd = '$', $allowedChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789./', $saltLength = 20) { # Added arguments for future customization
        $charsLength = strlen($allowedChars);
        $salt = "";
        for($i=0; $i < $saltLength; $i++)
        {
            $salt .= $allowedChars[mt_rand(0,$charsLength)];
        }
        return $blowfishPre . $salt . $blowfishEnd; # Removed unnecessary variable
    }

    public static function comparePasswords($input, $uid) {
        $user = DB::select()->from(self::$table)->where('id', '=', $uid)->execute()->object();
        if(count($user) > 0) {
            $user = $user[0];
            $password = $user->password;
            return crypt($input, $password) === $password;
        }
        return false; # Removed unnecessary else
    }

    public static function userExit() {
        if(isset($_SESSION[self::$key])) {
            unset($_SESSION[self::$key]);
        }
    }

    public static function changePassword($password, $uid) {
        $newPassword = self::generateHash($password);
        if(isset($newPassword['hashed_password']) && isset($newPassword['salt'])) {
            DB::update(self::$table)->set(array('password' => $newPassword['hashed_password'], 'salt' => $newPassword['salt']))->where('id', '=', $uid)->execute();
        }
    }

    public static function updateTime($uid) {
        DB::update(self::$table)->set(array('last_login' => time()))->where('id', '=', $uid)->execute();
    }

    public static function writeAccessLog($uid, $success) {
        DB::insert(self::$logtable)->set(array('admin_id' => $uid, 'date' => time(), 'success' => $success ? '1' : '0'))->execute(); # Inlined success var.
    }
}


Is this user login secure? When it come to security I try to be to better as possible but I don't have the knowledge.

According to what I read on-line my following code should be good but I could use some of your comment/critic/fixes

Here is a simple class just to example how I would do a login.  Does it look secure enough?

class UserClass
{
private $dbCon = null;
public $Error = '';
public function __construct(PDO $dbCon)
{
    $this->dbCon = $dbCon;
}
public function login($Email,$Password,$RegisterCustomerSession = FALSE)
{
    $GetSalt = $this->dbCon->prepare('SELECT id,salt,hashPass FROM `customer` WHERE `email` = :Email');
    $GetSalt -> bindValue(':Email',$Email);
    $GetSalt -> execute();
    if($GetSalt -> rowCount() == 0)
    {
        $this->Error = "No customer is registered with that email";
        return false;
    }
    elseif($GetSalt->rowCount()>0)
    {
        $CustomerInfo = $GetSalt->fetch(PDO::FETCH_ASSOC);
        if(sha1($Password.$CustomerInfo['salt'])==$CustomerInfo['hashPass'])
        {
            if($RegisterCustomerSession)
                self::RegisterAllCustomerSession($CustomerInfo['id']);
            return true;
        }
        else
        {
            $this->Error = "Invalid Password";
            return false;
        }
    }
}
public function SetPassword($CustomerId,$Password)
{
    $Salt = self::CreateSalt(16);
    $HashPass = sha1($Password.$Salt);

    $SetPasswordAndSalt = $this->dbCon->prepare('UPDATE `customer` SET `hashPass` = :HashPass,`salt` = :Salt WHERE `id` = :CustomerId;');
    $SetPasswordAndSalt -> bindValue(':CustomerId',$CustomerId);
    $SetPasswordAndSalt -> bindValue(':Salt',$Salt);
    $SetPasswordAndSalt -> bindValue(':HashPass',$HashPass);
    try{
        $SetPasswordAndSalt ->execute();
        return true;
    }catch(PDOException $e){echo $e->getMessage(); return false; }

}
private function CreateSalt($HowLong = 16)
{
    $CharStr = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-=+_<>';
    $ReturnStr = '';
    for($i = 0;$i<$HowLong;$i++)
    {
        $ReturnStr .= $CharStr{mt_rand(0,77)};
    }
    return $ReturnStr;
}
private function RegisterAllCustomerSession($CustomerId)
{
    // some code.
}
}

I'm not sufficiently familiar with PDO to say more on your database access than that you don't seem to have any injection vulnerabilities. However, there are a few things which I would do differently.

Salt entropy

There are two things which strike me as odd about your salt generation. Firstly, using a base-77 encoding. If you configure your database correctly then you can use the full 8 bits of each byte of salt. That's more efficient, avoids possible bugs with generating a number in the wrong range (how sure are you that strlen($CharStr) === 78?), and is closer to the assumptions made when analysing salted hashing.

Secondly, while mt_rand is better than rand it's not a cryptographic PRNG. The best portable secure PRNG in PHP is openssl_random_pseudo_bytes; if you're not planning to deploy to Windows then you could also get good entropy from /dev/random.

Hash

SHA is not generally recommended as a hash for passwords. The current conventional wisdom is that you want password hashing to be slow, and should use either bcrypt or scrypt. If you insist on SHA then you should use it as a component of PBKDF2.

Account existence oracle

There are two schools of thought on telling people "That username doesn't exist". The usability argument is that it's preferable to tell someone that they got their username wrong. The security argument is that you should always say "Either your username doesn't exist or you got your password wrong" to prevent people identifying accounts which do exist and then trying to brute-force their passwords. (Anti-brute-forcing techniques is a separate issue which I'm not going to address in detail).

If you favour the security argument over the usability argument then you need to avoid telling people indirectly that the username doesn't exist. That means that if $GetSalt -> rowCount() === 0 you should still do a hashing operation to avoid a quicker page load which leaks information.

Minor style points

if($GetSalt -> rowCount() == 0)
    ...
elseif($GetSalt->rowCount()>0)

I see three things wrong here:

What other possibilities are there? Shouldn't that elseif just be an else?
Why == instead of ===?
Pick a style for use of whitespace and stick with it.


Chrome extension -- replace words on a page from a large dictionary This works, but is very very slow. Any recommendations on better, less trivial ways of accomplishing this would be appreciated. With a smaller dictionary it's fine, but the dictionary is about 80 different mappings. I also hardly ever work with JS so any  input would be great. Obviously I have not included the dictionary.

var TEXTNODE = 3;

var elements = document.getElementsByTagName('*');

var wordMap = [
    [['word'], 'different word'],
    [['phrase'], 'something else'],
    [['one'], 'two'],

    /* etc for 80 lines */

];

function createRegularExpression(words)
{
    return new RegExp( '\\b(' + words.join('|') + ')s?\\b', 'gi');
}

function replaceText(text)
{
    for (var ii = 0; ii < wordMap.length; ii++)
    {
        var newText = text.replace(createRegularExpression(wordMap[ii][0]), wordMap[ii][1]);
        if (text != newText)
        {
            return newText;
        }
    }

return null;

}

for (var ii = 0; ii < elements.length; ii++)
{
    var element = elements[ii];

    for (var jj = 0; jj < element.childNodes.length; jj++)
    {
        var node = element.childNodes[jj];

        if (node.nodeType === TEXTNODE)
        {
            var text = node.nodeValue;
            var lastText = text;
            var newText = text;;

            /* Have to keep looping because it will only replace the first occurence */

            while (newText != null)
            {
                lastText = newText;
                newText = replaceText(lastText);
            }
            if (lastText != text)
            {
                element.replaceChild(document.createTextNode(lastText), node);
            }
        }
    }
}

UPDATE: Thanks to the awesome response I was able to rework this and it works lighting fast. I'll eventually look into that optimization. Thank you!!!!!!!! Updated code:

var wordMap = {
'word': 'new word',
'this is a phrase': 'new phrase',
'map this': 'to this'

//...
};

var regex = new RegExp('\\b(' + Object.keys(wordMap).join('|') + ')\\b', 'ig');

/* tree walker */
var treeWalker = document.createTreeWalker (
    document.body,
    NodeFilter.SHOW_TEXT,
    null,
    false
);

/* text nodes */
var textNodes = [];

while (treeWalker.nextNode())
{
    textNodes.push(treeWalker.currentNode);
}

/* iterate text nodes and modify in place */
for (var i = 0, len = textNodes.length; i < len; i++)
{
    textNodes[i].nodeValue = textNodes[i].nodeValue.replace(regex,
        function(match)
        {
            var replacementWord = wordMap[match.toLowerCase()];

            /* match capitalization -- wordMap is all lowercase */

            if (match != match.toLowerCase())
            {
                if (match == match.toLowerCase().charAt(0).toUpperCase())
                {
                    replacementWord = replacementWord.charAt(0).toUpperCase() + replacementWord.slice(1);
                }
                else if (match == match.toUpperCase())
                {
                    replacementWord = replacementWord.toUpperCase();
                }
            }

            console.log('replacing ' + match + ' with ' + replacementWord);
            return replacementWord;
        }
    );
}

Your logic currently does a lot more work that it needs to do.

First, you are iterating every element in DOM and then iterating on every child element of those (which is actually duplicate work, since all nested tags would already be present in elements). This is especially problematic in that you are really only interested in text nodes.  So, your first order of business, is to get only those nodes you care about.

Perhaps check out this Stack Overflow post on this topic. I personally like the TreeWalker approach in the accepted answer.

Now that you have a good flattened representation of these text nodes, you can just iterate through and directly modify them.

// build word map
var wordMap = {
   'match value': 'replace value',
   // etc.
}

// build single regex
var regex = new RegExp(Object.keys(wordMap).join("|"),"gi");

// get text nodes
var textNodes = ...; // derive similar to linked SO question above.

// iterate text nodes and modify in place
for (var i = 0, len = textNodes.length; i < len; i++) {
    textNodes[i].nodeValue = textNodes[i].nodeValue.replace(regex, function(match) {
        return wordMap[match];
    });
}

This example above would have you modifying the nodes in place rather than having to do any DOM replacements.

You could also potentially have some savings by dumping the regex approach to string search altogether in favor of simply iterating through the string a single character at a time comparing against key/property values in your word map, to see if any replacement strings are potentially starting or continuing.  You would like need to build a trie structure to give efficient lookup against your dictionary. This is certainly more complex from an implementation standpoint, but could be an optimization you need to make if you find the regex approach is just not operating fast enough for you.

You might find this interesting reading on tries in javascript - http://ejohn.org/blog/javascript-trie-performance-analysis/


Object wrapper/parameter injection module I made an object wrapper/parameter injector and I hope it will be useful. I made it for a project I am working on but then decided to try polishing it a bit.

The use case is for wrapping objects such that you can inject parameters into their methods by calling the wrapper as a function. I was heavily inspired by AngularJS and their use of parameter injection to almost create a DSL with their framework.

Questions:

Is this a well-designed module?
Is my code legible?
Am I over looking something that might lead to bugs later?
Is something like this safe to use?

Repo

negotiator.js

# A small tool for proxying objects behind a wrapper that can inject
# parameters into their methods.

# Extract parameter names from a function,
# position is given by position this array.
if module
  _ = require('underscore');
else
  _ = window._

utils = {};

utils.parameterNames = (func) ->

  funcArgs = func.toString().split('(')[1].split(')')[0].split(',')

  return (k.trim() for k in funcArgs)

# Inject parameters from a context object into a function.
# Passed parameters are supplied by the 'params' argument.
utils.injectAndApply =(func, parameters, context, target) ->

  signature = utils.parameterNames func
  parameters = [] if _.isEmpty(parameters)

  for position, name of signature
    parameters[position] = context[name] if context[name]?

  parameters.length = signature.length;
  return func.apply target ? {}, parameters

# Constructor for proxy object.
utils.Proxy = (real) ->

  @$real = real
  self = this

  for key, method of real when typeof method is 'function'
    do (method, key) ->
      self[key] = ->
        utils.injectAndApply method, arguments, @$context ? {}, @$real

  return this

# Build a context object from an arguments list.
utils.buildContextFromParams = (func, parameters) ->
  signature = utils.parameterNames func
  context = {}
  for key, value of parameters
    context[signature[key]] = value

  return context

# This is the circular wrapper that returns a version of itself with
# a set context.
utils.innerWrapper = (proxy, templateFunction, parameters) ->
  wrapper = ->
    utils.innerWrapper(proxy,templateFunction,arguments);

  context = utils.buildContextFromParams templateFunction, parameters
  context.$real = proxy.$real
  context.$wrapper = wrapper
  context.$proxy =  proxy
  context.$context = context;
  utils.injectAndApply templateFunction, parameters, context, proxy
  wrapper.$context = context;

  wrapper.__proto__ = proxy;

  return wrapper

# Returns a function that wraps object.
# not intended to be called as a constructor
utils.makeWrapper = (real, templateFunction) ->
  proxy = new utils.Proxy real

  return utils.innerWrapper proxy, templateFunction, []

negotiator = utils.makeWrapper
negotiator.utils = utils;

if module
  module.exports = negotiator
else
  window.negotiator = negotiator

return

Looks quite good.
Nevertheless here are some thoughts:

Parameter Names

You might get problems when calling utils.parameterNames without arguments. E.g. func.toString() would fail if func is undefined.
Moreover you'll get an array with one entry  (['']) if you pass a function with no arguments (utils.parameterNames(->)) where you might expect an empty array.
You could change your implementation to this:

utils.parameterNames = (func=->) ->
  funcArgs = func.toString().split('(')[1].split(')')[0].split(',')
  (t for k in funcArgs when (t=k.trim()) isnt '')

Here is an other implementation (source) using an regular expression

getArgumentNames = (fn=->) ->
  args = fn.toString().match ///
    function # start with 'function'
    [^(]* # any character but not '('
    \( # open bracket = '(' character
    ([^)]*) # any character but not ')'
    \) # close bracket = ')' character
  ///
return [] if not args? or (args.length < 2)
args = args[1]
args = args.split /\s*,\s*/
(a for a in args when a.trim() isnt '')

We could also write s.th. like this:

fnRgx = ///
  function # start with 'function'
  [^(]* # any character but not '('
  \( # open bracket = '(' character
  ([^)]*) # any character but not ')'
  \) # close bracket = ')' character
///

argRgx = /([^\s,]+)/g

parameterNames = (fn) ->
  (fn?.toString().match(fnRegex)?[1] or '').match(argRgx) or []

The performance depend on the browser but the last one seems to be pretty fast (see benchmarks)

Expressions

In CoffeeScript every thing is an expression so you can turn this

if module
  _ = require('underscore');
else
  _ = window._

into this

_ = if module then require('underscore') else window._

Iteration

This:

for position, name of signature
  parameters[position] = context[name] if context[name]?

could also be written in that way:

for position, name of signature when context[name]?
  parameters[position] = context[name]

or even:

parameters[position] = context[name] for position, name of signature when context[name]?

It compiles all to the same JS. I'd prefer the second variant.


Efficient Implementation of functional and Lazy evaluation in C++ I am creating a c++ library implementing Java Functional Programming alike interface. In short, the code will look like this:

vector<string> buffer = ... ; // A buffer contains some strings
new IntStream(0, 100).map([](int a){
    return (a * 0x2344DDEF) & 0xF;
}).map([=](int a) {
    return buffer[a];
}).foreach([](string a) {
    cout << a << '\n';
});

Now I want to support parallel evaluation. For the example above, I want to get 100 execution tasks and send them to a thread pool. To do this, I have created EvalOp classes. The stream returns a list of EvalOp objects. They will only perform the actual computation when you invoke EvalOp::eval

template <typename T>
class EvalOp {
public:
    virtual T eval() = 0;
};

template <typename FROM, typename TO>
class TransformOp : public EvalOp<TO> {
public:
    TO eval() override {
        return mapper_(previous_->eval());
    }
protected:
    unique_ptr<EvalOp<FROM>> previous_;
    function<TO(FROM&)> mapper_;
};

template <typename T>
class Stream {
public:
    virtual bool isEmpty() = 0;
    virtual EvalOp<T> next() = 0;
    Stream<N> map(function<N(T)> mapper) {
        return new MapStream<N,T>(this, mapper);
    }
}

template <typename FROM, typename TO>
class MapStream : public Stream<TO> {
protected:
    Stream<FROM>* previous_;
    function<TO(FROM)> mapper_;
public:
    EvalOp<TO> next() override {
        return new TransformOp<FROM, TO>(previous_->next(), mapper_);
    }
}

My stream will now return a bunch of EvalOp objects, which you can throw in a thread pool.

This code gets me the correct result. But as it creates many wrapper classes (the EvalOps), the execution is slower. I did a benchmark of the following two tasks:

uint32_t __attribute__ ((noinline)) hash1(uint32_t input) {
    return input * 0x12345768;
}

uint32_t __attribute__ ((noinline)) hash2(uint32_t input) {
    return ((input * 0x2FDF1234) << 12) * 0x23429459;
}

uint32_t sum = 0;

void summer(uint32_t input) {
    sum += input;
}

BENCHMARK(StreamBenchmark, Serial)(State& state) {
    for(auto _:state) {
        for(int i = 0 ; i < 10000; ++i)
            sum += hash2(hash1(i));
        }
    }
}

BENCHMARK(StreamBenchmark, Wrapper)(State& state) {
    for(auto _:state) {
        IntStream stream(0, 10000);
        stream.map(hash1).map(hash2).foreach(summer);
    }
}

From the benchmark result, I see for each element, only 1ns is spent on actual computation and 40ns overhead is spent on the Stream and EvalOp. I am looking for some suggestions to make a more efficient design. Thank you very much!

I think this question may be off-topic, as it does not consist of complete working code to be reviewed. I spent (too long) trying to get it to compile, and got as far as this: https://godbolt.org/z/qCE9S8

But you have a lot of problems with the current design. Most notably, you're using raw new all over the place, which creates pointers to the heap; but you aren't actually using the correct syntax to refer to those pointers. For example:

Stream<N> map(function<N(T)> mapper) {
    return new MapStream<N,T>(this, mapper);
}

Here new MapStream<...>() yields a value of type MapStream<N,T>*, but you're trying to return it as if it were a Stream<N> object. This flatly will not compile.

You could consider changing this return type to std::unique_ptr<Stream<N>> (and using make_unique), but that still won't really work for your use-case, because then you'll have to change this line:

stream.map(hash1).map(hash2).foreach(summer);

to:

stream.map(hash1)->map(hash2)->foreach(summer);

because now map returns a pointer. And worse, there's no way for the MapStream object itself to transfer its own ownership into the previous_ member of the next MapStream object in the chain. You end up with a bunch of temporary unique_ptrs, all linked together by raw pointers which will dangle as soon as the current full-expression finishes. That's okay for your benchmark, but it won't work at all in practice.

You might consider looking at a type erasure design, so that you could keep using Stream<int> as a value type (not a polymorphic base class, no visible pointers) but give it behavior that appeared polymorphic at run time.


Reimplementing enumerate() to produce a sequence or iterator Apparently it is possible to implement enumerate so it produces a sequence when given a sequence, and produces an iterable otherwise. Sequence can be safely reversed. This works by replacing the builtin function. Other similar functions could be made this way, too.

class EnumeratedSequence:
    def __init__(self, items):
        self.items = items

    def __getitem__(self, index):
        return (index,self.items[index])

    def __len__(self):
        return len(self.items)

def enumerate(items):
    if hasattr(items, '__getitem__'):
        print 'Sequence detected'
        return EnumeratedSequence(items)
    else:
        print 'Iterator detected'
        return __builtin__.enumerate(items)

print list(reversed(enumerate('abcdef')))
print list(enumerate(reversed('abcdef')))

Which outputs:

Sequence detected
[(5, 'f'), (4, 'e'), (3, 'd'), (2, 'c'), (1, 'b'), (0, 'a')]
Iterator detected
[(0, 'f'), (1, 'e'), (2, 'd'), (3, 'c'), (4, 'b'), (5, 'a')]

This was a solution to a problem of reversed(enumerate(...)) failing.

First of, depending on the implementations, __builtin__ may not be directly available (it seems to be in IPyton, but not in CPython). You should import __builtin__ beforehand so __builtin__ will always be in the global namespace. CPython implementation also have __builtins__ (with an s) directly available without imports.

Second, you should remove the print statements in enumerate. I guess it was for demonstrations purposes but if you want to use that in real code it will be pure noise.

Other than that, I’d just add a docstring to enumerate to explain the new behavior.

However, since you are using Python 2, you can take advantage of some features that were removed in Python 3. Namely the difference between zip and itertools.izip. The former returns a list, the latter an iterator: pretty much what you’re after. And since enumerate is somewhat equivalent to zipping itertools.count with its parameters, it is very simple to achieve what you’re looking for:

from itertools import count, izip

def enumerate(iterable):
    zip_ = zip if hasattr(iterable, '__getattr__') else izip
    return zip_(count(), iterable)


Squashing multiplication identity in an expression I'm learning Haskell, and one exercise was to implement squashing of identity-multiplications, e.g.:

\$5 + (2*1) => 5 + 2\$

Here is my implementation:

squashMulId :: (Ring a, Eq a) => RingExpr a -> RingExpr a
squashMulId (Mul x y)
    | x == (Lit (mulId))    = squashMulId y
    | y == (Lit (mulId))    = squashMulId x
    | otherwise             = if sqX == x && sqY == y then (Mul sqX sqY) else squashMulId (Mul sqX sqY)
                              where sqX = squashMulId x
                                    sqY = squashMulId y
squashMulId (AddInv x)      = (AddInv (squashMulId x))
squashMulId (Add x y)       = (Add (squashMulId x) (squashMulId y))
squashMulId x               = x

squashMulIdInt :: RingExpr Integer -> RingExpr Integer
squashMulIdInt = squashMulId

tests_squash = test [
                      "right" ~: (Add (Lit 5) (Lit 2)) ~=? squashMulIdInt (Add (Lit 5) (Mul (Lit 2) (Lit 1)) )
                     ,"left" ~: (Add (Lit 5) (Lit 2)) ~=? squashMulIdInt (Add (Lit 5) (Mul (Lit 1) (Lit 2)) )
                     ,"recursive" ~: (Add (Lit 5) (Lit 2)) ~=? squashMulIdInt (Add (Lit 5) (Mul (Lit 1) (Mul (Lit 2) (Mul (Lit 1) (Lit 1)))))
                    ]

How can I improve on this implementation? Is it idiomatic? Can it be written in a more succinct way?

The following code/definitions were given:

{-# OPTIONS_GHC -Wall #-}
module SQMULID where

import Data.Char
import Data.List
import Data.Maybe
import Test.HUnit
import Test.QuickCheck
import Test.QuickCheck.Gen

class Ring a where
  addId  :: a            -- additive identity
  addInv :: a -> a       -- additive inverse
  mulId  :: a            -- multiplicative identity

  add :: a -> a -> a     -- addition
  mul :: a -> a -> a     -- multiplication

-- | A datatype for storing and manipulating ring expressions.
data RingExpr a = Lit a
                | AddId
                | AddInv (RingExpr a)
                | MulId
                | Add (RingExpr a) (RingExpr a)
                | Mul (RingExpr a) (RingExpr a)
  deriving (Show, Eq)

instance Ring (RingExpr a) where
  addId  = AddId
  addInv = AddInv
  mulId  = MulId

  add = Add
  mul = Mul

-- The canonical instance for integers:
instance Ring Integer where
  addId  = 0
  addInv = negate
  mulId  = 1

  add = (+)
  mul = (*)

You are doing both too much work and not enough.

You are doing too much work because you don't need to repeatedly squash units in case the first pass has returned a modified expression.
You are not doing enough work because you are not squashing the units coming from MulId rather than Lit mulId

Here is a solution which one could argue is more idiomatic as well as dealing with the two shortcomings I pointed out earlier:

We start by implementing the recursion pattern first; it is a quite common thing to do in Haskell as it helps make it easier to understand from first sight how a function works (what does this function do? Oh! It goes by recursion on the structure! Let's see what the different cases are...).

To do that, you define a function fold taking one argument per constructor of the datatype and having a given return type (here b). The arguments are functions taking the same arguments as the constructors they correspond to except that the recursive occurences of RingExpr a have been replaced by the return type b (i.e. we assume we already know the result for the recursive subcomputations!)

fold :: (a -> b)      -> -- Lit    :: a -> RingExpr a
        b             -> -- AddId  :: RingExpr a
        (b -> b)      -> -- AddInv :: RingExpr a -> RingExpr a
        b             -> -- MulId  :: RingExpr a
        (b -> b -> b) -> -- Add    :: RingExpr a -> RingExpr a -> RingExpr a
        (b -> b -> b) -> -- Mul    :: RingExpr a -> RingExpr a -> RingExpr a
        RingExpr a -> b
fold litb addIdb addInvb mulIdb addb mulb = go
  where go (Lit a)    = litb a
        go AddId      = addIdb
        go (AddInv e) = addInvb $ go e
        go MulId      = mulIdb
        go (Add e f)  = go e `addb` go f
        go (Mul e f)  = go e `mulb` go f

The function you are looking to define is now a simple instance of the recursion pattern we just described: for the cases which are not Mul, we simply use the corresponding constructor but in the case of Mul we do the squashing:

squashMulId :: (Ring a, Eq a) => RingExpr a -> RingExpr a
squashMulId = fold Lit AddId AddInv MulId Add squashing
  where
    squashing a b
      | a == Lit mulId || a == MulId = b
      | b == Lit mulId || b == MulId = a
      | otherwise                    = Mul a b