Function dependence on iteration variable of another function Consider two versions of implementing the same functionality.

Version 1 -
Functions calculateA(),calculateB() and calculateC() dependent on iteration variable of calculateAll().

class Box
{
    void calculateAll()
    {
        for(unsigned i = 0; i < _values.size(); i++)
        {
            calculateA(i);
            calculateB(i);
            calculateC(i);
        }

    }

    A calculateA(unsigned i) const
    {
        //Calcualate A based on _values
    }

    B calculateB(unsigned i) const
    {
        //Calcualate B based on _values
    }

    C calculateC(unsigned i) const
    {
        //Calcualate A based on _values
    }

    std::vector<unsigned>           _values;
};

Version 2 - Functions calculateA(),calculateB() and calculateC() are not dependent on calculateAll()

class Box
{
    void calculateAll()
    {
        calculateA();
        calculateB();
        calculateC();
    }

    A calculateA() const
    {
        for(const auto& value: _values)
        {
            //Calcualate A based on _values
        }
    }

    B calculateB() const
    {
        for(const auto& value: _values)
        {
            //Calcualate B based on _values
        }
    }

    C calculateC() const
    {
        for(const auto& value: _values)
        {
            //Calcualate C based on _values
        }
    }

    std::vector<unsigned>           _values;
};

Which version is more cleaner? Which version is better performance wise? Is there a better implementation that these?

This largely depends on your use case. If there are situations that call for only calculating based off of certain values or subsets of values, then the second version should be preferred, but if you are always going be calculating all values, then the first should be preferred.


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

class Hashtable:
    def __init__(self,size):
        self.size = size
        self.mainList = [[] for i in range(0,1)for j in range(self.size)]
    def index(self,key):
        #To get the index number.
        asciiValue = 0
        for i in key:
            asciiValue = asciiValue + ord(i)
        index = asciiValue % self.size
        return index
    def put(self,key,value):
        #To add value to the list.
        index = self.index(key)
        self.mainList[index].append((key,value))
    def get(self,key):
        #To get value from the list
        index = self.index(key)
        for i in range(0,len(self.mainList[index])):
            if self.mainList[index][i][0] == key:
                return self.mainList[index][i][1]
        return "This key cannot be found."

"magic" method

If you're  learning the data structures in Python, I would read about the python data model and Collections abstract base classes to see what magic methods you can/should implement. A Hashmap is a mapping, so it should implement __getitem__, __iter__, __len__, __contains__, keys, items, values, get, __eq__, and __ne__

return vs exception

In case your mapping does not find a key, it returns "This key cannot be found.". This means users of your code should check against this sentinel value when retrieving something. What if that is the value they want to store in this mapping? The correct way to handle a missing key, is to raise an Exception. More specifically a KeyError

range

for i in range(0,1) is equivalent to range(1), so it only yields 0, which means that in this list comprehension it does nothing.

[[] for j in range(self.size)]

Would hav achieved the same. [[]] * self.size would not have worked, since the reference to the same inner list would have been copied.

variable names

size is not the size of the mapping, but the size of the hash table, so this name might be confusing. Moreover, this should not be a public variable of the, so _hashtable_size would be more appropriate.

According to PEP-8, variable names should be snake_case, so mainList would be _main_list

get

For a dict and a mapping, get has another argument default, which gets returned if the key is missing

iteration

In python, it is seldomly necessary to iterate over the index , like you do in for i in range(0,len(self.mainList[index])):

for dict_key, value in self.mainList[index]:
    if dict_key == key:
        return value

achieves the same, but is a lot more clearer and concise

docstrings

Python has a convention on how to document a method. It's called docstrings. Instead of a #, you use a """

def put(self, key, value):
    """To add value to the list."""

class Hashtable:
    def __init__(self, size):
        """explanation what this class does, and what the argument means"""
        self._hastable_size = size
        self._main_list = [[] for j in range(size)]

    def _index(self, key):
        # To get the index number.
        return sum(ord(i) for i in key) % self._hastable_size

    def __contains__(self, key):
        index = self._index(key)
        return any(dict_key == key for dict_key in self._main_list[index])

    def put(self, key, value):
        """To add value to the list."""
        if key in self:  # calls self.__contains__(key)
            raise ValueError(f"<{key}> already present")
            # you can also choose to overwrite the already present value
        index = self._index(key)
        self._main_list[index].append((key, value))

    __setitem__ = put

    def __getitem__(self, key):
        if key not in self:
            raise KeyError(f"<{key}> not present")
        index = self._index
        for dict_key, value in self._main_list[index]:
            if dict_key == key:
                return value

    def get(self, key, default=None):
        # To get value from the list
        try:
            return self[key]
        except KeyError:
            return default

    def __len__(self):
        return sum(len(sublist) for sublist in self._main_list)

    ...


the better method of I/O from file I am very new to python (probably too new to consider efficiency etc.). But I am bit confused about which method to use while reading a file for post-processing.
Here I am reading it line-by-line. There is other methods also for reading the file(e.g infile.read()), one of them is to load the complete file in memory.
Can someone kindly discuss this?

My code in current condition is:

#!/usr/bin/python
import sys,math,re
try:
  inf=sys.argv[1]
except:
    print "usage:", sys.argv[0], "argument"; sys.exit(1)

infile=sys.argv[1]; oufile=sys.argv[2]
ifile=open(infile, 'r'); ofile=open(oufile, 'w')
pattern=r'species,subl,cmp=\s{4}(.*)\s{4}(.*)\s{4}(.*)\s{3}s1,torque=(.{12})(.{12})'

ssc1=[];ssc2=[];ssc3=[]; s1=[]; t=[]
for line in ifile:
  match = re.search(pattern, line)
  if match:
    ssc1.   append(int(match.group(1)))
    s1.     append(float(match.group(4)))
    t.      append(float(match.group(5)))

for i in range(len(t)):
  print('%g %12.5e %12.5e' % (ssc1[i], s1[i], t[i]))

ifile.close(); ofile.close()

First, Where are you writing in ofile?

Second, ;) you can use with statement to opening and closing files like below:

pattern=r'species,subl,cmp=\s{4}(.*)\s{4}(.*)\s{4}(.*)\s{3}s1,torque=(.{12})(.{12})'
infile=sys.argv[1]; outfile=sys.argv[2]
with open(infile, "r") as ifile, open(outfile, "w") as ofile:
    for line in ifile:
        match = re.match(pattern, line)
        if match:
            ofile.write('%g %12.5e %12.5e' % (int(match.group(1)), float(match.group(4)), float(match.group(5))))


Finding matching Node instances in an array - running slow I have encountered a performance issue in Swift. I compared the same method between C# and Swift: Swift is much slower than C#.

Here is the code I'm working with. Could some one help me to speed it up in Swift?

CSharp version (for reference only - it takes 73 ticks):

var nodes = new Node[328];
            for (int i = 0; i < nodes.Length; i++)
            {
                nodes[i] = new Node();
            }

            var oneNode = new Node();
            var count = 1;

            Stopwatch sw = Stopwatch.StartNew();
            foreach(var node in nodes)
            {
                if (oneNode.Equals(node))
                {
                    count++;
                }
            }
            sw.Stop();

            Console.WriteLine(sw.ElapsedMilliseconds);

Swift version (it takes 1.78 ms):

import Foundation

public class Node : Equatable {
    var cost : Int = 0
    var x : Int = 100
    var y : Int = 100
}

public func == (a : Node, b : Node) -> Bool {
    return (a.x == b.x) && (a.y == b.y)
}

var t = NSDate()
let a = [Node](count: 328, repeatedValue: Node())
let b = Node()
var count = 1
for i in a {
    if i == b {
        count++
    }
}

var elapsed = t.timeIntervalSinceNow * -1000
println(elapsed)

What's absolutely crucial when doing time comparisons between languages is that we make sure we're actually testing the same thing.  In this case, we're not.

But before I get into that into too much detail, I'm going to suggest a slightly better method of testing this code.

First of all, I've moved the actual running code (everything but the class declaration and the == function definition) into an IBAction method so that I can press a button and test it multiple times all in the same build.

Second, I'm going to change the definition of the class for this test to assign random values to its properties.  So the class definition will look like this for my test:

public class Node : Equatable {
    var cost : Int = 0
    var x : Int = Int(arc4random_uniform(100))
    var y : Int = Int(arc4random_uniform(100))
}

This forces the timing to be a little more realistic since branch prediction might easily figure out that the result of == is always true and just fly through our loop.

And third of all, I'm going to test with a much larger array.  I want 10,000 elements.  This may be unrealistically large for actual usage, but when testing speed and trying to optimize time, it's good to work with very large sample sizes.

So, without changing anything else, this is what I'm going to test, and I will include results from the time profiler:

public class Node : Equatable {
    var cost : Int = 0
    var x : Int = Int(arc4random_uniform(100))
    var y : Int = Int(arc4random_uniform(100))
}

public func == (a : Node, b : Node) -> Bool {
    return (a.x == b.x) && (a.y == b.y)
}

class ViewController: UIViewController {
    @IBAction func test(sender: UIButton) {
        var t = NSDate()
        let a = [Node](count: 10_000, repeatedValue: Node())
        let b = Node()
        var count = 1
        for i in a {
            if i == b {
                count++
            }
        }

        var elapsed = t.timeIntervalSinceNow * -1000
        println(elapsed)
    }
}

Completely as written, and using whatever optimization defaults you get when you create a new project in Swift, this takes my desktop 47-50 milliseconds.  On an iPad Mini, this takes about 265-270 milliseconds.

This is the time profile of the code I posted.  I pressed the button to run the test 8 times.  Notice a few lines:

84.9% is where our Swift version of our IBAction method is.  The first 15.1 percent is the time it took to launch our app, initialize, run the view controller, receive the touch events, etc.  The other 84.9% is code that runs from inside our IBAction method.
1.1% is how much of the total time is spent comparing our Node objects with our custom == function.  The array is 10,000 elements large, and we're iterating through it completely 8 times.  We're calling the == function 80,000 times, and it only takes 1.1% of the total time.  Our == function is perfectly fine.

The code that's taking the most time is here:

Unfortunately, we can't see exactly what these are.  They're all from within the Swift library though.  And since Swift is still growing and rapidly changing, it'll probably be a while before the Time Profiler actually gives us the actual method names for these.

I tried moving code around, trying to get different results, but they're all pretty much the same.  I suspected that the array initialization was taking up about half the time and tried moving that out of the timer, but it doesn't seem to make a difference.

What I can tell you though, is this:

Your == method, which is what is all that you've written which is being tested here, is perfectly fast.  It takes almost no time at all.
It's relatively well known that at this point in time, Swift is absolutely horrendous with accessing arrays in loops like this.  But Swift is very much so a growing and evolving language.  Hopefully the Apple engineers are working on improving it.


SQL Inserting up to 10'000 records from a table to a secondary table I'm relatively new to ASP.net and working with larger data sets as a fresh graduate, and I am currently maintaining/developing for an ASP webform and T-SQL system at the moment where we have a database table with ~4000 records, but there is potential for multiple key differences leading up to 10,000 records as a rough estimate to be submitted at a time. This is used for printing to a thermal printer, so a separate standalone database table with duplicate data is necessary.

I'll rewrite the code for insertion to a similar enough format so it's a bit more readable. The query is pretty much as efficient as it can possibly be, it's 3 left joins on ID fields and 2 where clauses to sort to the record I want to copy.

INSERT INTO PRINT_TABLE(ID, KEYNAME1, KEYNAME2, KEYNAME3, KEYNAME4,
PRINTFORMATSTRING, PRINTQUANTITY, COLLECTION, COLLECTION_OWNER, MANAGER, LOCATIONID )
(SELECT
--parameters and joins necessary to copy data across
with parameters @quantity, @id, and @label
)

Now I made an efficiency change already in an attempt to speed it up, it currently runs a foreach loop on a set of data in a telerik radgrid table, and I moved it to start using selecteditems instead of all items and picking out items that are selected, and any that are selected are added to the queue to iterate through. My question is, is it best to run this query in the code behind, on a button submit, i.e.?

Protected Sub AddToPrintDatabaseOnButtonClick(sender As Object, e As System.EventArgs)
    Using connection As New SqlConnection(connectionString)
        Dim command = New SqlCommand()
        command.Connection = connection
        command.CommandText = "INSERT INTO PRINT_TABLE(ID, KEYNAME1,
        KEYNAME2, KEYNAME3, KEYNAME4,
        PRINTFORMATSTRING, PRINTQUANTITY, COLLECTION, COLLECTION_OWNER,
        MANAGER, LOCATIONID )
        (SELECT
            --parameters and joins necessary to copy data across
            with parameters @quantity, @id, and @label
        )"
        connection.Open()
        command.Parameters.AddWithValue("quantity", TextBox.Value)
        command.Parameters.AddWithValue("label", Dropdownlist.Value)
        command.Parameters.AddWithValue("id", 0)
        For Each GDI As GridDataItem In RadGrid.SelectedItems
            command.Parameters("id").Value = Integer.Parse(GDI("id").Text)
            command.ExecuteNonQuery()
        Next
    End Using
End Sub

Which is what I just implemented to speed up the page which would create a new connection string on every selected value. Or is there some other method such as using a procedure and parsing in a list of the ID's and doing it database side, or another method I'm not thinking about? I'm more interested in suggestions or ways of thinking and approach to the issue than being given a flat solution, I want to learn. If you have 1 or 2 snippets that will help me out as an alternative I'd be very thankful.

Ideally, you should provide all relevant code (including the whole query) in order to receive more targeted answers (and also obey site's rules), but based on you have provided, one way to optimize the insertion is the following:

1) Define a session table which will hold all the keys that require inserting the data for

ThermalPrinterSession

ThermalPrinterSessionId INT PRIMARY KEY,
SessionId UNIQUEIDENTIFIER,
Quantity INT,
Label VARCHAR(100),
Id INT

2) Create a stored procedure that will be able to process data for a session

CREATE PROCEDURE uspProcessThermalPrinterData
(
   @SessionId UNIQUEIDENTIFIER,
   @Quantity INT,
   @Label VARCHAR(100),
   @Id INT
)
AS
BEGIN
   INSERT INTO PRINT_TABLE(ID, KEYNAME1,
       KEYNAME2, KEYNAME3, KEYNAME4,
       PRINTFORMATSTRING, PRINTQUANTITY, COLLECTION, COLLECTION_OWNER,
       MANAGER, LOCATIONID )
   SELECT 1                       -- all columns for insertion
   FROM ThermalPrinterSession S
      JOIN OtherTable1 T ON T.Id = S.Id        -- other joins and other tables come here
   WHERE T.SessionId = @SessionId
END
GO

3) For each insert session you can define a new unique identifier

Guid.NewGuid();

4) Actual insertion can be done using bulk insert for improved performance. Entity framework has an extension that helps with bulk inserts and an elegant generic solution can also be found here.

5) call the stored procedure for current session

6) Session table maintenance

If this table is not going to be very big (depends on session inserts), it is better not to perform deletes during the working hours and truncate it once a day (e.g.in a nightly job)


First Python program: Tic-Tac-Toe This is my first program written outside of using books and tutorials. Any help on style and functionality would be helpful.

import sys
from textwrap import dedent
import os
import random

os.system('CLS')

# board number setup
board = [0,1,2,
         3,4,5,
         6,7,8]

# Defines the board layout printed to the console
def board_layout():
    print(dedent(f'''
    *************
    * {board[0]} | {board[1]} | {board[2]} *
    *-----------*
    * {board[3]} | {board[4]} | {board[5]} *
    *-----------*
    * {board[6]} | {board[7]} | {board[8]} *
    *************
    '''))

move_count= 0
def main():
    while  True:

        #Prints board layout to console.
        board_layout()
        #checks for a winner when called at end of each turn
        def check_winner():
            global move_count
            #list of lists with all the winning combinations for from the tic tac toe board
            winning_list = [[board[0],board[1],board[2]],[board[3],board[4],board[5],],
            [board[6],board[7],board[8]],[board[0],board[4],board[8]],[board[2],board[4],board[6]],
            [board[0],board[3],board[6]],[board[1],board[4],board[7]],[board[2],board[5],board[8]]]
            #Keeps a reference to winning_list so it is updated at the end of every turn
            new_list = winning_list

            #iterates over the lists in winning_list
            for i,j,k in winning_list:
                #looks at the lists in winning_list to determine if a list has all x's for a win
                if i ==  'x' and j ==  'x' and k ==  'x' :
                    print('X wins')
                    end()
                #looks at the lists in winning_list to determine if a list has all o's for a win
                elif i ==  'o' and j ==  'o' and k ==  'o' :
                    print('O wins')
                    end()
            #possible moves is 9 in tic tac toe. If all moves are taken and there is no winner no winner forces a draw.
            if move_count == 9:
                print('You Tied')
                end()

        #Takes user input for the move
        move =int(input('Please select a spot: '))
        print(move)

        #Player move, makes sure the spot is not taken and adds 1 to move_count
        if board[move] !='x' and board[move] != 'o':
            board[move] = 'x'
            move_count += 1
            check_winner()
            #npc move, chooses a random spot that is not taken and adds 1 to move_count
            while True:
                npc = random.randint(0,8)
                if board[npc] != 'o' and board[npc] != 'x':
                    board[npc] = 'o'
                    print('Computer chooses spot ', npc)
                    move_count += 1
                    check_winner()
                    break
        #If spot is taken prints that the spot is already taken
        else:
            print('This spot is taken')

        #Game ending
        def end():
            print('Thank you for playing')
            sys.exit()

if __name__ == "__main__":
    main()

The function check_winner() does not need global move_count.  Using global is code smell, avoid if at all possible, which tends to be always.  But in this case it is completely unnecessary, as move_count, like board, is already accessible in check_winner().

winning_list is constructed every time check_winner() is called.  It does not persist from one call to the next, so new_list = winning_list and the comment immediately above it should be removed.

The statement move = int(input(...)) can crash if the user enters invalid input.  Even if a valid integer is given, the integer could be outside the valid range, like 42, which will cause when board[move] is evaluated.  Place user input in a try ... except statement, inside a loop, and don’t let the program continue until valid input has been given.

You have a game loop that handles two turns (a move by both players) each pass through the loop.  While this does work, it will paint you into a corner in subsequent programs.  3 or more players is going to make writing the game loop much harder.

It is usually simpler to handle one turn (a move by only one player), in each pass through the loop.  At the end of the loop, the “current player” is incremented, wrapping around to the first player when necessary.  With only 2 players, this alternates between them.  More advanced games may require skipping player when a “lose a turn” move is made.  Other games may even reverse the direction of play mid game.  All of these would be horrible to try to write the game loop for if each pass through the loop tried to handle all player moves in one pass.

When the game loop is change to handle only a single move at each pass, it is much easier to handle the “game over” condition.  A while game_is_running loop is all that is required.  Or, for tic-tac-toe, you could use:

for move_count in range(9):
    # moves made here
    # break if someone wins
else:
    print("You Tied")

The else: clause of a for loop only executes if the loop finishes without executing break, so after 9 moves with no winner, it is a tie game.

Using sys.exit() to stop the interpreter on a “game over” condition is a bad idea.  It works here, but it makes test code impossible to write, because the program can kill the interpreter, and the test code can’t stop that.


HTML single page layout This is a single page HTML/CSS layout. This is my first ever code and I know it's not perfect, but I would love for someone with a lot more experience to let me know of my mishaps.

<!DOCTYPE HTML>

    <link type="text/css" rel="stylesheet" href="dereke.css"/>
    <title>

    </title>

Ohio State Buckeyes

Home

|

About Us

|

Contact Us

Ohio State is the best college in the United States! 2016

CSS

.Header {
    width: calc(100%-16px);
    height: 150px;
    border-radius: 5px;
}
.Header p {
    color: white;
    margin-top: -5px;
    width: 600px;
    font-size: 70px;
}
.MidBody {
    background-color: #141414;
    width: 100%;
    height: 850px;
    margin-top: 10px;
    border-radius: 5px;
    position: relative;
    display:block;

}
.footer {
    bottom: 0;
    height:50px;
    width: 100%;
    margin-top: 10px;
    border-radius: 5px;
    clear:both;
    position:relative;
    text-align:center;
    color: white;
}
#leftbody {
    width: 49%;
    height: 425px;
    left: 0;
    margin-top: 3px;
    margin-left: 3px;
    position: relative;
    z-index: 1;
    border-radius: 5px;
    float:left;
}
#leftbody img {
    width: 490px;
    height: 420px;
    border-radius: 5px;
    margin-top: 2px;
    margin-left: 2px;
}
#rightbody {
    background-color: #F1F1F1;
    width: 49%;
    height: 425px;
    margin-top: 3px;
    margin-right: 3px;
    position: relative;
    z-index: 1;
    border-radius: 5px;
    float:right;
}
#lowerbody {
    width: 100%;
    height:auto;
    left:0;
    right:0;
    margin-left: auto;
    margin-right: auto;
    postion: relative;
    display: inline-block;
    text-align:center;
}
#lowerbody img {
    position:relative;
    border-radius: 5px;
    margin-left: auto;
    margin-right: auto;
}
body {
    background-color: black;
}
li {
    display: inline;
    padding: 1px;
    text-decoration: underline;
}
#navbar {
    width: 350px;
    color: #F8F8F2;
    font-family: Arial Black;
    margin: -35px;
    text-align: left;
    line-height: 10px;
}
#noUD {
    text-decoration: none;
}
#logo img {
    margin-top: -150px;
    margin-right: 50px;
    width: 15%;
    height: 15%;
    float: right;
}

You have some repeat id tags which aren't good. Using class names instead is much better.

Home

|

About Us

|

Contact Us

And in the CSS it becomes:

.noUD {
  text-decoration: none;
}

You also have standard HTML elements for headers, footers, navigations, and articles:

 becomes

 becomes

 becomes

You are still able to keep classes and ids with this. It just adds extra readability for systems (and humans) and it makes it better in case your CSS fails because browsers apply their own styling to some standard elements.

position: relative; bottom: 0 does nothing in terms of positioning because it sets the element 0 up from where it was before.

Other than that the code looks pretty good.


String of numbers to Integers (without using .to_i) My exercise was to take a string of numbers, such as "1234" and return them as an integer 1234. However, I wasn't allowed to use #to_i, #to_f, Integer, etc.

def convert(int)
  x = 0
  array = int.split('')
  array.each.with_index do |y, i|
    x = (x + (y.hex))
    x = x * 10 unless i == array.length-1
  end
  return x
end

p convert('1234')
p convert('3225')
p convert('4321') == 4321
p convert('570') == 570

When I first started on this exercise, I wondered if I would be able to convert the String into a binary, octal, or other type of representation. Then, take that representation and convert it into integers.

Unfortunately, using #pack and #unpack resulted in more strings, and I couldn't really understand some of the methods that I could use with pack/unpack. Right now I don't have much knowledge in programming, so I couldn't really figure out how to take those strings and parse them into the Integers that I wanted. (My math kinda sucks too...)

Then I read about hex some more and realized that the hex representation
of numbers 0-9 are the same as the integers! So I simply converted each number to a hex, added it to a variable and multiplied it by 10 with each iteration of the loop.

I know there's probably a better way to do this, but this is what I came up with.

I know another way (and possibly simpler) is to make a hash of 10 numbers, 0 - 9, with the keys being the Stringed numbers and their values being Integers, then iterating through that.

Anyways, please let me know what you think of my code and my thought process, and suggest what you can.

So I think using hex isn't in the spirit of the challenge, but that part is easy anyway:

def to_i(digit)
  '0123456789'.index(digit)
end

Your hash idea would have worked fine here too, but the above is a bit terser.

The rest is a good fit for reduce:

def convert(str)
  str.split('').reduce(0) { |m,x| 10 * (m + to_i(x)) } / 10
end

Or you can forget splitting the solution into 2 functions and just do a 1 liner:

def convert(str)
  str.split('').reduce(0) { |m,x| 10 * (m + '0123456789'.index(x)) } / 10
end

Finally, another alternative which sacrifices some brevity to mirror the definition of what a number is even more closely:

def convert(str)
  str.split('').reverse().map.with_index { |x,i| to_i(x) * 10**i }.reduce(:+)
end


Function for splitting an integer into smaller values This is the function I've implemented in an answer to this question.  I've tried to make this as simple and idiomatic as possible, using C++11.  Could this still be improved in any way?

#include <algorithm>
#include <cstdint>
#include <iostream>
#include <iterator>
#include <vector>

typedef std::vector<int> SplitValues;

SplitValues splitter(std::int32_t number)
{
    SplitValues values(8);

    for (auto& iter : values)
    {
        iter = static_cast<int>(number & 0xF);
        number >>= 4;
    }

    return values;
}

int main()
{
    std::int32_t number = 432214123;

    SplitValues values = splitter(number);

    std::reverse_copy(values.begin(), values.end(),
        std::ostream_iterator<int>(std::cout, " "));
}

Test run

Questions:

Is SplitValues an appropriate name for a typedef, or does it sound more like a variable?
If I pass the hard-coded number instead of the number variable, will it still be treated as the same type as the parameter?

The first problem I see is that you're doing a right shift on a signed integer type (std::int32_t).

If a negative value is passed, the result will be implementation defined. When you're going to treat something as a collection of bits instead of treating the whole thing together as a number, you usually want to use an unsigned type, not signed.

I'd also prefer to get rid of the magic (and inter-related) numbers sprinkled throughout the code, such as the 8, 4 and the 0xf. These three are inter-related--the 8 being the number of 4-bit fields contained in a single 32-bit item, and 0xf being a mask with 4 bits set. I'd prefer to specify only one of these values in one place, and compute the others from that one.

As to the specific questions you've asked:

I don't think SplitValues does a good job of indicating a type. This has been a problem for a long time, and I don't think I have a silver bullet to finally put it to rest. In this case, I'd rather just avoid it completely (about which, more below).
If you pass a literal to a function instead of passing a variable, that literal will start with some type--in this case, since it's digits and can fit in an int, its type will be int. That will then be converted to the function's parameter type (std::int32_t). In this case, at least on a typical compiler where int is a 32-bit type, that "conversion" will really be a NOP because the source and target types are basically identical.

As to how to avoid the problem with having to define names, C++ has two mechanisms that can help quite a bit: templates (especially function templates) and auto. Both of these can deduce a result type based on input types, to avoid having to explicitly specify types throughout much of the code.

I'd also generally prefer to write splitter as a generic algorithm. While we're at it, its name should be changed from a noun to a verb. Non-functor classes should have nouns for names, but functors and functions do things, so their names should normally be verbs.

template <classs T, class OutIt>
void split(T number, OutIt result) {
    unsigned bits = 4;
    unsigned mask = (1<<bits) - 1;
    unsigned iter_count = sizeof(T) * CHAR_BIT / bits;

    for (unsigned i=0; i<iter_count; i++) {
        *result++ = number & mask;
        number >>= bits;
    }
}

This part is a little longer than the code in your answer, but most of that extra length stems from specifying the number of bits, then computing the mask and iteration count from that shift value instead of having interrelated magic numbers throughout.

With this code, we can change the type of the input (e.g., to a 64-bit unsigned long long) or the shift count (e.g., to 8 bits) independently of each other, without having to patch the rest of the code to compensate.

Although it's a rather minor point, all else being equal, I prefer to avoid creating variables with default values, then filling in the real values later. This is fairly innocuous when the values involved are at least initialized as they are in a vector, but it's still better to just initialize values when possible.

That leads us to the calling code. Since we changed the function to look like a standard algorithm, we need to change the calling code to suit.

int main() {
    std::deque<int> values;

    split(432214123, std::front_inserter(values));

    std::copy(values.begin(), values.end(), std::ostream_iterator<int>(std::cout, " "));
}

Of course, there are a lot of other ways this job could be handled, but I guess that's enough for the moment anyway.


Combining .txt files A guy at the company I work for needed a small application that would combine multiple text files into one, larger text file.

I wrote a console application for this. it seems pretty efficient, but I was wondering if there would be an even more efficient way of doing this.

It has 2 important functions, one that gets the files from a folder, where string input is the folder location:

static string[] getFiles(string input)
{
    DirectoryInfo dinfo = new DirectoryInfo(@input);

    FileInfo[] files = dinfo.GetFiles("*.txt");
    List<string> list = new List<string>();

    foreach(FileInfo file in files)
    {
        list.Add(input + @"\" + file.Name);
    }

    string[] arr = list.ToArray();
    return arr;
}

And of course the function that combines the files together, its input are the name of the file (string newName) and an array with the names of the files found in the folder by getFiles() (string[] files):

static void writeDump(string newName, string[] files)
{
    if (!File.Exists(newName))
    {
        using (StreamWriter sw = File.CreateText(newName))
        {
            for (int i = 0; i < files.Length; i++)
            {
                using (StreamReader sr = File.OpenText(files[i]))
                {
                    string s = "";
                    while ((s = sr.ReadLine()) != null)
                    {
                        sw.WriteLine(s);
                    }
                }
            }
        }
    } else
    {
        Console.Clear();
        Console.ForegroundColor = ConsoleColor.Red;
        Console.WriteLine("File already exists");
        start(); //start is called from the main function
    }
}

And because start(); might be confusing, I'll also add the main function here:

static void Main(string[] args)
{
    start();
}

How efficient is this and could it be more efficient?

list.Add(input + @"\" + file.Name);

Seems a bit pointless: file.FullName would get you the fully qualified name without throwing information away and reconstructing it. In fact, that method could be simplified with Linq to

static string[] getFiles(string input)
{
    DirectoryInfo dinfo = new DirectoryInfo(@input);
    return dinfo.GetFiles("*.txt").Select(f => f.FullName).ToArray();
}

I also note that the .Net convention for the name would be GetFiles with an initial uppercase letter.

for (int i = 0; i < files.Length; i++)
{
    using (StreamReader sr = File.OpenText(files[i]))
    {
        string s = "";
        while ((s = sr.ReadLine()) != null)
        {
            sw.WriteLine(s);
        }
    }
}

Since you don't care about i you could simplify things with foreach; and the initial value of s is unnecessary, so you could have

foreach (var filename in files)
{
    using (StreamReader sr = File.OpenText(filename))
    {
        string s;
        while ((s = sr.ReadLine()) != null)
        {
            sw.WriteLine(s);
        }
    }
}

But now we get to two key points of the requirements which aren't explicitly stated:

If the files don't end with newlines, this code will insert newlines. That may or may not be intended, and it may or may not be desirable.
This code is using an Encoding to parse the bytes to strings, then using an Encoding to convert the strings back to bytes. The particular encoding used is implicit. This isn't particularly efficient, but it does have some benefits:

If the files were generated by Microsoft tools, they are quite likely to start with BOMs (even if they're UTF-8). In the nasty case that they mix UTF-8-BOM, UTF-8, and UTF-16 then you rely on the encoding conversion.
Even if the files are consistent, you're going to avoid the appearance of BOMs embedded in the text that a straightforward byte-by-byte concatenation would give.

It also has at least one non-performance-related disadvantage:

Regardless of the encoding of the input files, the output file is likely to be UTF-8-BOM, which may be an undesirable side-effect if they were all UTF-8 or UTF-16.

If you wanted a straight byte-by-byte conversion then it would be more efficient to use

using (var strmOut = File.Create(newName))
{
    foreach (var filename in files)
    {
        using (var strmIn = File.OpenRead(filename))
        {
            strmIn.CopyTo(strmOut);
        }
    }
}

If you can guarantee that the input files are all UTF-8-BOM then it would be more efficient to use

using (var strmOut = File.Create(newName))
{
    foreach (var filename in files)
    {
        using (var strmIn = File.OpenRead(filename))
        {
            strmIn.Position = 3;
            strmIn.CopyTo(strmOut);
        }
    }
}

although that's not production-quality code (should check that there are 3 bytes and that they correspond to a BOM).


Excel macro to move data across workbooks using an ADO connection I am writing a macro to move data from a CSV to an excel template. Currently, I have the code set to search for a keyword in column A of the CSV, and extract data from specified columns in the keywords row. I would like to know if there is something I can do to simplify the code.

Public Sub MoveData()

    '**defines the project name as a variable
    Dim fileName As String
        fileName = Worksheets("Cover").Range("B5").Value

    '**defines the path of the CSV summary from BlueBeam
    Dim path As String
        path = "C:\Users\(users)\Documents\(folder)\" & fileName & ".csv"

    '**defines the two workbooks that the data will move between
    Dim currentWB As Workbook
    Set currentWB = ThisWorkbook

    Dim openWB As Workbook
    Set openWB = Workbooks.Open(path)

    Dim openWs As Worksheet
    Set openWs = openWB.Sheets(fileName)

    '**connects using ADODB to transfer the data
    Dim cn As ADODB.Connection
    Set cn = New ADODB.Connection
    With cn
        .Provider = "Microsoft.ACE.OLEDB.12.0"
        .ConnectionString = "Data Source=" & path & ";" & _
    "Extended Properties=Excel 12.0 Xml;"
        .Open
    End With

    '**selects the first column to be read and sorted
    Dim subCell As Range
    Dim myRange As Range
    Set myRange = Range("A1:A500")

    Dim cmdOpen1 As Boolean
    cmdOpen1 = False
    Dim cmdOpen2 As Boolean
    cmdOpen2 = False

    For Each subCell In myRange

        If subCell Like "*keyword1*" Then

            strQuery = "SELECT [Measurement] FROM [" & fileName & "$] Where Subject = '" & subCell.Value & "'"

            Set cmd1 = New ADODB.Command
            With cmd1
                .ActiveConnection = cn
                .CommandText = strQuery
            End With

        Dim rst1 As New ADODB.Recordset
            With rst1
                If cmdOpen1 = False Then
                    .Open cmd1
                    cmdOpen1 = True
                End If
            End With

            currentWB.Worksheets("Bms").Range("C7").CopyFromRecordset rst1

        ElseIf subCell Like "*keyword2*" Then

            strQuery = "SELECT [Notes (C)], [Col Top (C)], [Col Base (C)] FROM [" & fileName & "$] Where Subject = '" & subCell.Value & "'"

            Set cmd2 = New ADODB.Command
            With cmd2
                .ActiveConnection = cn
                .CommandText = strQuery
            End With

            Dim rst2 As New ADODB.Recordset
            With rst2
                If cmdOpen2 = False Then
                    .Open cmd2
                    cmdOpen2 = True
                End If
            End With

            currentWB.Worksheets("Cols").Range("B7").CopyFromRecordset rst2

        End If

    Next subCell
    openWB.Close

rst1.Close
rst2.Close
End Sub

I'm fairly new to VBA and extremely new to recordsets, so please let me know if you notice anything that should be changed. In total, I will be writing the code to search for about 6-7 keywords and the columns that the data will change based on the keyword. I'd like to revise this code before I move forward with adding more data selection.

If you need anymore information on what I am trying to accomplish or how I coded an area, please let me know. Any help will be greatly appreciated.

The first thing is that you do not seem to be using Option Explicit because there are some variables (strQuery, cmd1 and cmd2) that are not declared. You should always use Option Explicit and this can be turned on in Tools -> Options, on the Editor tab check the "Require Variable Declaration" checkbox. The IDE will add Option Explicit at the top of every new module.

You are currently mixing the "standard" Excel data access methods (cells, ranges, etc) with using sql which makes it harder to keep track of what your code is doing. This can be simplified by using just sql to get data from the csv file. You don't need to read down through the csv file looking for keywords, you can include the keyword in your sql with a WHERE clause. This also means you do not need to open the csv file. In sql the % character is the wildcard for zero or many characters.

If you are retrieving data using ADODB, then you only need to use Recordset, you don't need to use a Command object and a Recordset. If you will be repeatedly opening a recordset and copying the data to a range then you could put that code into a separate procedure. In my code below, I have the CopyFromFileToRange procedure.

Some of your variable names are good but some are too short/unclear (cn, rst1 and cmd) and strQuery looks like you might be thinking of using Hungarian notation. See this post for a discussion of Hungarian. The main thing is to be consistent in your choice/style.

Option Explicit

Public Sub MoveData()

    '**defines the project name as a variable
    Dim fileName As String
        fileName = Worksheets("Cover").Range("B5").Value

    '**defines the path of the CSV summary from BlueBeam
    Dim filePath As String
        filePath = "C:\Users\(users)\Documents\(folder)\"

    '**defines the destination workbook
    Dim currentWB As Workbook
    Set currentWB = ThisWorkbook

    '**connects using ADODB to transfer the data
    Dim dbConn As ADODB.Connection
    Set dbConn = New ADODB.Connection

    With dbConn
        .ConnectionString = "Driver={Microsoft Text Driver (*.txt; *.csv)};Dbq=" & filePath _
            & ";Extensions=asc,csv,tab,txt;"
        .Open
    End With

' Added declarations
Dim measurementQueryString As String
Dim notesQueryString As String
Dim keyword1 As String
Dim keyword2 As String

    keyword1 = "some_value"
    measurementQueryString = "SELECT [Measurement] FROM [" & fileName & "] " _
        & "Where Subject LIKE '%" & keyword1 & "%';"
    Call CopyFromFileToRange(dbConn, measurementQueryString, currentWB.Worksheets("Bms").Range("C7"))

    keyword2 = "some_value"
    notesQueryString = "SELECT [Notes (C)], [Col Top (C)], [Col Base (C)] FROM [" & fileName & "] " _
        & "Where Subject LIKE '%" & keyword2 & "%'"
    Call CopyFromFileToRange(dbConn, notesQueryString, currentWB.Worksheets("Cols").Range("B7"))

    dbConn.Close

End Sub

Private Sub CopyFromFileToRange(ByRef dbConn As ADODB.Connection, ByRef queryString As String, ByRef targetRange As Range)

' Opens a recordset using queryString as the source
' Writes the data to targetRange

Dim dataFromCsv As ADODB.Recordset

    Set dataFromCsv = New ADODB.Recordset
    dataFromCsv.Open Source:=queryString, ActiveConnection:=dbConn
    If Not dataFromCsv.EOF Then
        targetRange.CopyFromRecordset dataFromCsv
    Else
        ' No data found
    End If

    dataFromCsv.Close

End Sub


Iterator and Generator versions of Python's range() I have created iterator and generator versions of Python's range():

Generator version

def irange(*args):
 if len(args) > 3:
     raise TypeError('irange() expected at most 3 arguments, got %s' % (len(args)))
 elif len(args) == 1:
     start_element = 0
     end_element = args[0]
     step = 1
 else:
     start_element = args[0]
     end_element = args[1]
     if len(args) == 2:
         step = 1
     elif (args[2] % 1 == 0 and args[2] != 0):
         step = args[2]
     else:
         raise ValueError('irange() step argument must not be zero')

 if((type(start_element) is str) or (type(end_element) is str)
    or (type(step) is str)):
     raise TypeError('irange() integer expected, got str')

 count = 0
 while (( start_element + step < end_element )
        if 0 < step else
        ( end_element < start_element + step )) :
     if count == 0:
         item = start_element
     else:
         item = start_element + step
     start_element = item
     count +=1
     yield item

Iterator version

class Irange:
 def __init__(self, start_element, end_element=None, step=1):
     if step == 0:
         raise ValueError('Irange() step argument must not be zero')
     if((type(start_element) is str) or (type(end_element) is str)
        or (type(step) is str)):
         raise TypeError('Irange() integer expected, got str')

     self.start_element = start_element
     self.end_element = end_element
     self.step = step
     self.index = 0

     if end_element is None:
        self.start_element = 0
        self.end_element = start_element

 def __iter__(self):
     return self

 def next(self):
     if self.index == 0:
         self.item = self.start_element
     else:
         self.item = self.start_element + self.step
     if self.step > 0:
         if self.item >= self.end_element:
                raise StopIteration
     elif self.step < 0:
         if self.item <= self.end_element:
             raise StopIteration

     self.start_element = self.item
     self.index += 1
     return self.item

Usage

 >>> for i in irange(2,5):
 ...  print i,
 2 3 4
 >>> for i in irange(2,-3,-1):
 ...  print i,
 2 1 0 -1 -2
 >>> for i in Irange(3):
 ...  print i,
 0 1 2

I would like to know if the approach is correct.

First of all, to validate that the function is working,
it's good to use assert statements:

assert [0, 1, 2, 3, 4] == [x for x in irange(5)]
assert [2, 3, 4] == [x for x in irange(2, 5)]
assert [2, 1, 0, -1, -2] == [x for x in irange(2, -3, -1)]

With these statements covering my back,
I refactored your irange method to this:

def irange(*args):
    len_args = len(args)

    if len_args > 3:
        raise TypeError('irange() expected at most 3 arguments, got %s' % len_args)

    if len_args < 1:
        raise TypeError('irange() expected at least 1 arguments, got %s' % len_args)

    sanitized_args = [int(x) for x in args]

    if len_args == 1:
        start_element = 0
        end_element = sanitized_args[0]
        step = 1
    else:
        start_element = sanitized_args[0]
        end_element = sanitized_args[1]
        step = 1 if len_args == 2 else sanitized_args[2]

    current = start_element

    if step > 0:
        def should_continue():
            return current < end_element
    else:
        def should_continue():
            return current > end_element

    while should_continue():
        yield current
        current += step

Points of improvement:

Since len(args) is used repeatedly, I cache it in len_args
Added len(args) < 1 check too, in the same fashion as len(args) > 3
Simplified the type checking of args:

Sanitize with a single, simple list comprehension
If there are any non-integer arguments, a ValueError will be raised with a reasonably understandable error message

Simplified the initialization of start_element, end_element and step
Greatly simplified the stepping logic

As for the iterator version,
it would be easiest and best to implement that in terms of the generator version.


Searching in Multidimensional Array I was wondering about the Array built in methods in java 8. As there is no specific method other than Arrays.binarySearch. So for multidimensional array I wrote following method.
public int[] search(Integer[][] mat,int key)
    {
        int r=0,c=0;
        for(Integer[] a:mat)
        {
            if(Arrays.asList(a).contains(key)){
                c = Arrays.asList(a).indexOf(key);
                break;
            }
            else r++;
        }
        return new int[]{r+1,c+1};
    }

checking
 Integer[][] mat = {{1,2,3,99},{4,5,6,57},{7,8,9,89},{10,11,12,13}};
int [] rc = new int[2];
 rc = search(mat, 8);
 System.out.println(Arrays.toString(rc));
 rc = search(mat, 9);
 System.out.println(Arrays.toString(rc));
 rc = search(mat,10);
 System.out.println(Arrays.toString(rc));
 rc = search(mat, 1);
 System.out.println(Arrays.toString(rc));

output
[3, 2]
[3, 3]
[4, 1]
[1, 1]

I would like a review of efficiency and general code quality. Also any improvements I can do to make it better.

A few things to mention:

As has been mentioned, you're using 1-based indexing instead of 0-based indexing.  0-based is much more intuitive, since most implementations, of indexing in most programming languages, use 0-based indexing.

You're using the Arrays.asList twice for each iteration of the loop. This increases the inefficiency.

Calling indexOf inside the loop is basically creating another loop and doesn't gain anything a for loop wouldn't give you.

I think the name of your method could be better.  search indicates all you want is the value.  However you're returning the indexes.  Therefore indexesOf might be more appropriate, and more in fitting with similar methods in other collections.

Your method is hard coded for int.  This isn't very extensible.  A generic version would probably be more fitting.

You're returning an int[].  This doesn't really tell anyone what this is supposed to represent.  I would suggest a class(Indexes) to hold the indexes would allow you to represent them with a name that will immediately let the user know what it represents.

You haven't made any provision to handle failed searches.

It looks to me that the 2D array is basically a matrix.  Therefore I would suggest labeling the indexes as such(row,column).

The Indexes class would look like this:

public class Indexes {
    int row = 0;
    int col = 0;

    public Indexes(int row, int col) {
        this.row = row;
        this.col = col;
    }

    public Indexes() {
    }

    public int getRow() {
        return row;
    }

    public void setRow(int row) {
        this.row = row;
    }

    public int getCol() {
        return col;
    }

    public void setCol(int col) {
        this.col = col;
    }
    @Override
    public String toString(){
        return String.format("[%d, %d]",row,col);
    }
}

A generified version with the points I mentioned could look something like this:

public static<T> Indexes indexesOf(T[][] arr, T value){
    for(int row = 0; row < arr.length; ++row){
        for(int col = 0; col < arr[row].length; ++col){
            if(arr[row][col].equals(value)){
                return new Indexes(row,col);
            }
        }
    }
    return new Indexes(-1,-1);
}

EDIT Alternative code based on OP's comments

static final int DEFAULT_RETURN_VALUE = -1;
public static<T> Indexes indexesOf(T[][] arr, T value){
    for(int row = 0; row < arr.length; ++row){
            int col = indexOf(arr[row],value);
            if(col > DEFAULT_RETURN_VALUE){
                return new Indexes(row,col);
            }
    }
    return new Indexes(DEFAULT_RETURN_VALUE,DEFAULT_RETURN_VALUE);
}
public static<T> int indexOf(T[] arr, T value){
    for(int i = 0; i < arr.length; ++i){
        if(arr[i].equals(value)){
            return i;
        }
    }
    return DEFAULT_RETURN_VALUE;
}


Placement of Conditionals I've noticed a tendency towards two styles of conditionally executing methods:

Conditional before the call

def foo(thing)
  puts thing
end

if thing
  foo(thing)
end

Conditional within the call

def foo(thing)
  return unless thing
  puts thing
end

foo(thing)

Pros of 1:

Method is concise and easier to read
Method can assume input is valid
Obvious what's going on in calling code

Pros of 2:

Caller doesn't need to check condition every time the method is invoked
Calling code is cleaner and easier to read
Conditional is centralised in one location and not duplicated (DRY!)

Anecdotally, I've noticed that most people tend to lean towards the first style. Lately I've found my self leaning towards the second style, for the pros I've given above.

Thoughts? Any other advantages/disadvantages I've missed? Any other styles?

I think that one of the pros you listed for style 2 - "Conditional is centralized in one location and not duplicated" is sufficient all by itself to tip the scale to style 2.

Non-duplication of code is a very powerful idea, particularly in a system under development. There is always the possibility that something will change. Duplicated code is very fragile, because of the possibility of overlooking one or more instances. Even if you don't overlook an instance, changing multiple instances of duplicated code is always harder than changing one single instance of non-duplicated code.


Toggling dropdowns using jQuery, with binding and unbinding This code is used to toggle some dropdowns. I've added the click event on the document to close the dropdown on click outside, but I'm not sure if I'm doing it the right way. I want it to be optimized for better performance.

Let me know if you have a better way to bind/unbind the events.

$('.header-top-bar .toggle').each(function (index) {
  var $this = $(this),
      $parent = $this.parent(),
      $dropdown = $this.siblings('.toggle-content');

  $this.on('click.toggle' + index, function (event) {
    event.preventDefault();

    $dropdown.toggleClass('isOpen');

    // Outside
    $document.on('click.toggle' + index, function (event) {
      if (!$(event.target).closest($parent).length) {
        $dropdown.removeClass('isOpen');
        $document.off('click.toggle' + index);
      }
    });
  });
});

jsFiddle

First, regarding readability, I suggest changing the following names:

$dropdown -> $content
.toggle (class) -> .togglable
.toggle (namespace) -> .toggling
.isOpen -> isToggleOpen

On the other hand this can be achieved with reduced code (and less internal working impact), with only one event binded:

var $togglables = $('.togglable-content');
$(document).on('click.toggling', function (event) {
  var $target = $(event.target),
  $content = $target.siblings('.togglable-content');
  if (!$target.hasClass('isTogglableOpen')) {
    $togglables.not($content).removeClass('isTogglableOpen');
    $content.toggleClass('isTogglableOpen');
    return false;
  }
});

Here is the modified fiddle.


Preventing floods while saving logs I had a logging function…

@task
def log_request(label, target, request_as_dict):
    # Detect flood

    # In general this task do not process misuse. But it can send
    # an alarm to another celery task if something strange happened.
    if incr_with_ttl(INDICATOR_KEY, INDICATOR_TTL) >= INDICATOR_THRESHOLD:
        process_misuse.apply_async()
        redis.delete(INDICATOR_KEY)

    # Save entry
    fields = request_as_dict['fields']
    fields.update({
        "label": label,
        "type": 1 if target else 2, # 1: target, 2: extra

        "suspicion": suspicion_rating(request_as_dict),
        "raw_request": request_as_dict
    })
    RequestEntry.objects.create(**fields)

… that I rewrote using two inner functions for readability.

@task
def log_request(label, target, request_as_dict):

    def detect_flood():
        '''
        In general this task do not process misuse. But it can send
        an alarm to another celery task if something strange happened.
        '''
        if incr_with_ttl(INDICATOR_KEY, INDICATOR_TTL) >= INDICATOR_THRESHOLD:
            process_misuse.apply_async()
            redis.delete(INDICATOR_KEY)

    def save_entry():
        fields = request_as_dict['fields']
        fields.update({
            "label": label,
            "type": 1 if target else 2, # 1: target, 2: extra

            "suspicion": suspicion_rating(request_as_dict),
            "raw_request": request_as_dict
        })
        RequestEntry.objects.create(**fields)

    save_entry()
    detect_flood()

Is the rewrite an improvement?

Yes this is fine.

By the single responsibility principle, and to achieve functional cohesion, a function should do one thing. As the responsibility of this function takes two logical steps, it makes sense to move those steps to their own functions. And since these functions are only used once, and so they would not be useful not desirable in the global namespace, it makes sense to hide them as inner functions.


Shifting the origin of a large list of coordinates Problem and goal

Given a large edge list file (such as 2 million nodes, 30 million edges) containing source and destination coordinates, I want to 'convert' them in such a way so that the coordinates start at 0. Hopefully the example will clear this up. My goal is to increase the performance (speed) of my current code.

Example

input_file.txt

54    414
12    54
414   87
20    54
12    3

Output

0    1
2    0
1    3
4    0
2    5

Notice how 54 becomes 0, 414 becomes 1, 12 becomes 2, 87 becomes 3 etc. In other words: if a vertex is re-used like 54, the same coordinate will be used.

My solution

While my solution works fine, it is slow for a large edge list such as: this edge list (or this small one for testing purposes), and I was wondering whether there are areas to improve my code.

Essentially, I am using a dictionary (map) that keeps track of each vertex and its coordinate respectfully. If a vertex is not present in the dictionary, it uses the current coordinate and increments it for any possible new vertex.

Code

#pragma warning (disable : 4996)
#include <iostream>
#include <unordered_map>
#include <iostream>
#include <string>
#include "conio.h"
#include <sstream>
#include <fstream>

using namespace std;

void load_graph_from_edge_list_file_to_coo(vector<int>& , vector<int>&, string);
int add_vertex_as_coordinate(vector<int>&, unordered_map<int, int>&, int, int);

int main() {
    vector<int> source_vertices;
    vector<int> destination_vertices;
    string input_file_path = "C:\\Users\\User\\Desktop\\input_file.txt";
    load_graph_from_edge_list_file_to_coo(source_vertices, destination_vertices, input_file_path);

    return 0;
}

void load_graph_from_edge_list_file_to_coo(vector<int>& source_vertices, vector<int>& destination_vertices, string file_path) {
    unordered_map<int, int> map_from_edge_to_coordinate;
    int current_coordinate = 0;

    std::ifstream infile(file_path);
    string line;
    while (getline(infile, line))
    {
        std::istringstream iss(line);
        int vertex_source, vertex_destination;
        if (!(iss >> vertex_source >> vertex_destination)) {
            continue;
        } else {
            current_coordinate = add_vertex_as_coordinate(source_vertices, map_from_edge_to_coordinate, vertex_source, current_coordinate);
            current_coordinate = add_vertex_as_coordinate(destination_vertices, map_from_edge_to_coordinate, vertex_destination, current_coordinate);
        }
    }
    infile.close();

    printf("\nTotal amount of vertices: %d", map_from_edge_to_coordinate.size());
    printf("\nTotal amount of edges: %d", source_vertices.size());
}

int add_vertex_as_coordinate(vector<int>& vertices_type, unordered_map<int, int>& map_from_edge_to_coordinate, int vertex, int coordinate) {
    if (map_from_edge_to_coordinate.count(vertex)) {
        vertices_type.push_back(map_from_edge_to_coordinate.at(vertex));

        return coordinate;
    }
    else {
        map_from_edge_to_coordinate[vertex] = coordinate;
        vertices_type.push_back(coordinate);
        coordinate++;

        return coordinate;
    }
}

Any tips to increase the performance would be greatly appreciated.

The only thing I can see that will speed stuff up is this part:

string line;
while (getline(infile, line))
{
    std::istringstream iss(line);
    int vertex_source, vertex_destination;
    if (!(iss >> vertex_source >> vertex_destination)) {

There is a lot of copying from line to iss. This provides some good error checking and correction ability. But does slow the code down.

Replacing the above with

while (iss >> vertex_source >> vertex_destination)
{

results in a 10% speed improvement in my code (though I took out the vectors; I did not need them).


Book program with arrayList I have this Book program that contains 2 classes: Book and Library.  I have the Book class done, but need some help on the Library class. Please help me check my code. I can provide the Book class if you need it.

Instruction here:

  Fields: A single private ArrayList of Book field is all that is
  necessary. This will hold all the Book objects in the library.

  Methods: public Library(ArrayList other)

  Throws a NullPointerException if other is null. Otherwise, the
  Library’s Book ArrayList should take on all the books in other.

  public Library( ) Creates an empty ArrayList of books.

  public boolean add(Book book)

  First checks for null or empty Strings and calls the appropriate
  exception. Otherwise it adds the Book argument to the end of the
  library ArrayList. Notice it returns a boolean (whether or not the add
  operation succeeded) See the add method of ArrayList:

  public ArrayList findTitles(String title)

  Generates an ArrayList of all books which have titles that match
  (exactly) with the passed argument and returns this list to the
  calling program. The String compareTo method is useful here.

  public void sort( )

  Sorts the library’s book ArrayList in ascending order according to the
  title field (sort these titles just as they are, i.e. don’t worry
  about articles such as The or A, etc.). As illustrated in the
  textbook, p. 666 (Don’t let this number concern you :) ), you will use
  the Collections sort.

  public String toString( )

  returns a properly formatted String representation of all the books in
  the library (Title followed by authors).

    import java.util.ArrayList;

import java.util.Collections;

public class Library {
    private ArrayList<Book> allBook = new ArrayList<Book>();

    public Library(ArrayList<Book> other) {
        if (other == null) {
            throw new NullPointerException("null pointer");
        } else
            this.allBook = other;
    }

    public Library() {
        this.allBook = new ArrayList<Book>();
    }

    public boolean add(Book book) {
        if (book != null && !book.equals("")) {
            throw new IllegalArgumentException("Can't be empty");
        }
        allBook.add(book);
        return true;
    }

    public ArrayList<Book> findTitles(String title) {
        for(Book b: allBook) {
            if(title.compareTo(b.getTitle())== 0) {
                return allBook;
            }
        }
        return null;
    }

    public void sort() {
        Collections.sort(allBook);
    }

    public String toString() {
        return Library.this.toString();
    }
}

    public class Book implements Comparable<Book> {
    private String bookTitle;
    private ArrayList<String> bookAuthor;

    public Book(String title, ArrayList<String> authors) {
        if(title == null && authors == null) {
            throw new IllegalArgumentException("Can't be null");
        }
        if(title.isEmpty() && authors.isEmpty()) {
            throw new IllegalArgumentException("Can't be empty");
        }
        bookTitle = title;
        bookAuthor = authors;
    }

    public String getTitle() {
        return bookTitle;
    }
    public ArrayList<String> getAuthors( ) {
        return bookAuthor;
    }

    public String toString( ) {
        return bookTitle + bookAuthor;
    }
    public int compareTo(Book other){
        return bookTitle.compareTo(other.bookTitle);
    }
    public boolean equals(Object o) {
         if(!(o instanceof Book)) {
             return false;
        }
         Book b = (Book) o;
         return b.bookTitle.equals(bookTitle)
                 && b.bookAuthor.equals(bookAuthor);
    }
}

Your default constructor Library() initializes the allBook member variable twice, with the same value in each case.  Probably not what you had in mind.  The immediate fix is to remove the (re) assignment in the default constructor.

Now you have two different constructors that do the right thing two different ways.  To save yourself maintenance headaches, you'd normally prefer to have one code path that does "everything", so it would be good to combine the two.  There are two different ways of doing that, depending on the requirements....

Consider this test.

ArrayList source = new ArrayList();
source.add(book1);
source.add(book2);
source.add(book3);

Library library = new Library(source);
library.add(book4);

At this point, the library should contain four books.  But how many books should the source array contain?

If the answer is four, the the library is supposed to be modifying the array it was passed.  In that case, the easy answer to the constructor problem is to have the default constructor call the other, like so:

Library () {
    this(new ArrayList<Book>);
}

On the other hand, if the source array should still contain three books, then the Library should contain a copy of the ArrayList, rather than holding onto the original.  If that's your requirement, then the good answer is to go the other way around - initialize the allBook member where you declare it, but use copy semantics when you are passed the ArrayList

ArrayList<Book> allBook = new ArrayList<Book> ();

Library () {}

Library (ArrayList<Book> books) {
    allBook.addAll(books);
}

Your code in this version of the puzzle is very confused about Books, titles, and Strings.  Your requirements said

public ArrayList findTitles(String title)

but the signature in your class is

public ArrayList<Book> findTitles(Book title)

The code in a number of places suggests that, once upon a time, you thought all books were just Strings, and then you changed your mind.  Your compiler should be telling you that you aren't being consistent.

The requirements for findTitles says that you should be returning an ArrayList of books with matching titles.  That means you probably need to be creating a new ArrayList, and then add()ing the matching Books to it.  The code you've written returns all of the books, but takes extra time to compare at the books first.  You probably don't want to call Book.compareTo (although you can implement the solution correctly that way), but instead Book.getTitle().equals(otherBook.getTitle())

There are better answers than for loops for visiting all of the elements in an ArrayList.  See Iterable.  Since you've written Book.toString() already, you can start Library.toString() by building a big String out of all of the Book.toString()s.


Forum system implementation I have a function which does the following:

When new Post Entity is added some fields in Category table are updated.
When new Thread Entity is added the same fields in Category table are updated.

Because of this, some lines are completely the same.

public function postPersist(LifecycleEventArgs $args)
{
    $entity = $args->getObject();
    $entityManager = $args->getObjectManager();

    if ($entity instanceof Post) {
        $post = $entity;

        $thread = $post->getThread();

        $category = $thread->getCategory();
        $category->setLastPostThreadTitle($thread->getTitle());
        $category->setLastPostThreadSlug($thread->getSlug());
        $category->setLastPostBody($post->getBody());
        $category->setLastPosterUsername($post->getUser()->getUsername());
        $category->setLastPostCreatedAt($post->getCreatedAt());
        $category->setIsLastPostOp(false);
        $category->setPosts($category->getPosts() + 1);

        $entityManager->merge($category);
        $entityManager->flush();
    }

    if ($entity instanceof Thread) {
        $thread = $entity;

        $category = $thread->getCategory();
        $category->setLastPostThreadTitle($thread->getTitle());
        $category->setLastPostThreadSlug($thread->getSlug());
        $category->setLastPostBody($thread->getBody());
        $category->setLastPosterUsername($thread->getUser()->getUsername());
        $category->setLastPostCreatedAt($thread->getCreatedAt());
        $category->setIsLastPostOp(true);

        $entityManager->merge($category);
        $entityManager->flush();
    }
}

How would be possible to refactor this code to avoid WET?

How about something like this there is some checking that needs to be done because I inverse the $isPost bool and that could be buggy

public function postPersist(LifecycleEventArgs $args)
{
    $entity = $args->getObject();
    $entityManager = $args->getObjectManager();

    $isPost = $entity instanceof Post;

    $thread = $isPost ? $post->getThread() : $entity;

    $details = $isPost ? $entity : $thread;

    $category = $thread->getCategory();
    $category->setLastPostThreadTitle($thread->getTitle());
    $category->setLastPostThreadSlug($thread->getSlug());
    $category->setLastPostBody($details->getBody());
    $category->setLastPosterUsername($details->getUser()->getUsername());
    $category->setLastPostCreatedAt($details->getCreatedAt());

    //NOTE the inversion of the isPost bool
    //TODO Test this
    $category->setIsLastPostOp(!$isPost);

    if($isPost){
        $category->setPosts($category->getPosts() + 1);
    }

    $entityManager->merge($category);
    $entityManager->flush();
}


Avoid embedded for loops Can the following be rewritten in a more programmatic way, possibly getting rid of the embedded for loops?

function createCards() {
    var cards = [];
    var i, j, k, l;

    for(i = 0; i < 3; i += 1)
        for(j = 0; j < 3; j += 1)
            for(k = 0; k < 3; k += 1)
                for(l = 0; l < 3; l += 1)
                    cards.push([i, j, k, l]);

    return cards;
}

Since this is code is targetting Node, I'm happy to use EcmaScript 6 features.

I'm happy to use EcmaScript 6 features

Try array comprehensions!

var vals = range(0, 3);
return [ [i, j, k, l] for (i of vals) for (j of vals) for (k of vals) for (l of vals) ]

Still, it translates to the same nested loops. If you need a variable nesting level, you  can use a recursive solution.


Keep track of Terminal access I am fairly new to bash (I have about 3 months of experience), and now I've written my first real application (sort of).

It's a function inside of ~/.bash_profile, but it has an installer. It is called loginfo and it keeps track of when the terminal was used by the user, and allows them to view this data.

It comes with an installer, and a built-in uninstaller.

install.sh

#!/bin/bash

width=$(tput cols)
printSolid(){
printf "%${width}s\n" " " | tr " " "$1"
}

printCenter(){
textsize=${#1}
padding=$((($width+$textsize)/2))
printf "%${padding}s\n" "$1"
}

install_loginfo(){
touch ~/.bash_profile
if grep -q "#loginfo-->" ~/.bash_profile
then
    echo "[ER] Already installed 'loginfo'."
else
    loginfofile=$(echo "$(dirname $0)/loginfo.txt")
    test ! -e $loginfofile && (echo "[ER] Unable to locate '$loginfofile'")
    cat $loginfofile >> ~/.bash_profile
    test $? -eq 0 && (echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'")
    test $? -ne 0 && (echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'")
fi
}

printSolid "#"
printCenter "Installer for 'loginfo'"
printCenter "By 545H4"
printSolid "#"
echo "Would you like to install 'loginfo'? (y/n)"
while true
do
read -n 1 -r -s confirm
case $confirm in
y | Y)
    install_loginfo
    break
    ;;
n | N)
    echo "'loginfo' was not installed."
    break
    ;;
*)  echo "Please enter a valid option (y/n)"
    ;;
esac
done
echo "You have got to restart your terminal for any change to take place."
exit

loginfo.txt

#loginfo-->

currLogin=$(date '+%d/%m/%Y %H:%M:%S')
echo "$currLogin" >> terminallogins.txt

deltLoginfo(){
while true
do
echo "Remove 'loginfo'? (y/n)"
read -n 1 -r -s confirm
case $confirm in
y | Y)
        echo "$(sed '/^#loginfo-->/,/^#<--loginfo/d' ~/.bash_profile)" > ~/.bash_profile
        test $? -eq 0 && (echo "[OK] Successfully removed 'loginfo' from '~/.bash_profile'")
        test $? -ne 0 && (echo "[ER] Failed to remove 'loginfo' from '~/.bash_profile'")
        touch ~/terminallogins.txt
        rm ~/terminallogins.txt
        test $? -eq 0 && (echo "[OK] Successfully removed '~/terminallogins.txt'")
        test $? -ne 0 && (echo "[ER] Failed to remove '~/terminallogins.txt'")
        echo "You have got to restart your terminal for any change to take place."
        break
        ;;
n | N)
        echo "'loginfo' wasn't removed."
        break
        ;;
*)
        echo
        ;;
esac
done
}

loginfo(){
case $1 in
last) echo "$(tail -2 ~/terminallogins.txt)" | head -n 1;;
curr) echo "$currLogin";;
list) cat ~/terminallogins.txt;;
delt) deltLoginfo;;
*) echo "Usage: loginfo <last/curr/list/delt>";;
esac
}

#<--loginfo

GitHub

Inside of the repository install.sh and loginfo.txt are in the same folder, I am new to GitHub as well but using two files seemed the most easy to manage.

indent your code!
don't do this

printf "%${padding}s\n" "$1"

do this

printf "%*s\n" "$padding" "$1"

in this section:

cat $loginfofile >> ~/.bash_profile
test $? -eq 0 && (echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'")
test $? -ne 0 && (echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'")

the 2 test command is testing the exit status of the previous command, not the cat command. It's either testing the exit status of the previous test command or the first echo command. Either way, you get the result you want, but really by luck. You want

if cat $loginfofile >> ~/.bash_profile; then
    echo "[OK] Successfully appended '$loginfofile' to '~/.bash_profile'"
else
    echo "[ER] Something went wrong appending '$loginfofile' to '~/.bash_profile'"
fi

Be aware that the logic of A && B || C is different from if A; then B; else C; fi in one subtle way: what happens when B exits non-zero?

in if A; then B; else C; fi when A succeeds but B fails, C will NOT be executed.
in A && B || C when A succeeds but B fails, C WILL be executed -- the || operator detects the previous command in the pipeline (B) exits non-zero and will execute its right-hand side command.

I'd recommend using select instead of a while loop to get your answer

PS3="Would you like to install 'loginfo'? "
select ans in Yes No; do
    case $ans in
        Yes) install_loginfo
            break
            ;;
        No) echo "'loginfo' was not installed."
            break
            ;;
    esac
done


Parsing of a (Linux) netlink hotplug uevent packet The netlink service is used, among other things, to notify userspace about hotplug events (e.g. "a device has been connected"). These events come in the form of packets, which follow the following format, explained here:

  The netlink packet contains a set of null terminated text lines. The
  first line of the netlink packet combines the $ACTION and $DEVPATH
  values, separated by an @ (at sign).  Each line after the first
  contains a KEYWORD=VALUE pair defining a hotplug event variable.

I have made an "SSCCE" of the code I use to parse these kinds of packets, and posted it on Ideone.com.  I'd like a review on potential risks and bad practices.

#include <cstdlib>

#include <vector>
#include <unordered_map>
#include <string>
#include <algorithm>
#include <iostream>

int main (void)
{
    /* Even if a std::string was used, a string literal would not work, because
    the fake packet contains '\0'. */
    const std::vector<char> msg = {
        'a','d','d','@','/','s','y','s','/','d','e','v','i','c','e','s','\0',
        'S','U','B','S','Y','S','T','E','M','=','b','l','o','c','k','\0','D',
        'E','V','T','Y','P','E','=','p','a','r','t','i','t','i','o','n','\0'
    };

    std::unordered_map<std::string , std::string> hotplug_event_variables;

    auto it1 = std::find( msg.begin() , msg.end() , '@' );
    if ( it1 != msg.end() )
    {
        /* As per the kernel documentation, the piece before the '@' token is
        the implicit "ACTION" event variable. */
        hotplug_event_variables.emplace( std::piecewise_construct , std::forward_as_tuple("ACTION") , std::forward_as_tuple(msg.begin(),it1) );

        /* It points to the '@'; increment it. */
        ++it1;

        auto it2 = std::find( it1 , msg.end() , '\0' );
        if ( it2 != msg.end() )
        {
            /* As with the previous event variable, DEVPATH is implicit. */
            hotplug_event_variables.emplace( std::piecewise_construct , std::forward_as_tuple("DEVPATH") , std::forward_as_tuple(it1,it2) );

            /* It points to the '\0'; increment it. */
            ++it2;

            /*
             * An example packet:
             *
             * Pass of the loop
             *                   |$ACTION@$DEVPATH0·····=·····0······=······0|
             *                           ^^       ^^    ^     ^^     ^      ^
             *                          it1      it2    |     ||     |      |
             * #1                                it3   it4   it5     |      |
             * #2                                             it3   it4    it5
            */
            for ( auto it3 = it2 , it4 = std::find( it2 , msg.end() , '=' ) , it5 = std::find( it4+1 , msg.end() , '\0') ; it3 != msg.end() , it4 != msg.end() , it5 != msg.end() ; it3 = it5+1 , it4 = std::find( it3 , msg.end() , '=' ) , it5 = std::find( it4 , msg.end() , '\0' ) )
            {
                /* While playing with the fake packet, I found that empty pairs
                could made it into the map. */
                if ( std::distance(it3,it4) > 0 && std::distance(it4+1,it5) > 0 )
                {
                    hotplug_event_variables.emplace( std::piecewise_construct , std::forward_as_tuple(it3,it4) , std::forward_as_tuple(it4+1,it5) );
                }
            }

            /*
             * Expected output:
             *
             * "ACTION"    = "add"
             * "DEVPATH"   = "/sys/devices"
             * "SUBSYSTEM" = "block"
             * "DEVTYPE"   = "partition"
            */
            for ( const auto & x : hotplug_event_variables )
            {
                std::cout << '"' << x.first << "\" = \"" << x.second << "\"\n";
            }
        }
    }

    return EXIT_SUCCESS;
}

More information about netlink may be found at the netlink(7) man page.

The empty line after the first #include is a little odd.  They can all be kept together and still be grouped by type (system, library, and local).

Also, consider ordering these in some way for organization, such as alphabetically.
void parameters are not needed on C++, unlike in C.
C++11 now supports list initialization, which you've mostly done here with msg.  You can omit the = to make full use of this.
This for loop is way too long horizontally:

for ( auto it3 = it2 , it4 = std::find( it2 , msg.end() , '=' ) , it5 = std::find( it4+1 , msg.end() , '\0') ; it3 != msg.end() , it4 != msg.end() , it5 != msg.end() ; it3 = it5+1 , it4 = std::find( it3 , msg.end() , '=' ) , it5 = std::find( it4 , msg.end() , '\0' ) )

You may be better off just using a while loop and making the calls to std::find() within the body of the loop.  Another option would be to wrap this line, but that may also look ugly.


A small module that allows threads to return their result I have the following that works similar to forkIO, but returns a blocking "channel" that lets you receive the result of a forked IO computation. My main concern is its handling of exceptions. It will return Nothing if something is raised, but can't supply any more information then that.

I tried using try inside of wrapAction, which returns an (Either Exception a), but I couldn't figure out what exception type to use. I tried SomeException thinking it would catch everything, and supply a message, but it doesn't. Running a computation that divides by 0 resulted in Right being returned, but threw a "divide by 0" exception when I tried inspecting the result. Surely in a generic situation like this I can't specifically catch every possible exception (given a user could create their own). How should I set up the exception handling so that a Leftcan be returned with an exception message?

import Control.Concurrent
import Control.Exception

data RThread a = RThread (IO a) (MVar (Maybe a))

newReturn :: IO a -> IO (RThread a)
newReturn act = newEmptyMVar >>= return . RThread act

wrapAction :: RThread a -> IO ()
wrapAction (RThread act chan) = onException (act >>= putMVar chan . Just) (putMVar chan Nothing)

returnFork :: IO a -> IO (RThread a)
returnFork act = do
    nR <- newReturn act
    forkIO $ wrapAction nR
    return nR

waitForResult :: RThread a -> IO (Maybe a)
waitForResult (RThread _ chan) = takeMVar chan

Any critique would be appreciated

Simple usage:

import System.Random
import Control.Monad

testFn :: IO [Int]
testFn = returnFork (replicateM 100 $ randomRIO (1,100) >>=
    waitForResult

Although realistically you would pass the result of waitForResult somewhere else to wait.

What you are experiencing is a common problem that occurs with threads and lazy evaluation. What happens is that the thread creates only an unevaluated thunk, which is passed through RThread to the main thread, and evaluated only then, and the exception is thrown there. This is also a very good example why using exceptions in pure values is a bad idea (but of course if you're making a library like this you have to take this possibility into account).

The solution is to fully evaluate a value inside the try block, so that any exception that might occur is thrown there. For this we need a combination of two functions: evaluate and force. The former creates an IO action that forces the evaluation, the latter one performs deep evaluation (to a normal form, not just to a weak head normal form).

Also you might also want to separate the return value from the thread action. If you're not going to operate on thread actions, you can even omit them from the data type, as once you start an action, all you need is the MVar channel.

An updated solution could look like this:

import Control.Applicative
import Control.Concurrent
import Control.DeepSeq
import Control.Exception

newtype RValue a = RValue (MVar (Either SomeException a))

returnFork :: (NFData a) => IO a -> IO (RValue a)
returnFork act = do
    chan <- newEmptyMVar
    forkIO $ try (act >>= evaluate . force) >>= putMVar chan
    return $ RValue chan

waitForResult :: RValue a -> IO (Either SomeException a)
waitForResult (RValue chan) = takeMVar chan

main = returnFork (return $ Just (1 `div` 0 :: Int)) >>= waitForResult >>= print


Basic form to create, load and edit entries (server-sided stored in a text file) To understand the basic workings of PHP I wrote a test page where one can create, load and edit entries (name-info-pairs) in a list.

It would be cool if you could give me some advice on that to improve.

<?php
class User {
  public $name;
  public $info;
  Public function __construct($name, $info){
    $this->name = $name;
    $this->info = $info;
  }
}

function show_user_radio_button($user) {
  return '<input type="radio" name="load" value="' .
  $user->name .'">' . $user->name . "
\n";
}

function read_user_from_text_line($line) {
  $cells = explode(",", $line);
  return new User($cells[0], $cells[1]);
}

function user_to_text_line($user) {
  return $user->name . ',' . $user->info;
}

function load_all_users() {
  $contents = file_get_contents('user_database.csv');
  $lines = array_filter(explode("\n", $contents));
  $allUsers = array();
  foreach ($lines as $line)
    $allUsers[] = read_user_from_text_line($line);
  return $allUsers;
}

function save_all_users($allUsers) {
  $contents = '';
  foreach ($allUsers as $user)
    $contents .= user_to_text_line($user) . "\n";
  file_put_contents('user_database.csv', $contents);
}
function save_user($newUser) {
  $allUsers = load_all_users();
  $replaced = false;
  for ($i = 0; $i < count($allUsers); $i++) {
    $user = $allUsers[$i];
    if ($newUser->name === $user->name) {
      $allUsers[$i] = $newUser;
      $replaced = true;
    }
  }
  if (!$replaced) {
    $allUsers[] = $newUser;
  }
  save_all_users($allUsers);
}
function loadUser($name) {
  $allUsers = load_all_users();
  foreach ($allUsers as $user)
    if ($user->name === $name) return $user;
  return new User('', '');
}
function sanitize($str) {
  return filter_var($str, FILTER_SANITIZE_EMAIL);
}

$newName = sanitize($_POST['name']);
$newInfo = sanitize($_POST['info']);
if ($newName && $newInfo)
  save_user(new User($newName, $newInfo));
?>

<!DOCTYPE html>

  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  <title>basic form to create and edit entries in a text file</title>

  from database:

  <form name="load" action="index.php" method="get">
    <?php
    $allUsers = load_all_users();
    foreach ($allUsers as $user)
      echo show_user_radio_button($user);
    echo '<input type="submit" value="load">';
    ?>
  </form>

  -------

  edit or create

  <form name="save" action="index.php" method="post">
    <?php
    $user = loadUser($_GET['load']);
    echo 'name: <input type="text" name="name" value="' .
    $user->name . '">
';
    echo 'info: <input type="text" name="info" value="' .
    $user->info . '">
';
    ?>
    <input type="submit" value="save">
  </form>

First of all, welcome to the world of PHP. It seems you are on the right track. I have a few pointers I can give you though:

I don't know if you pasted this all together for the question, but all that code should not be in a single file. While it is fine for this 'test' page, it will very quickly become a big mess if you start building real sites or applications. There are some nice patterns you can use to structure your code. MVC is the most popular in PHP, and I would recommend looking into that.
You start of very well with your User class, but then you resort to 'old school' functions. I would recommend you go OOP all the way. You will have to use built in functions when working with php unfortunately, but all the code you write should be in classes! Better to learn it the right way from the start.
I am not sure why you wrote that show_user_radio_button function. It could just as well be inside your view (the HTML bit at the bottom). Whenever you write a function with only a single line of code, you should wonder whether it is worth putting it inside a function.
All those functions that handle reading and writing Users to and from your text file should be in a class. I would call it my UserMapper, and it would have an API that looks something like this:

class UserMapper {
    private $_users = array();

    public function getById($id) { ... }
    public function getByName($name) { ... }
    public function getAll() { ... }
    public function save(User $user) { ... }

    private function _textLineToUser($line) { ... }
    ...
}

If you ever decide to store your Users in a different way, you could create a class that implements this same interface and just replace it in your application, and all should keep working.

The way you load and save users right now is not very performant. You have to go over your entire array of users when you save or look for a specific user. It would be much better to use your id (the name in your example) as a key in your array of users. That way you can easily:

check if a user exists with array_key_exists
load a specific user with $users[$name]
update a user by just setting $users[$name] = new User($name, $info). No need to check if it exists as it will overwrite the array value automatically if that is the case.

There is probably a lot more that could be said about your code, but I leave that up to others. This should already give you some food for thought and point you in the right direction. Do feel free to ask if anything I wrote isn't clear, and happy coding!


Authentication management I have the following code and I think it's almost impossible to read and to maintain. I prefer self-explaining and modular code where variable and function names tell what I'm trying to do, but I don't know how to change the part using Eval and inside an ascx.

I'm doing much research but I still cannot understand it completely well. The performance of this part is not critical, the readability is!

How can I refactor it and split into many variable assignments?

<tr class="GridRowE" visible='<%#((String.IsNullOrEmpty(Eval("IDLanguage").ToString()) OrElse Eval("IDLanguage").ToString().Replace(" ", "").Split(","c).Contains(WebProfile.Current.IDLanguage.ToString())) And (String.IsNullOrEmpty(Eval("Roles").ToString()) OrElse Roles.IsUserInRole(WebProfile.Current.UserName, Eval("Roles").ToString())))%>' runat="server">

ASP.NET and code-behind

One of the main advantages of ASP.NET is the code-behind model. This ensures that the logic and the markup are separated in different files. The markup's goal is to display data, not to determine whether controls should be visible or not. So, your code is in violation with that principle.

The Evil of Eval

I suggest your read following article: The Evil of Eval() in ASP.NET. Here's one of the many reasons:

  Eval takes about 1/20th of a second per call, and the other way takes about 1/1000th of a second per call. On a grid with 5 columns and 8 rows, Eval would take 2 seconds and the other would take under 1 second. Nobody would notice the difference. However, change that grid to 10 columns and 50 rows. You now have a difference of 25 seconds. It is the kind of thing that might not matter very much, most of the time but when it does, it makes a big difference.

The article contains more information and links to other resources.

Solution

Make the tr server-side by adding runat="server" to to the tag and give it an ID. Now you can access the tr-tag from the code-behind and set it's visibility property. Example:

Markup

    This is not visible

Code-behind

Dim isVisible As Boolean = False
'apply logic to determine whether "isVisible" should be true or false
myRow.Visible = isVisible

Solution for the Repeater - ItemTemplate

Here's a solution that works when your data resides in an ASP.NET Repeater control. Following is a sample markup:

<asp:Repeater ID="PeopleRepeater" runat="server">
    <HeaderTemplate>

    </HeaderTemplate>
    <ItemTemplate>

    </ItemTemplate>
    <FooterTemplate>

    </FooterTemplate>
</asp:Repeater>

And here's the code that does the magic:

Sub PeopleRepeater_Bind(ByVal sender As Object, ByVal e As RepeaterItemEventArgs)
    Handles PeopleRepeater.ItemDataBound

    If e.Item.ItemType = ListItemType.Item Or
       e.Item.ItemType = ListItemType.AlternatingItem Then

        Dim isRowVisible As Boolean = False 'Your condition here
        CType(e.Item.FindControl("UserRow"), HtmlTableRow).Visible = isRowVisible

    End If

End Sub

The code checks if the current bound item is an item or an alternating item. This is necessary because the binding of the header and footer will also trigger this event. If this is the case, prepare your boolean variable to set the row visible or not. Lastly, get the table row tag using the FindControl method and set its visibility to the variable.


How can I Implement a better version of my function find the closest number to zero? I recently had to do a test for a job interview, and the prompt was to code a function that returns the closest number to 0 given an array of negative and positive numbers.

My function works correctly but I heard back from the recruiter saying that my function is not very performant and that the readability was not great, here are the problem that they mentioned:

Performances à double check (filter + sort + for + push + reserve + shift, 4 identical call to Math.abs)
Lack of readability ( 4 if / else if)
I would like to know how I could have improved my function to make it more performant and more readable?

here's my function:

const closestToZero = (arr) => {
  // 1-a variables:
  let positiveNumbers = [];
  let negativeNumbers = [];

  // 1-b returns 0 if the input is either undefined or an empty array
  if(typeof(arr) === 'undefined' || arr.length === 0) {
    return 0
  }

  // 2- filter out non-number values then sort the array
  const onlyNumbers = arr.filter(item => typeof(item) === 'number').sort((a,b) => a-b);

  // 3- split the numbers into positive numbers and negative numbers
  for(let i = 0; i < onlyNumbers.length; i++) {
    if(onlyNumbers[i] > 0) {
      positiveNumbers.push(onlyNumbers[i])
    } else if (onlyNumbers[i] < 0) {
      negativeNumbers.push(onlyNumbers[i])
    }
  }

  // 4- reverse the negative array to get the closest to 0 at the first index
  let reversedNegativeArray = negativeNumbers.reverse()

  // 5- get rid of all the other values and keep only the closest to 0, if array empty return 0
  let closestPositiveNumber = positiveNumbers.length > 0 ? positiveNumbers.shift() : 0
  let closestNegativeNumber = reversedNegativeArray.length > 0 ? reversedNegativeArray.shift() : 0

  // 6- Compare the result of the substraction of the closestPositiveNumber and the closestNegativeNumber to find the closestToZero
  if(closestPositiveNumber - Math.abs(closestNegativeNumber) > 0 && closestNegativeNumber === 0 ) {
    return closestPositiveNumber
  } else if (closestPositiveNumber - Math.abs(closestNegativeNumber) < 0 && closestPositiveNumber === 0) {
    return closestNegativeNumber
  } else if(closestPositiveNumber - Math.abs(closestNegativeNumber) > 0) {
    return closestNegativeNumber
  } else if (closestPositiveNumber - Math.abs(closestNegativeNumber) <= 0) {
    return closestPositiveNumber
  }
}

requirements:

if the closest number in input could be either negative or positive, the function returns the positive one
if the input array is undefined or empty, the function returns 0

when input is [8, 5, 10] the function returns 5

when input is [5, 4, -9, 6, -10, -1, 8] the function returns -1

when input is [8, 2, 3, -2] the functions return 2

Your reviewers are correct. You are doing way too much work here. To find the number closest to zero you only need to find the smallest absolute value in the list. You should also clarify in the interview if the list can contain non-numeric values. There's no reason to test that the values are numbers unless you need to for the purpose of the review, although asking about this shows that you are thinking about edge cases, which is good.

Either way, this can be done in a single loop with no storage other than keeping track of the current minimum:

function absSmallest(arr){
   if (!arr || arr.length === 0 ) return 0            // address requirement to return 0 when arr is undefined or empty
   let res = undefined                                // smallest value seen so far
   for (let i = 0; i < arr.length; i++){
      if (res === undefined || Math.abs(arr[i]) <= Math.abs(res)){
         res = Math.abs(arr[i]) === Math.abs(res)    // address requirement of positive value when there's a tie
         ? Math.max(res, arr[i] )
         : res = arr[i]
      }
   }
   return res
}

console.log(absSmallest([5, 4, -9, 6, -10, -1, 8] ))
console.log(absSmallest([8, -2, 3, 2] ))
// check order of tied values
console.log(absSmallest([8, 2, 3, -2] ))

// edge case empty list returns 0:
console.log(absSmallest())
console.log(absSmallest([]))


Bisection method for finding the root of a function For those who aren't familiar with the bisection method for finding the root of a function (i.e. finding where \$f(x) = 0\$) the basic idea is:

Take a function \$f(x)\$ and an interval \$[a,b]\$
If \$[f(a)] \cdot [f(b)] < 0\$, there is a root in the interval, as one of those is a negative value
Repeatedly half the interval until you find an acceptable root within a maximum error of \$\varepsilon\$

I have the following code to emulate this process:

float bisect(float(*f)(float), float e, float a, float b){
    if (f(a)*f(b) > 0) { printf("Invalid interval - returning\n");  return 0; }
    float c;
    while (1){
        c = (a + b) / 2;
        if ( ((b-c) / c) <=e ) break;
        f(b)*f(c) <= 0 ? a = c : b = c;
    }
    return c;
}

Which I would pass a test function representing \$f(x) = x^{3}-x-2\$ with \$\varepsilon = 10^{-6}\$ and check for a root in the interval \$[a,b]\$.

typedef float (*func)(float);

float test(float x){
    return (x*x*x) - (x) - 2;
}

int main(){
    func f = &test;
    double start, end, ans;

    start = get_time();
    ans = bisect(f, .00001f, 1, 2);
    end = get_time();

    printf("The root is %f and took %e seconds", ans, end-start);
    return 0;
}

This code works, and returns 1.521385, which properly approximates the actual value of 1.5213928 (the actual error is 0.0000078).

I'm on Windows right now (using Visual Studio 2013), and I'm using this function for the benchmarking:

#include <windows.h>
double get_time()
{
    LARGE_INTEGER t, f;
    QueryPerformanceCounter(&t);
    QueryPerformanceFrequency(&f);
    return (double)t.QuadPart / (double)f.QuadPart;
}

I was interested to see if there were a way to optimize any of this to make it run as fast as possible.

It is customary to use eps instead of e.
A ((b-c) / c) <=e condition doesn't look right (try \$f(x) = x\$ on the \$(-1, 1)\$ interval and observe a division by 0).
You seem to assume that b > a (otherwise the loop breaks immediately). Make sure to swap a and b if it is not so. Making sure that eps is positive also helps.
The code produces wrong results for a non-continuous \$f\$. I recommend to test that the function value at a suspected root is close enough to \$0\$.
Avoid break. while (b - a > eps) is much cleaner.