A class for simple and efficient operations on boost::unordered_set I wrote this class to perform simple template-set operations on boost::unordered_set. These operations are part of a larger code base. Calling them will be explicit with SetUtilities::set_intersection(setA, setB).

The main reason I wanted this was to optimize intersection operations (using constant time find in the larger set). After taking to heart the comments from here, I edited the class and provided the rest of the methods.

For now I want to use Boost instead of C++11. I can always do this in the future.

#ifndef SET_UTILITIES
#define SET_UTILITIES

#include <boost/unordered_set.hpp>
#include <boost/foreach.hpp>

class SetUtilities{

public:

    //!set OPERATION intersection for boost::unordered_set
    template<typename Type>
    static boost::unordered_set<Type>
        set_intersection(const boost::unordered_set<Type> &setA,
                         const boost::unordered_set<Type> &setB)
    {
        //iterate the smaller set O(n)
        if(setA.size() <= setB.size()){
            boost::unordered_set<Type> intersectionSet;
            BOOST_FOREACH(const Type &element, setA){
                //find element in O(1)
                if(set_hasElement(setB,element)){
                    intersectionSet.insert(element);
                }
            }
            return intersectionSet;
        }else{
            return set_intersection(setB,setA);
        }
    }

    //!set OPERATION union for boost::unordered_set
    template<typename Type>
    static boost::unordered_set<Type>
        set_union(const boost::unordered_set<Type> &setA,
                  const boost::unordered_set<Type> &setB)
    {
        if(setA.size() <= setB.size()){
            boost::unordered_set<Type> unionSet = setB;
            //iterate the smaller set O(n)
            BOOST_FOREACH(const Type &element, setA){
                unionSet.insert(element);
            }
            return unionSet;
        }else{
            return set_union(setB,setA);
        }
    }

    //!set OPERATION difference A - B
    template<typename Type>
    static bool
        set_difference(const boost::unordered_set<Type> &setA,
                       const boost::unordered_set<Type> &setB)
    {
        boost::unordered_set<Type> differenceSet;
        //is size are not equal, sets are not equal
        BOOST_FOREACH(const Type &element,setA){
            if(set_hasElement(setB,element) == false){
                differenceSet.insert(element);
            }
        }
        return differenceSet;
    }

    //!set UTILITY funciton to determine if a set contains an element
    template<typename Type>
    static bool
        set_hasElement(const boost::unordered_set<Type> &mySet,
                       Type element)
    {
        return mySet.find(element) != mySet.end();
    }

    //!set equality test for boost::unordered_set
    template<typename Type>
    static bool
        are_equal(const boost::unordered_set<Type> &setA,
                  const boost::unordered_set<Type> &setB)
    {
        //is size are not equal, sets are not equal
        if(setA.size() != setB.size()){
            return false;
        }else{
            BOOST_FOREACH(const Type &element, setA){
                if(set_hasElement(setB,element)){
                    return false;
                }
            }
            return true;
        }
    }
};
#endif

I believe that this version is very readable and explicit. I think that adding typedefs with stuff like this in a single class would create more confusion. Let me know what you think.

Fix compiler errors

Your set_difference function does not compile. You have declared it returning bool, but then return differenceSet; in the function body. This is a type mismatch: your function should return a boost::unordered_set.

Reinventing the wheel

boost::unordered_set already supports operator== for equality comparison. You don't need to implement it yourself -- your are_equal() function is unnecessary.

Additionally, your set_union can be simplified. You can replace your foreach-loop:

BOOST_FOREACH(const Type &element, setA){
            unionSet.insert(element);
        }

with using the two-iterator overload of insert():

unionSet.insert(setA.begin(), setA.end());

This should be no less efficient than what you have already written, and is slightly more readable.

Remove irrelevant comments

set_difference() contains this comment: //is size are not equal, sets are not equal. This has no relevance to what the function actually does. This, combined with the type error listed above, leads me to believe that this is a copy-paste error. When you do that, make sure things like this don't slip through.

Boolean comparison

You don't need to explicitly compare Boolean conditions against false:

if(set_hasElement(setB,element) == false)

Can be more succinctly stated:

if(!set_hasElement(setB,element))

Naming

This is a minor nitpick, but set_hasElement combines underscore_separated and CamelCase capitalization conventions. I would rename it to set_contains() for name consistency with the rest of your functions.

Namespaces

Creating a class with only public static functions is very Java-esque. C++ allows namespaces, which are a way of organizing functions and classes. Your set functions might be better as free functions placed in a namespace SetUtilities instead of a class.


UnitOfWork as Repository object factory I have done some research about intentions of UnitOfWork pattern. In short, what I understood is that UoW is needed for transactions. But I saw some code examples where people use UoW as factory for Repository objects as well, one example is DbContext from EF. My first thought about it doesn't it break single responsibility principle?
In my current project I am using both UoW and Repository patterns. Here are my interfaces:

UnitOfWork:

public interface IUnitOfWork : IDisposable
{
    void Commit();
    void Rollback();
    IRepository<T> GetRepository<T>() where T : class;
}

Repository interface:

public interface IRepository<T> where T : class
{
    IQueryable<T> FindAll();
    IQueryable<T> Find(Expression<Func<T, bool>> predicate);
    T FindById(Id id);
    void Add(T newEntity);
    void Remove(T entity);
}

My concern is GetRepository<T> method in IUnitOfWork interface. Should this method be here at all? Or should I move it somewhere else? What would you suggest?

PS
I am using NHibernate as my ORM tool, but I think this should not make any difference

Leaky abstractions
A specific implementation should not leak into an abstraction. By making IUnitOfWork extend IDisposable, that's what you're doing:

public interface IUnitOfWork : IDisposable

The goal of the abstraction, is to abstract implementation details away - in this case, the fact that you're using NHibernate. You had a specific implementation in mind when you created this interface, and that implementation leaked into the abstraction. What if you wanted to implement an IUnitOfWork that worked off a List<T>, for testing purposes?
Not all implementations of IUnitOfWork need to implement IDisposable - better leave that up to the implementation:
public class NHibernateUnitOfWork : IUnitOfWork, IDisposable
{
    private readonly ISession _session;

    /* implementation */

    public void Dispose()
    {
        _session.Dispose();
    }
}

public class TestUnitOfWork : IUnitOfWork
{
    /* implementation */
}

IQueryable
Every single implementation of the Repository & Unit of Work patterns I have seen, were different. Every single one.
Without seeing what your implementations and client code looks like (/how you're using these interfaces), it's very hard to tell whether you've done something wrong.
For one thing, exposing IQueryable<T> is leaking the query outside your repository, making it possible for client code to call IRepository<T>.FindAll() and then from there, add a Where clause or Join on another repository, which means the repository completely loses control of query materialization, which [unless I missed something] defeats its purpose.
By exposing a IRepository<T> GetRepository<T>() method on your IUnitOfWork, you're also making it possible for the unit of work's clients to own the materialization of the query.
Whether that's good or bad, entirely depends on your architecture; I cannot tell just from the interfaces... I can only raise a flag.


Merge Sort Implementation Any reviews/ways to speed up/general improvements for my Merge Sort? Maybe something to simplify it or add more to it?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace prjT02L08_Predator_Prey
{
    abstract class MergeSort
    {
        public static List<Node> Sort(List<Node> input)
        {
        List<Node> Result = new List<Node>();
        List<Node> Left = new List<Node>();
        List<Node> Right = new List<Node>();

        if (input.Count <= 1)
            return input;

        int midpoint = input.Count / 2;
        for (int i = 0; i < midpoint; i++)
            Left.Add(input[i]);
        for (int i = midpoint; i < input.Count; i++)
            Right.Add(input[i]);

        Left = Sort(Left); //Recursion! :o
        Right = Sort(Right);
        Result = Merge(Left, Right);

        return Result;
    }

    private static List<Node> Merge(List<Node> Left, List<Node> Right)
    {
        List<Node> Result = new List<Node>();
        while (Left.Count > 0 && Right.Count > 0)
        {
            if (Left[0].F < Right[0].F)
            {
                Result.Add(Left[0]);
                Left.RemoveAt(0);
            }
            else
            {
                Result.Add(Right[0]);
                Right.RemoveAt(0);
            }
        }

        while (Left.Count > 0)
        {
            Result.Add(Left[0]);
            Left.RemoveAt(0);
        }

        while (Right.Count > 0)
        {
            Result.Add(Right[0]);
            Right.RemoveAt(0);
        }

        return Result;
    }
}
}

abstract class MergeSort

This class shouldn't be abstract, it should be static.

public static List<Node> Sort(List<Node> input)

Why does the method require List? Wouldn't it be better if it accepted any IList (or IReadOnlyList if you're on .Net 4.5)? Also, it would make sense to make your code generic, so that it would work on types other than Node.

List<Node> Result = new List<Node>();

The usual convention in C# is to name local variables in camelCase, not PascalCase.

return input;

Returning the input might be unexpected. If you don't want to change it, you should clearly document it.

Left.RemoveAt(0);

This is very inefficient operation for a List. Either use a collection that can do this efficiently (like LinkedList or Queue) or, even better, just use indexes instead of modifying the collection.

Your code also does a lot of copying and creating new lists. That's good for readability, but if you want your code to be fast, you need just two lists that you can modify.

You would need to work with sublists (list, start index, length), not whole lists. First, merge the 1-element sublists from list A to list B, then merge the 2-element sublists from B to A, etc.

Also, another optimization is not to start with 1-element sublists, but take advantage of the fast that there are likely to be some sorted sublists in the input already. This is especially effective if the input is already sorted (or mostly sorted). But I think it wouldn't work with your recursive approach.


"Vaishnav and Pizzas" challenge I am trying to solve the below mentioned problem from HackerEarth.

Although the results are fine, the code takes extraordinarily long time to run execute. I am pretty sure the problem is because of the nested loops, and I am working on it.

I would like to know:

How can this code be optimized to run within 1 sec?
What are the general programming principles to optimize your execution time, say while writing embedded applications with time constraints.

#include <iostream>
#include <vector>
using namespace std;

int main(int argc, char * argv[])
{
   int testcase;
   cin >> testcase;
   int Number[10000];
   for(int i = 0; i < testcase ; i++)
   {
      cin >> Number[i];
   }

   float numerator, denominator,result;
   //float myArr[100000];
   vector<float> myArr;

   for(int i = 0; i < testcase ; i++)
   {
      for(int j = 1; j <= Number[i] ; j++)
      {
         for(int k = 1; k <= Number[i] ; k++)
         {
            numerator = j;
            denominator = k;
            //cout << numerator << denominator << endl;
            if(numerator < denominator)
            {
               result = numerator/denominator;
               //cout << result << endl;
               bool found = false;
               for(int x=0; x < myArr.size() ; x++)
               {
                  if(result == myArr[x])
                  {
                     found = true;
                     break;
                  }
               }
               if(found == false)
               {
                  myArr.push_back(result);
               }
            }
         }
      }
      cout << myArr.size() << endl;
      myArr.clear();
   }

   return 0;
}

Yes, the execution time amounts to O(n^3) because of the nested loops. A brute force approach cannot be healed; you need to change the algorithm.

A given denominator Q produces exactly phi(Q) rational numbers, so the problem boils down to calculating $$ \sum \phi (k) $$

There are many approaches for doing that. For the small numbers (according to constraints, less than 10000) I'd recommend a straightforward use of multiplicative property. A precomputed table of primes may also help.


Very basic networking in Java This question presents two rather small Java programs: the first one is a simple client that reads text from command line and sends it to the server (TextSender.java); the server does nothing more but echoes the received text on the command line (TextReceiver.java)

TextSender.java:

import java.io.IOException;
import java.io.PrintWriter;
import java.net.Socket;
import java.net.UnknownHostException;
import java.util.Scanner;

public class TextSender {

    public static final int PORT = 1857;

    public static void main(String[] args) {
        if (args.length == 0) {
            System.out.println("Usage: java Main IP");
            return;
        }

        try (Socket clientSocket = new Socket(args[0], PORT);
             PrintWriter out = new PrintWriter(clientSocket.getOutputStream(), true);) {
            Scanner scanner = new Scanner(System.in);

            while (scanner.hasNextLine()) {
                String line = scanner.nextLine();

                if (line.trim().equals("quit")) {
                    break;
                }

                // Terminate the line so that the receiver process knows that
                // the end of input is occured.
                out.print(line + "\n");
                out.flush();
            }
        } catch (UnknownHostException ex) {
            System.err.println("[ERROR] Could not reach host \"" +
                               args[0] + "\".");
            System.exit(1);
        } catch (IOException ex) {
            System.err.println("[ERROR] IO failed on host \"" +
                               args[0] + "\".");
            System.exit(1);
        }
    }
}

TextReceiver.java:

import java.io.BufferedReader;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.Scanner;
import java.util.logging.Logger;

public class TextReceiver implements Runnable {

    public static final int PORT = 1857;

    public static void main(String[] args) {
        new Thread(new TextReceiver()).start();
    }

    @Override
    public void run() {
        ServerSocket serverSocket = null;
        Socket clientSocket = null;
        BufferedReader in = null;

        try {
            serverSocket = new ServerSocket(PORT);
            System.out.println("[RECEIVER] Waiting for a client.");

            clientSocket = serverSocket.accept();

            in = new BufferedReader(
                    new InputStreamReader(clientSocket.getInputStream()));

            System.out.println("[RECEIVER] A client connected.");

            Scanner scanner = new Scanner(in);

            while (scanner.hasNextLine()) {
                System.out.println(scanner.nextLine());
            }
        } catch (IOException ex) {
            Logger.getLogger("CONNECTION").severe(ex.getMessage());

            if (in != null) {
                close(in, clientSocket, serverSocket);
            } else if (clientSocket != null) {
                close(clientSocket, serverSocket);
            } else if (serverSocket != null) {
                close(serverSocket);
            }
        }

        close(in, clientSocket, serverSocket);
    }

    private void close(Closeable... closeables) {
        for (Closeable c : closeables) {
            try {
                c.close();
            } catch (IOException ex) {

            }
        }
    }
}

Please, tell me anything I could improve.

Why define PORT as a constant independently in TextReceiver and TextSender? Isn't it the whole point that it's one and the same value? Let's stick to Single Source of Truth then. And if not, why make them public?
I'm not sure if this is necessary:

if (in != null) {
    close(in, clientSocket, serverSocket);
} else if (clientSocket != null) {
    close(clientSocket, serverSocket);
} else if (serverSocket != null) {
    close(serverSocket);
}

I'd rather have close skip over null values when iterating through closeables.
It could be a better choice to use a finally clause for closing resources (in TextReceiver). It executes regardless of whether there was an exception or not.
If we're logging all other exceptions, I can't see the reason not to log the IOException in close.


Download a zip archive and extract one file from it I wrote a function that downloads a file https://www.ohjelmointiputka.net/tiedostot/junar.zip if it not already downloaded, unzips it and returns the content of junar1.in found in this zip. I have PEP8 complaints about the length of lines that I would like to fix. Is there a way to make the code more readable?

My code :

import os.path
import urllib.request
import shutil
import zipfile

def download_and_return_content():
    if not os.path.isfile('/tmp/junar.zip'):
        url = 'https://www.ohjelmointiputka.net/tiedostot/junar.zip'
        with urllib.request.urlopen(url) as response, open('junar.zip', 'wb') as out:
            data = response.read() # a `bytes` object
            out.write(data)
        shutil.move('junar.zip','/tmp')
    with zipfile.ZipFile('/tmp/junar.zip', 'r') as zip_ref:
        zip_ref.extractall('/tmp/')
    with open('/tmp/junar1.in') as f:
        return f.read()

PEP-8 Line Length

For particularly lengthy lines, you can always use parentheses to wrap them instead of the usual \ sign:

x = (1 + 2 + 3 +
     4 + 5 + 6)

Function Refactor

I would skip the step in your if statement where you use shutil.move, just save the file in /tmp directly:

def download_and_return_content():
    if not os.path.isfile('/tmp/junar.zip'):
        url = 'https://www.ohjelmointiputka.net/tiedostot/junar.zip'
        with urllib.request.urlopen(url) as response, open('/tmp/junar.zip', 'wb') as out:
            data = response.read() # a `bytes` object
            out.write(data)

Furthermore, if you are just looking to extract a single file, you can open one of the archives directly using ZipFile.open

    with ZipFile('/tmp/junar.zip') as myzip:
        with myzip.open('junar1.in') as f:
            return f.read()

ZipFile can also take a file-like object, so you can use a BytesIO object to hold your zip-file bytes, since /tmp implies you might not need to hold onto this data:

from io import BytesIO

def download_and_return_content():
    # your temporary file-handle
    tmp_file = BytesIO()

    url = 'https://www.ohjelmointiputka.net/tiedostot/junar.zip'
    with urllib.request.urlopen(url) as response:
        tmp_file.write(response.read())

    tmp_file.seek(0)

    with ZipFile(tmp_file) as myzip:
        with myzip.open('junar1.in') as fh:
            return fh.read()

Lastly, the if check implies that maybe you want to cache the data somehow. You could in theory use BytesIO as a mutable default. You can use BytesIO.tell() as your check if it has content:

def get_content(tmp=BytesIO()):
    # buffer is at position 0, it has not been read or written to
    # therefore it is probably empty
    if not tmp.tell():
        tmp.truncate() # just in case
        url = 'https://www.ohjelmointiputka.net/tiedostot/junar.zip'
        with urllib.request.urlopen(url) as response:
            tmp.write(response.read())

    # set buffer to position 0 to read content
    tmp.seek(0)

    with ZipFile(tmp) as myzip:
        # this will move the buffer to a non-zero position
        # so, now tmp.tell() will be non-zero and will pass the
        # if check on the next function call
        with myzip.open('junar1.in') as fh:
            return fh.read()

As a caveat, there are caching libraries in python that can accomplish this as well, I'm just not familiar enough with them to suggest any in a meaningful way.

Before everybody grabs their torches and pitchforks, the non-mutable-default way (usually mutable defaults are seen as bad design) could look something like this:

# refactor into two functions, one that does the actual urllib call
# for you to retrieve your data
def get_data(tmp=None):
    tmp = tmp or BytesIO()

    with urllib.request.urlopen(url) as response:
        tmp.write(response.read())

    return tmp

# and one to actually extract the file
def extract_file(tmp=None):
    tmp = tmp or get_data()

    tmp.seek(0)

    with ZipFile(tmp) as myzip:
        with myzip.open('junar1.in') as fh:
            return fh.read()

# now you can hold that BytesIO object
tmp_file = get_data()

content = extract_file(tmp_file)
# returns b'10\r\n6\r\n1\r\n4\r\n10\r\n7\r\n2\r\n3\r\n9\r\n5\r\n8\r\n'

# and if you want to write that temp file somewhere
with open('/tmp/somefile.zip', 'wb') as fh:
    tmp_file.seek(0)
    fh.write(tmp_file.read())

Of course this all depends on what you need that zipfile for, but this cuts down on the amount of reads and writes you are doing under the hood.


Deciding between four user statuses based on two flags I think that the following code isn't really nice. I can't see however another way (other than a switch statement) to better implement this. Especially since I don't think replacing this with the strategy pattern for instance is a better way.

String status;
if(userHasContact&&contactHasUser) {
    status = "HAS_CONTACT";
} else if(userHasContact&&!contactHasUser) {
    status = "PENDING";
} else if(!userHasContact&&contactHasUser) {
    status = "ACCEPT_REQUEST";
} else {
    status = "ADD_CONTACT";
}

I want to return the value of status to the view (this code is from the controller) of the page to show to the user whether that profile he is seeing is already a contact, pending (he already sent a status), accept (it was sent to him), or remove (he already has him as contact).

I think it is time to bring out the enum!

Enums in Java are pretty handy overall when there is a fixed number of possible values. In this case there is only four possible values, so an enum can be a good candidate for the job.

public enum Status {
    HAS_CONTACT(true, true),
    PENDING(true, false),
    ACCEPT_REQUEST(false, true),
    ADD_CONTACT(false, false);

    private final boolean hasContact;
    private final boolean hasUser;

    private Status(boolean hasContact, boolean hasUser) {
        this.hasContact = true;
        this.hasUser = true;
    }

    public static Status findStatusWith(boolean hasContact, boolean hasUser) {
        for (Status status : Status.values()) {
            if (status.hasContact == hasContact && status.hasUser == hasUser) {
                 return status;
            }
        }
        return null;
    }
}

You can get the string representation of the enum by calling .name() or .toString()

Status status = Status.findStatusWith(userHasContact, contactHasUser);
String statusDescription = status.name();

Overall I don't recommend using Strings for this kind of data, you have no guarantee that your String will only have one of the four possible values. What if the String suddenly says "ACCPT_SMTHNG"? An enum is much more preferable in this way.


JavaScript coding tips on creating a string from variables tags To familiarize myself with Javascript (and in particular NodeJS) I embarked on a concrete project.
In this project I need to "build" the URLs of different IP cameras.
All Cameras Information are in a JSON file
Everything works correctly, but I think to use some "bad" coding part and that there is certainly better to do (not always easy to adapt and learning to a new language)
Today I therefore submit my code to you for your criticism/advice/Optimization/...
Extract from the URL creation code:
const cameras_cfg = require("./cfg/cameras.json");
const replaceAllTagsToValue = (obj, chaine) => obj.reduce((f, s) => `${f}`.replace(new RegExp(Object.keys(s)[0], 'g'), s[Object.keys(s)[0]]), chaine)

makeCameraUrl = function (camera, type, flux, user) {
    var url = camera.url;
    var Obj = {
        "@@login@@": user.login,
        "@@passwd@@": user.passwd,
        "@@ip@@": camera.ip,
        "@@stream@@": camera.flux[type].stream,
        "@@port@@": camera.flux[type].port,
        "@@flux@@": camera.flux[type][flux]
    };
    return replaceAllTagsToValue(Obj, url)
}

let cam_id = "102";
let url = makeCameraUrl(cameras_cfg.devices[cam_id], "rtsp", "hd", cameras_cfg.users.view);
console.log("Url:", url);

Extract from the cameras JSON file:
{
    "users": {
        "admin": {
            "login": "admin",
            "passwd": "xxxxx"
        },
        "view": {
            "login": "user",
            "passwd": "yyyyyy"
        }
    },
    "devices": {
        "102": {
            "id": 102,
            "name": "Cam 102 PTZ",
            "ip": "192.168.0.102",
            "url": "@@stream@@://@@login@@:@@passwd@@@@@ip@@:@@port@@/@@flux@@",
            "flux": {
                "rtsp": {
                    "stream": "rtsp",
                    "port": 554,
                    "hd": "1",
                    "sd": "2"
                }
            }
        },

        ../..

        "106": {
            "id": 106,
            "name": "Cam 106",
            "ip": "192.168.0.106",
            "url": "@@stream@@://@@ip@@:@@port@@/user=@@login@@&password=@@passwd@@&@@flux@@",
            "flux": {
                "rtsp": {
                    "stream": "rtsp",
                    "port": 554,
                    "hd": "channel=1&stream=0.sdp",
                    "sd": "channel=2&stream=0.sdp"
                }
            }
        }
    }
}

Thank you in advance for helping me to progress in the right techniques !

Fun project, but I feel like you have overengineered this quite a bit... Why not just build the url from the given values without all the "@@"? I see there is some difference between URLs, but my first suggestion would be to make them uniform and have a simple function that builds the url from the given components to reduce the complexity enormously.
But ok, let us focus on your code and how it can be improved.
I copied your code and it didn't run. In your function replaceAllTagsToValue, you are calling .reduce on a value, which will always be an object in your code. You cannot reduce an object so I don't know how this "works correctly" or, in fact, works at all. Nevermind that, I understand what you're trying to do! Let's have a look at my suggested edits:
const translator = {
  "@@login@@":  (user) => user.login,
  "@@passwd@@": (user) => user.passwd,
  "@@ip@@":     (_, camera) => camera.ip,
  "@@stream@@": (_, camera, type) => camera.flux[type].stream,
  "@@port@@":   (_, camera, type) => camera.flux[type].port,
  "@@flux@@":   (_, camera, type, flux) => camera.flux[type][flux]
};

const makeCameraUrl = (user, camera, type, flux) =>
  Object.entries(translator)
  .reduce(
    (url, [key, value]) => url.replace(key, value(user, camera, type, flux)),
    camera.url);

const user = json.users.view;
const camera = json.devices[102];
const type = "rtsp";
const flux = "hd";
makeCameraUrl(user, camera, type, flux);
// > "rtsp://user:yyyyyy@192.168.0.102:554/1"

You are actually doing a lot of nice stuff:

Properly using const for immutable values
Using arrow notation for simple one-line functions
Tricky higher-order array functions such as reduce

But there seems to be a problem with every one of those points. To start:

⚛ constantly changing
In javascript, you have four ways of declaring (I know, JS is horrible) a variable: var, let, const or none of those to define a global. For the sake of brevity: never use var (it's weird) and never make globals (this pollutes your namespace and maybe even other people's code).
As for let and const, they function identically to eachother but with an important difference: const declares a variable (or a constant, rather) that is immutable -- it cannot be changed after declaration.
const immutable = 1;
immutable = 2;
// > TypeError: Assignment to constant variable.

You can't reassign it.
This is useful for technical reasons (your javascript engine could optimise your code under the hood), but also for reasons of readability and deliberateness. When you use const, you're telling me, the reader, "I'm declaring a value here that I will not change later on, don't worry." I know I can trust this because javascript wouldn't even let you re-assign it even if you tried. You start off your program with two consts, but then you drop it and create a global :
makeCameraUrl = function (camera, type, flux, user) {
  //...
}

You have now created a function that will take up memory forever until you explicitly remove it from the global scope, and maybe you have just overwritten some other global value with that name... Globals, like I said, are a nightmare and I strongly advice against ever using them (like this anyways). The simplest fix here is to just plonk a const before it and we're save again:
const makeCameraUrl = function (camera, type, flux, user) {
  //...
}

 lets set this straight
But then, you're suddenly using let...
let cam_id = "102";
let url = makeCameraUrl(cameras_cfg.devices[cam_id], "rtsp", "hd", cameras_cfg.users.view);

The purpose of let is to create a mutable variable; its value can change at any time, in any place. Personally I think that's not a wise way to treat a value because we humans are dumb and history has shown us we are not good at keeping track of mutation of our variables.
But alright, if you have to change a value then go ahead and use let. But what I'm seeing here are values that never change. I know this because I read through the entire code, but I would have liked to know this from the start. When you use let, you're telling the reader "Hey, I'm defining a value here but I'm going to change it later on." When you never change a variable you've defined with let, you're lying  -- and code should never lie.
Again, the simplest fix here is to just make it const:
const cam_id = "102";
const url = makeCameraUrl(cameras_cfg.devices[cam_id], "rtsp", "hd", cameras_cfg.users.view);

 Consistency
Not only did you use three ways of defining variables, you also use two ways of defining functions. Again, all these inconsistencies make your code have a high amount of WTF/minute.

It doesn't matter wich way you define functions, as long as you are deliberate about it. In javascript there's only one reason to differentiate between function types but that doesn't apply to your code, so I suggest you just use arrow functions like you did in the beginning:
const makeCameraUrl = (camera, type, flux, user) => {
  //...
}

This code tells me there is nothing fancy going on besides a plain old function -- just how I like them.
This matter of consistency also applies to the way you name things. You use both snake_case and camelCase, but I couldn't really find a reason for the difference.
const cameras_cfg = //...
const replaceAllTagsToValue =  //...

makeCameraUrl =  //...

let cam_id =  //...
let url = //...

If I'm right in this assumption (code that requires assumptions has a high rate of WTF/minute), please stick to either one to avoid such confusion. Again, doesn't matter which one, just stick to it after you choose (things such as casing and other issues of consistency can be alleviated by using code linters such as eslint).
Personally I don't like either way of casing my variables but my preference would go to camelCase because that way I don't have to contort my hand to type underscores all the time. As for legibility, I'd say that snake_case is better. You choose.
const cameraConfig = //...
const replaceAllTagsToValue =  //...

makeCameraUrl =  //...

let cameraId =  //...
let url = //...

On a side-note: I took the liberty to change some variables to better describe what they are:

camera_cfg... Luckily I know that cfg is short for config, but don't make me guess. Plase don't abbreviate your variable names -- we're not in the 90's anymore . Also, is it really a config? Are you using this json to allow for customisation, or are you just using it to store data?
cam_id... Camper id? Camouflage id? I understand that you mean camera, but the less I have to interpret what you mean, the better I understand what you actually mean. Deliberateness is actually the most important part of programming; functioning code is merely a byproduct.

 The meat
Now that the low hanging fruit is out of the way, let's look at the most important part (the meat) of your program:
makeCameraUrl = function (camera, type, flux, user) {
    var url = camera.url;
    var Obj = {
        "@@login@@": user.login,
        "@@passwd@@": user.passwd,
        "@@ip@@": camera.ip,
        "@@stream@@": camera.flux[type].stream,
        "@@port@@": camera.flux[type].port,
        "@@flux@@": camera.flux[type][flux]
    };
    return replaceAllTagsToValue(Obj, url)
}

If I understand correctly, you're using an object to encode information about a camera, and putting that info into the camera's url and return it. That's a great idea, but the way you did it has a few nasty side-effects.
Creating an object in every function call (Obj in this case) is expensive, and the garbage collector will make your browser hang a little everytime your single-use objects need discarding. My proposed solution is to yoink the object out of the function scope, and into the scope of the rest of the program. To keep the dynamic nature of the values of the object, I used functions:
const translator = {
  "@@login@@":  (user) => user.login,
  "@@passwd@@": (user) => user.passwd,
  "@@ip@@":     (_, camera) => camera.ip,
  "@@stream@@": (_, camera, type) => camera.flux[type].stream,
  "@@port@@":   (_, camera, type) => camera.flux[type].port,
  "@@flux@@":   (_, camera, type, flux) => camera.flux[type][flux]
};

The way these functions are set up, allows you to just call them with predefined arguments (makeCameraUrl(user, camera, type, flux)) and always get the correct value back. The underscore (_) in some of these functions is a colloquial way of saying "I'm not interested in this parameter." It's quite universal between languages.
Lastly, I took the liberty to change your code a bit to make it run so your makeCameraUrl function does what I think you meant it should do:
const makeCameraUrl = (user, camera, type, flux) =>
  Object.entries(translator)
    .reduce(
      (url, [key, value]) => url.replace(key, value(user, camera, type, flux)),
      camera.url);

We loop over all entries of the translator object, which yields an array like this:
[
  ["@@login@@",  () => /*...*/]
  ["@@passwd@@", () => /*...*/]
  ["@@ip@@",     () => /*...*/]
  ["@@stream@@", () => /*...*/]
  ["@@port@@",   () => /*...*/]
  ["@@flux@@",   () => /*...*/]
]

An entry is a two-element array with its first element being the key, and the second being its value.
We then reduce over these entries, starting with a default value of the camera's URL. I'm going to write out the lengthy version of what this reduce does:
const makeCameraUrl = (user, camera, type, flux) =>
  Object.entries(translator)
    .reduce(
      (url, [key, value]) => url.replace(key, value(user, camera, type, flux)),
      camera.url);

// Is the same as the following:

const makeCameraUrl = (user, camera, type, flux) =>
  camera.url
    .replace("@@login@@",  translator["@@login@@"](user, camera, type, flux))
    .replace("@@passwd@@", translator["@@passwd@@"](user, camera, type, flux))
    .replace("@@ip@@",     translator["@@ip@@"](user, camera, type, flux))
    .replace("@@stream@@", translator["@@stream@@"](user, camera, type, flux))
    .replace("@@port@@",   translator["@@port@@"](user, camera, type, flux))
    .replace("@@flux@@",   translator["@@flux@@"](user, camera, type, flux));

// Is the same as the following:

const makeCameraUrl = (user, camera, type, flux) =>
  camera.url
    .replace("@@login@@",  user.login)
    .replace("@@passwd@@", user.passwd)
    .replace("@@ip@@",     camera.ip)
    .replace("@@stream@@", camera.flux[type].stream)
    .replace("@@port@@",   camera.flux[type].port)
    .replace("@@flux@@",   camera.flux[type][flux]);

Quite elegant, if this was your intention.

 The end
Hopefully you've learned some things form this answer (if not, sorry for the long post ). It's a good start, but keep my tips in mind and you'll be a JS guru in no time :)


Binary search function I have written this function to perform a binary search on an array, given begin/end indices and a value to look for in the supplied array:

def binarySearch(array, beginIndex, endIndex, value):
    while (beginIndex < endIndex):
        mid = (beginIndex + endIndex) / 2
        if array[mid] < value:
            beginIndex = mid + 1
        elif array[mid] > value:
            endIndex = mid - 1
        else: #equal
            return mid

    if array[beginIndex] == value:
        return beginIndex
    else:
        return -1

Test cases / output:

print binarySearch([2,3], 0, 1, 2)
 0
print binarySearch([2,3], 0, 1, 3)
 1
print binarySearch([2,3], 0, 1, -1)
-1
print binarySearch([2,3,3,3,4,5], 0, 5, 3)
 2

So it works as intended. I'm interested in feedback mostly from a functional perspective, but I'm open to anything else that might catch your eye.

Your function doesn't have docstrings.  Docstrings are good because they give you an opportunity to tell users of your code how to use it.  I had to read the whole code to determine that the binary search you were referring to was for element extraction on a (presumably sorted) list.
You don't check for (or even document) that your function only works on sorted lists.  For example binarySearch('acb', 0, 2, 'b') gives -1 which might not be what users expect.  You could add an assert array == sorted(array) statement to guard against unsorted arrays being passed in, or at the very least mention the requirement for sortedness in the docstring.
You could easily supply default arguments for beginIndex and endIndex in the common use case that the search should start on the whole array.  The case of endIndex is a bit tricky since the default value depends on the length of the whole array.  But you could so something like:

def binarySearch(array, value, beginIndex=0, endIndex=None):
    """Docstring you need to add"""
    if endIndex is None:
        endIndex = len(array) - 1
    # rest of code

That way, users would not always have to supply arguments for beginIndex and endIndex if they didn't feel like it.  Notice how I had to re-order the parameter to achieve this effect, because arguments with default values must all be listed after arguments with no default (and you won't likely ever want a default value for value).

You could make the code forward-compatible with Python 3 by adding a from __future__ import division at the top and using // instead of /.
For consistency with most other regular Python functions, you might want to raise ValueError or something similar instead of return -1 when the search does not succeed.  I'm only slightly joking when I say that Pythonic users might interpret a -1 as meaning that the desired value was found at the end of the array!
You don't use PEP8 style recommendations.  binary_search() and begin_index would be preferred names relative to binarySearch() and beginIndex.


Managing book of Excel sheets This is a follow-on from a previous question I posted here.

I've got code here that works for what I want, but the problem is the loop takes ages to perform. I was wondering if anyone could follow this and tidy it up a bit for me.

Sub Refresh_Data()
    Application.CutCopyMode = False

    'Turns screen updating off to increase speed
    Application.ScreenUpdating = False

    'Get 'G/L Account numbers
    Sheet1 = "BW TB"

    Sheets(Sheet1).Activate
    Range("A1").Activate

    'Find last row - always named "Overall Result" in ColA
    Cells.Find(What:="Overall Result", After:=ActiveCell, LookIn:=xlFormulas, LookAt _
    :=xlWhole, SearchOrder:=xlByColumns, SearchDirection:=xlNext, MatchCase:= _
    True, SearchFormat:=False).Activate

    'This looks up to row 25 (title row), but adjusts to only copy data from row 26 down to the penultimate row (the subtotal is not required)
    lastrow = Selection.Row - 1
    colno = Selection.Column
    firstrow = Selection.End(xlUp).Row + 1

    'CopyPaste loop

    'First sheet is titled "4020"
    i = Sheets("4020").Index

    'Due to all the sheet names being numeric. This is a slight workaround.
    'It basically runs the macro starting at the 4020 sheet and ending at the last sheet with a numeric sheets.
    'i.e. pastes values for all numbered tabs.
    Do While IsNumeric(Sheets(i).Name) = True

    'clear all formulae except first formulaic row (Row5)
    Sheets(i).Activate
    Range("A6").EntireRow.Select
    Range(Selection, Selection.Offset(1000, 0)).ClearContents

    'Copy G/L Account numbers from BW TB sheet to current sheet
    Sheets(BWTB).Activate
    Range(Cells(firstrow, colno), Cells(lastrow, colno)).Copy

    Sheets(i).Activate
    Range("a5").PasteSpecial xlPasteValues

    'Copy down formulae
    Range("B5:L5").Copy
    Range("B5:L5", Range("B5:L5").Offset(lastrow - firstrow, 0)).PasteSpecial xlPasteFormulas

    ActiveSheet.Calculate

    'Paste As Values
    Range("B6:L6", Range("B6:L6").Offset(lastrow - firstrow, 0)).Copy
    Range("B6").PasteSpecial xlPasteValues

    i = i + 1
    Loop
End Sub

The scenario is about 25 'numericlly named' sheets (eg 4020) for which I need to first clear (this is for a rolling document updated periodically with a differing number of rows required, the determine the number of rows, then copy-pastespecial data from an unformatted mastersheet (BW TB).

Apologies for the mess it's in.  I'm in the process of breaking up the code into more sub functions for easier reading.

You should clean up every Select and Activate and use objects instead

You'd better use the object model of VBA.
For instance, if you only want to copy the value of a cell:

Don't do

Range("A1").Select
Selection.Copy
Range("A2").Select
Selection.Paste

Do

Range("A2").Value = Range("A1").Value

Another example:

Don't do

Cells.Find(...).Activate
lastrow = Selection.Row - 1
colno = Selection.Column
firstrow = Selection.End(xlUp).Row + 1

Do

Dim mycell as Range
Set cell = Cells.Find(...)
lastrow = mycell .Row - 1
colno = mycell .Column
firstrow = mycell .End(xlUp).Row + 1

And so on, especially on your Sheet objects.

Example to copy-paste between sheets

You only have to adapt this kind of statement to your specific case:

Worksheets("Sheet1").Range("A1").Copy
Worksheets("Sheet2").Range("A2").PasteSpecial xlPasteFormulas

Other tips

You can also have a look at the very good website of Chip Pearson

Edit

Instead of:

Sheets(i).Activate
Range("A6").EntireRow.Select
Range(Selection, Selection.Offset(1000, 0)).ClearContents

You can try:

Dim lastCol as Long
With Sheets(i)
    lastCol = .Cells(6, .Columns.Count).End(xlToLeft).Column
    .Range("A6", .Cells(1000, lastCol)).ClearContents
End With

This will find the last column where you have data (so that you don't have to clear contents of the entire row) on the 6th row and then it will clear the content of the Range between A6 and the last column and the 1000th row.

Another edit

You also have a minor issue in your declaration part.

This:

Dim mycell As Range, LastRow, ColNo, FirstRow, i As Integer

doesn't work, you have to do:

Dim mycell As Range, LastRow As Integer, ColNo As Integer, FirstRow As Integer, i As Integer


Nested Classes in C# I wondered how nested classes in C# work as I often use them when I write native C# code (like Woods.Tree.Add(new Tree).Leaf.Color) so i tried it by my self with a Navigation example. There is a Position integer which declares a point on a line. To move left the position will be subtracted by 1, to move right the position will added by 1.

I think its quite pedantic and much code to write for such a 'simple' task. Is this the right way or is there an easier, smaller way to archive this?

using System;

namespace ConsoleApp2
{
    class Program
    {
        static void Main(string[] args)
        {
            NestedTest nt = new NestedTest();//create test
            nt.Navigate.Left();
            Console.WriteLine(nt.Position);
            nt.Navigate.Left();
            nt.Navigate.Left();
            Console.WriteLine(nt.Position);
            nt.Navigate.Rigth();
            Console.WriteLine(nt.Position);
            Console.WriteLine("Press any key to exit...");
            Console.ReadKey();
        }
    }
    class NestedTest
    {
        public int Position { get; private set; }
        public NavigateClass Navigate;//create the NavigateClass in NestedTest

        //Nested NavigateClass
        public class NavigateClass
        {
            private NestedTest _o;//the placeholder
            public NavigateClass(NestedTest obj)
            {
                _o = obj;//Set the NestedTest class so we know which Position we have to navigate trough.
            }
            public void Left()
            {
                Console.Write("<-");
                _o.Position--;//Move Position to the left (-1)
            }

            public void Rigth()
            {
                Console.Write("->");
                _o.Position++;//Move Position to the right (+1)
            }
        }

        public NestedTest(int num = 10)
        {
            this.Position = num;//Set start position
            Navigate = new NavigateClass(this);//Set the _o in Navigate so it knows which Position it has to navigate trough
        }
    }

}

Is this the right way or is there an easier, smaller way to archive this?

It depends on how you define the right way. Is your goal short and small code or is your goal testability and maintainability? These two goals are mutualy exclusive.

If you want to have it short then of course there is a way. You don't need the nested class at all.

class NestedTest
{
    public int Position { get; private set; }

    public void Left()
    {
        Console.Write("<-");
        Position--;
    }
}

If you want to have maintainable code then you need to write more code and correctly use dependency injection and possibly use a few interfaces as abstractions. Nested public classes are a bad style anyway so you should avoid them.

This is how such an implementation could look like. The NestedTest gets an interface that allows accesing its Position. Its constructor also requries a lambda that lets you specify the navigator for this instance.

public interface IPositionable
{
    int Position { get; set; }
}

class NestedTest : IPositionable
{
    public int Position { get; set; }

    public INavigator Navigator { get; }

    public NestedTest(int num, Func<IPositionable, INavigator> createNavigator)
    {
        Position = num;
        Navigator = createNavigator(this);
    }
}

Where the Navigator is now also abstracted by an interface and can be faked for testing or exachnged for another navigator later:

public interface INavigator
{
    void Left();
}

public class Navigator : INavigator
{
    private IPositionable _positionable;

    public Navigator(IPositionable positionable)
    {
        _positionable = positionable;
    }

    public void Left()
    {
        _positionable.Position--;
    }
}

It's up to you to decide. Shortest code possible or testable and maintainable?


Finding Matches and writing results to file C# I have a folder that contains around 1000 text files, their sizes varies from few KBs up to 500 MB (Total size is around 60 GB).

I have a text file that contains a 166k lines, each line contains a set of numbers (length up to 11), what I'm doing here is to go through each file in that folder and try to find a match between the 166k and those files, then store the line from the file and print that out to a file.

My approach is working but there are several issues:

It is not fast  (taking a lot of time to go through the files)
Memory consumption, I believe that is because I'm storing everything into a list of strings and then print all of it at once

Please take a look at my code below, and let me know if I can enhance it and improve it in any possible way.

    static void Main(string[] args)
    {
        char splitter = '\u0001';

        //166k file
        string path = "Z:\\subid.txt";

        //Destination file
        string RxPath = "Z:\\matched.txt";

        List<string> subid = File.ReadAllLines(path).ToList();
        List<string> RxClaims = new List<string>();

        string[] lineObject;
        int count = 0;

        //folder location (contains 1000 text files)
        string folderPath = "Z:\\rawfiles";

        foreach (string file in Directory.EnumerateFiles(folderPath))
        {
            Console.WriteLine("Processing " + file);
            foreach (string line in File.ReadLines(file))
            {
                lineObject = line.Split(splitter);

                //Check if that value is equal to any of the numbers in the 166k, if so store in the list to print out later
                if(subid.Contains(lineObject[14]))
                {
                    count++;
                    RxClaims.Add(line);
                }

            }
        }

        File.WriteAllLines(RxPath, RxClaims);
        Console.WriteLine("Done, Number of Claims" + count);

        Console.ReadLine();
    }

Some things to speed this process up

subid is only used if it contains a certain string. Use a hashset or dictionary where each lookup is O(1) instead of O(n) ; which is in this case O(166K) in worst case
EDIT: StreamReader and File.ReadLines actually behave the same
Start using using for handling input/output; it makes sure that object are garbage collected one it's out of scope. Using declares a scope for these.
Personally I put all configuration on top
You could also use StreamWriter to write the file, which is just a handle (same principle as the StreamReader, but then for writing)

With only minimal adjustments, this should be significantly faster.

Improved code

public static void Main(string[] args)
{
    // file paths
    string path = "Z:\\subid.txt";      //166k file
    string RxPath = "Z:\\matched.txt";  //Destination file
    string folderPath = "Z:\\rawfiles"; //folder location (contains 1000 text files)

    char splitter = '\u0001';

    // subid is ONLY used to check if it contains something. Make it a hashset
    HashSet<string> subid = new HashSet<string>(File.ReadAllLines(path));
    List<string> RxClaims = new List<string>();

    string[] lineObject;
    int count = 0;

    foreach (string file in Directory.EnumerateFiles(folderPath))
    {
        Console.WriteLine("Processing " + file);

        // use a streamreader to go through files!
        using (StreamReader reader = new StreamReader(file))
        {
            string line = reader.ReadLine();
            lineObject = line.Split(splitter);

            //Check if that value is equal to any of the numbers in the 166k, if so store in the list to print out later
            if (subid.Contains(lineObject[14]))
            {
                count++;
                RxClaims.Add(line);
            }
        }
    }

    File.WriteAllLines(RxPath, RxClaims);
    Console.WriteLine("Done, Number of Claims" + count);

    Console.ReadLine();
}


Cracking the Coding Interview - 4.1 Graph Traversal I'm working through the classic Cracking the Coding Interview book. I'm trying to solve the following graph problem:

  4.1) Route Between Nodes: Given a directed graph, design an algorithm to find out whether there is a route between two nodes.

I've written a solution in JavaScript and found it was a lot less complicated than the book's solution. Am I missing something? Why does the solution use a Visiting state? Can't we just use Visited instead?

function RouteBetweenNodes( node1, node2 ) {

    const q = new Queue()
    q.add( node1 )

    while ( !q.isEmpty() ) {
        const node = q.remove()
        if ( node === node2 ) {
            return true
        }
        node.visited = true
        node.children.forEach( child => !child.visited && q.add( child ) )
    }

    return false

}

Here is an implementation of a Queue for completeness:

class QueueNode {

    constructor( data ) {
        this.data = data
        this.next = null
    }

}

class Queue {

    constructor() {
        this.first = null
        this.last = null
    }

    add( item ) {
        const newNode = new QueueNode( item )
        if ( this.last ) {
            this.last.next = newNode
        } else {
            this.first = newNode
        }
        this.last = newNode
    }

    remove() {
        if ( this.first ) {
            const data = this.first.data
            this.first = this.first.next
            if ( this.first == null ) {
                this.last == null
            }
            return data
        }
        throw Error( 'empty queue' )
    }

    peek() {
        if ( this.first ) {
            return this.first.data
        }
        throw Error( 'empty queue' )
    }

    isEmpty() {
        return this.first === null
    }

}

Here is the book's solution.

Your worst case performance (every node is connected to each other node but node2) is \$\mathcal{O}(n^2)\$, up from the "normal" \$\mathcal{O}(n)\$, because nodes can be added to the queue multiple times. This should also increase the average runtime. This gets even worse if multiple edges between any 2 nodes are allowed.

Note that adding another check q.contains(node) would still result in a runtime of \$\mathcal{O}(n^2)\$ as those checks add a factor of \$\mathcal{O}(n)\$. The Visiting state prevents this.
visited is never reset to false (in the presented code). This will lead to errors in subsequent calls.
The definition of Queue is missing.


strcat implementation I've written my own implementation of strcat in C.

char* my_strcat(char *str1, char *str2)
{
   int len_str1 = 0, len_str2 = 0, len_str3;

   for (int i = 0; str1[i] !='\0'; i++)
   {
      len_str1++;
   }

   for (int i = 0; str2[i] != '\0'; i++)
   {
      len_str2++;
   }

   len_str3 = len_str1 + len_str2;
   char* str3 = malloc(len_str3*sizeof(char));

   for (int i = 0; i < len_str1; i++)
   {
      *(str3+i) = *(str1+i);
   }

   for (int i = 0; i < len_str2; i++)
   {
      *(str3+i+len_str1) = *(str2+i);
   }

   return str3;
}

I'm looking for anything that could be improved: readability, efficiency, code cleanup, bugs, ...

The most obvious is that although this matches the prototype of the standard strcat function, it works very differently. The standard strcat function does not allocate memory.
You do not account for the terminating NUL character when allocating memory for the new string.
Following from (2), you do not actually NUL terminate the new string.
The value of sizeof(char) is defined to be 1, so you don't really need to multiply by that value.
Your use of spaces is slightly inconsistent, in the first loop there is no space after != but there is in the second loop.
Although the use of *(str3+i) is valid and not wrong, it is more readable and common to use str3[i] instead. MISRA compliant functions have to use the latter.
You could move the declaration of int len_str3 down to where its value is computed, so you would have int len_str3 = len_str1 + len_str2;. That way you don't have what might appear to be an uninitialised variable at the top.


How to make static methods testable? I am creating an application which will be testable(unit + integration). In this application I have a FileHelper static class,

public static class FileHelper
{
    public static void ExtractZipFile(Stream zipStream, string location)
    {
        ..................................
    }
    public static void CreatePageFolderIfNotExist(string directory)
    {
        .................................................
    }

    .......................................................
    .......................................................
}

But due to static I think it is not testable. How to make this class testable?

If you want to write tests for this helper class only - they would be integration tests since they touch the file system.

In order to make the code unit testable you need to make it independent from hardware and physical interaction with network or disk.

If you want to unit test the business logic that uses this FileHelper, then you should introduce Inversion of Control to separate business logic from file activities. You'll have to create an interface that describes all operations you can do with files, transform this static helper class into class implementing that interface, and reference file operations from business logic via interface only. Example of file helper:

public interface IFileOperations
{
    void ExtractZipFile(Stream zipStream, string location);
    void CreatePageFolderIfNotExist(string directory);
}

public class FileOperations : IFileOperations
{
    public void ExtractZipFile(Stream zipStream, string location)
    {
        ..................................
    }

    public void CreatePageFolderIfNotExist(string directory)
    {
        .................................................
    }
}

Then, in order to properly cover you code with tests, you would need to write unit tests for business logic and add integration tests for the implementation of FileOperations.


Anagrams for a given input This question is the first draft, my new revised code is here:
Anagrams for a given input 2.0

Here is my code that I wrote, which basically lists the anagrams for a given input.

I did this by reading each line in my dictionary file and comparing it to the anagram.  Only if it matched did I add it to the list, instead of adding all the words to the list and then sorting through them.

I compared the words by first checking the length of both words, and then if they matched I put put each word into a list and then sorted the list, then I compared the two lists using the equals() function. And iterated my counter.

The function of my getOutPut() was just to neaten up the printout.

What I would like are some critiques on my work and what I should/could have done differently, especially pertaining to semantic efficiency, code correctness, common practices that I may be unaware of, and code efficiency.

This is only my second project, so I want to learn from it to become a better programmer.  That's why I'm asking.

The methods are in the order they are called in:

(Note that I split the code up to make it easier to see, all the methods are inside the Anagramatic Class)

package anagramatic;

/**
 * IDE : NETBEANS
 * Additional Libraries : Guava
 * @author KyleMHB
 */
import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Scanner;
import javax.swing.JOptionPane;

public class Anagramatic {
static int k;

-

public static void main(String[] args) throws FileNotFoundException{
    String anagram;
    anagram=input();
    ArrayList<String> words=readFile(anagram);
    String output= getOutPut(words);
    JOptionPane.showMessageDialog(null,
            "The anagram "+anagram+" has "+k+" matches, they are:\n\n"+output);
}//PVSM

-

private static String input() {
    String input;
    input = JOptionPane.showInputDialog(null,
     "Enter the Word or words you would like to be processed");
    return input;
}//input

-

private static ArrayList readFile(String anag) throws FileNotFoundException{
    k=0;
    ArrayList<String> list;
    ArrayList<Character> a;
    ArrayList<Character> b;
    try (Scanner s = new Scanner(new File("words.txt"))){
        list = new ArrayList<>();
        while (s.hasNext()){
            String word= s.next();
            if (word.length()==anag.length()){
                a = new ArrayList<>();
                b = new ArrayList<>();
                for(int i=0;i!=word.length();i++){
                    a.add(anag.charAt(i));
                    b.add(word.charAt(i));
                }//forloop to make two lists of the words
                Collections.sort(a);
                Collections.sort(b);
                if(a.equals(b)){
                    list.add(word);
                    k++;
                }//comparing the two lists
             }//if length
          }//while
       }//try
    return list;
}//readfile

-

private static String getOutPut(ArrayList<String> words) {
    String wordz="[";
    int x=0;
    int y=0;
    for(int i=0; i!=words.size()-1;i++){
        if(x!=7){
            wordz+=words.get(i)+", ";
            x++;
        }else{wordz+="\n";x=0;}
    }//for
    wordz+=words.get(words.size()-1)+"]";
return wordz;
}//getOutPut
}//Anagramatic

I'll focus on the heart of your solution first, which is your readFile() method.  Here's how I would write it:

public static List<String> anagramsInFile(String word, File f)
        throws FileNotFoundException {
    char[] canonical = canonicalize(original);
    ArrayList<String> anagrams = new ArrayList<String>();
    try (Scanner s = new Scanner(f)) {
        while (s.hasNext()) {
            String candidate = s.next();
            if ( (candidate.length() == word.length()) &&
                 (Arrays.equals(canonical, canonicalize(candidate))) ) {
                anagrams.add(candidate);
            }
        }
    }
    return anagrams;
}

private static char[] canonicalize(String original) {
    char[] array = original.toCharArray();
    Arrays.sort(array);
    return array;
}

Points to note:

Naming is important.  handleFile() is vague; anagramsInFile() is meaningful.  Also, I find anag to be confusing parameter name.
Hard-coding the filename inside the method is a bad idea.
Unless the caller has a reason to require an ArrayList, your method signature should just commit to returning some kind of List.  Better yet, consider returning a Set.
You only have to canonicalize the original string once, not once per word in the file.
Canonicalization deserves a helper method, since you have to do it to both the original string and to the contents of the file.
Canonicalizing to a char[] avoids working character by character, since you can take advantage of String.toCharArray().  It's probably more efficient as well.
There's no point in maintaining k: it's just the size() of the returned list.  Maintaining k as a class variable is particularly egregious, as there's no reason for that count to be part of the state of the class.
If your blocks are so lengthy and deeply nested that you need comments on the closing braces, consider that a Bad Code Smell and address that issue instead.  (By the way, your braces were misaligned, which causes confusion that even your close-brace comments can't compensate for.)

Next, let's look at getOutPut().  Here's how I would write it:

private static String formatOutput(List<String> words) {
    StringBuilder out = new StringBuilder('[');
    int wordsPrinted = 0;
    Iterator<String> w = words.iterator();
    while (w.hasNext()) {
        out.append(w.next());
        if (w.hasNext()) {
           out.append((++wordsPrinted % 8 == 0) ? "\n" : ", ");
        }
    }
    return out.append(']').toString();
}

Points to note:

When concatenating so many strings, you really need to use a StringBuilder.  Repeated string + string would create a temporary string each time.
Your output is buggy: you drop every eighth word, replacing it with a newline.
Be careful with edge cases: your code crashes when words is empty.
Try to structure your code so it's obvious that certain properties hold true.  For example, in my solution, you can see that every word gets printed (because if .hasNext() is true, the next word will get appended).  You can also see that every word except the last word is followed by a delimiter (either newline or comma).
Avoid cryptically named variables like x and y.  In fact, you don't even use  y.
Naming your result wordz when there's another variable named words makes the code a pain to read.

Finally:

public static void main(String[] args) throws FileNotFoundException{
    String word = input("Enter the word or words you would like to be processed");
    List<String> anagrams = anagramsInFile(word, new File("words.txt"));
    display(String.format("The word %s has %d matches.  They are:\n\n%s",
                          word, anagrams.size(), formatOutput(anagrams)));
}

private static String input(String prompt) {
    return JOptionPane.showInputDialog(null, prompt);
}

private static void display(String message) {
    JOptionPane.showMessageDialog(null, message);
}

Observations:

I've decomposed the work differently.  There's input() and display(), which are analogous to each other, and encapsulate the Swing UI.  (If you ever want to convert the program to work in the text console, you just modify those functions.)
More importantly, you can now see at a glance what the whole program does just by looking at main().  Note that all string constants are there as well: the prompt, the filename, and the output.
Don't declare any more variables than you need to.  If possible, when you declare variables, assign them in the same statement.
Use String.format().


Slow delete query on table with composite index after profiling my application, it turns out that a single method is taking 3 minutes to run, which is about a third of the total runtime.

The method deletes approx. 400.000 rows from each table (PROCESSED_CVA and PROCESSED_DVA).

The code executing the queries :

public final static String DELETE_CVA = "delete from PROCESSED_CVA where RUN_ID = ?";
public final static String DELETE_DVA = "delete from PROCESSED_DVA where RUN_ID = ?";
public void purge(Run run) throws HibernateException {
    Session session = null;
    if (session == null) {
        session = sessionFactory.openSession();
    }
    Transaction t = session.beginTransaction();
    try {
        SQLQuery query = session.createSQLQuery(DELETE_CVA);
        query.setLong(0, run.getRunId());
        query.executeUpdate();
        query = session.createSQLQuery(DELETE_DVA);
        query.setLong(0, run.getRunId());
        query.executeUpdate();
        t.commit();
    } catch (HibernateException he) {
        logger.error("Failed to purge processed cva and dva for run: " + run.getRunId(), he);
        t.rollback();
        throw he;
    }
}

Both tables have the same structure.

CREATE TABLE "PROCESSED_CVA" (
"DEAL_ID" VARCHAR2(23 BYTE),
"NTT_ID" VARCHAR2(10 BYTE),
"CVA" FLOAT(126),
"RUN_ID" NUMBER(10,0))  ;
ALTER TABLE "PROCESSED_CVA" ADD CONSTRAINT "PK_CVA" PRIMARY KEY ("DEAL_ID", "RUN_ID")

There is an index on the primary key.

The execution plan :

OPERATION                       OBJECT_NAME     OPTIONS         COST
DELETE STATEMENT                                                100582
    |_ DELETE                   PROCESSED_CVA
        |_ INDEX                PK_CVA          SKIP SCAN       100582
            |_ Access Predicates
                |_ RUN_ID=100
            |_ Filter Predicates
                |_ RUN_ID=100

Can I speed this up ?

UPDATE
DBMS : Oracle

Oracle uses the following strategy when deleting data. It:

identifies the rows that need to be deleted (it does use your PRIMARY KEY index to check the RUN_ID value, but because the RUN_ID is not the first column in the index it needs to 'skip' values in the index).
it deletes the record in the online version of the data
it writes a physical record to the transaction / redo log to record the values that were deleted

Oracle works on a per-block bases for it's redo log:

  A redo record, also called a redo entry, is made up of a group of change vectors, each of which is a description of a change made to a single block in the database

Each time you change a record, the block it is stored in is changed, and the difference is recorded in the redo log. The number of blocks you change is a key factor in determining the amount of redo-log work that you do. The number of blocks is closely related to the amount of data you are changing, and the way that the data is distributed.

If you are deleting a bunch of records that are all stored really close to each other, then the chances are that the number of blocks that are affected will be small. If the records are scattered on many blocks, then the number of blocks affected is high.

Based on the key you have specified for your data "DEAL_ID", "RUN_ID" it appears to me that your data for a specific RUN_ID will be scattered all over the database.

This means that, for each time you delete the data, you are actually inserting 400,000 redo-log entries, modifying 400,000 blocks of storage (let's say 8K each, so that's 3GB of IO), and generally processing the system quite hard.

So, apart from the basic problem of deleting 400K records, and inserting 400K redo-entries, and writing all that data to disk, what else could it be?

Locks will likely need to be escalated. Oracle will start by trying to lock the records one at a time, but will quickly find that the lock-management requires a bigger lock strategy, so it make replace the row-locks with block-locks, and then finally escalate the block locks to a full table lock. In itself, this is not a significant performance problem, but what is a problem is if anyone else is running anything against the table.... the lock escalation will have to wait until all other locks on the table are serviced. Only then will it gain exclusive access.

Ways to improve the performance would be:

monitor the database. Confirm that IO is a real problem
monitor the lock strategies... are there significant lock-wait situations.
reduce the logging requirments...  physically order the data in the same order as the RUN_ID. you can 'cluster' the data, or, in Oracle terms, you can have "index organized table". Much fewer blocks will change with this.
improve the log-device performance - put your log files on an SSD?


Database abstraction in Java I am working on a project in Java which includes a lot of database related tasks. For instance, I would have to insert, update, retrieve data so many times within the application. To use code reusability, I've just written this abstraction for my database class. I'm planing to call its methods where ever its needed to perform database related tasks within the code. Please review my code. Are there any bad practices?

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;

import com.teamincredibles.settings.Settings;

public class Database {

    private Connection connection;

    public void connect() {
        try {
            Class.forName(Settings.getDatabaseDriver());
            connection = DriverManager.getConnection(Settings.getJdbcUrl(), Settings.getDBUserName(), Settings.getDBPassword());
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

    public boolean prepareStatement(String query, String choice1) {
        boolean flag = false;
        try {
            PreparedStatement ps = connection.prepareStatement(query);
            ps.setString(1, choice1);
            ResultSet rs = ps.executeQuery();
            flag = rs.next();
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return flag;
    }

    public boolean prepareStatement(String query, String choice1, String choice2) {
        boolean flag = false;
        try {
            PreparedStatement ps = connection.prepareStatement(query);
            ps.setString(1, choice1);
            ps.setString(2, choice2);
            ResultSet rs = ps.executeQuery();
            flag = rs.next();
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return flag;
    }

    public void executeUpdate(String query) {
        try {
            Statement statement = connection.createStatement();
            statement.executeUpdate(query);
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

    public boolean execute(String query) {
        boolean result = false;
        try {
            Statement statement = connection.createStatement();
            result = statement.execute(query);
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return result;
    }

    public ResultSet executeQuery(String query) {
        ResultSet result = null;
        try {
            Statement statement = connection.createStatement();
            result = statement.executeQuery(query);
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return result;
    }

    public void disconnect() {
        try {
            if (connection != null) {
                connection.close();
            }
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Code Smells
Remember the DRY and KISS principles of programming.
Here are some other resources with good info on general programming principles.

3 Key Software Principles You Must Understand
The Principles of Good Programming

As a maintainer or reviewer of anyone's code (our own or someone elses), when examining code we look for code smells. These give general indications of programming practices which are less than ideal. When we find code smells, it's a good clue that we need to look more closely and consider why the code smells.
There are a few obvious code smells, mostly pointed out by others already:

Ambiguous Naming Convention
Re-purposing Existing Names
Repetitive Code Patterns
Improper Use of Exceptions
Failure to Release Resources
Lack of Comments

Naming Conventions
Objects and other Variables are nouns. Methods and other functions are verbs. Name them accordingly. Do not use acronyms or abbreviations, but do not use sentences for your names either. There is no need for joining words such as 'The' or 'With'. Most importantly, one should also always strive to maintain consistent naming convention, whatever style you choose. This is more in reference to @Shree's use of executeWithPreparedStatement.
executePreparedStatement is better. It conforms to the naming convention used for the other methods in the library and avoids the use of an unnecessary preposition that doesn't add any meaning to the identifier.
Same for DBUtil. These abbreviations are common enough that there is little risk of confusion but it should still be avoided. DatabaseUtility is best. We can only assume that DB means Database, we can't be sure until we inspect source code or documentation. Forcing the reader of the code to have to guess, or research should be avoided whenever possible.
Re-using names that you are deriving from is OK when you are recreating a function that does the same thing in a different way. It is not OK when you have extended the functionality of the code you are deriving from. Pick a new name that better describes why your method is different than the original.
Repetitive Code Patterns
Next to KISS, DRY should be the most important principle of programming. DRY is why we do programming. To automate things. Failing the DRY principle means you are doing more work than you need to, introducing more potential for error and making it more difficult to maintain.
When you see repetitive code patterns, such as blanket exception handling or methods that look almost identical, consider why is it repetitive and how can you reduce the duplication of those pieces of code. Often it means reconsidering the method of implementation, not just splitting things up more.
Exception Handling
Errors and exceptions should always be handled gracefully.
Never trap exceptions and then not handle them. Printing a stack trace is not handling them. You are merely acknowledging that a problem can happen at that point but you don't care. Why add extra code if you don't care? The only thing accomplished is that the problem is guaranteed to not be fixable from outside of your function, since no caller can know that anything went wrong.
Exceptions that are unhandled should be allowed to flow up the call stack until they are handled, or until the main function is reached and then an unhandled exception error occurs.
You are better off to simply not catch the exceptions at all and just let them get passed on until you get an unhandled exception error.
You can trap them do something with them and then rethrow the exceptions if they are not actually being handled.
Resource Leaks
Make sure that you are not overriding valid handles when you open new connections.
The connect() method needs to make sure that the connection object is not already valid.
As well, in the disconnect() method you should reassign connection=null after it has been closed so that the invalid handle can't accidentally be used.
Make sure to close all open handles when you are done with them. This includes things like PreparedStatement, Statement and ResultSet objects, not just connections. These objects go into object pools just like the connection pool, which can build up and eat RAM if you forget to close things. (TIP: they all derive from the AutoCloseable/Closeable interface)
Lack of Comments
This seems so trivial but it is not. This is the biggest code smell. While we should always strive to create self-documented code, we still need comments to summarize what it is the code is doing and so that formal documentation can be generated with the least effort, written by the people who actually wrote the code. Comments come first, before code. Always. This is one most programmers most often neglect. Me too.
I should mention that I'm mostly speaking of inline documentation, not trying to teach the reader how every bit of code works.
The converse code smell to a lack of comments is too much comments. If you need comments to describe how your code works and not just what it does, then your code is probably too complex.


Input data from file, calculating average by a function I want to thank you guys firstly for your previous comments and feedback. Really appreciate it. I am learning C with school at the moment, but I plan to be "fluent" in C at the end of august of this year.

For school I have to write the following program:

Open up a file. The file is a *.txt one. It consists of 32 numbers. The first number corresponds to the number of tests distributed under the students. The second number is saying how many have returned the test. The other 30 numbers could be seperated into a 5 by 6 table. There were 6 question with each 5 possible answers. the numbers represent how many students have chosen the answer. The file is posted at the end of this post.

Now I have to open this file and making a display like this:

number of passed tests: 20
Number of returned tests: 19
Response is 95%.
                  A      B      C      D    E    grade     passed
question 1   2      2      2      2    2     2.4         yes
question 2   2      2      2      2    2     2.4         yes
question 3   2      2      2      2    2     2.4         yes
question 4   2      2      2      2    2     2.4         yes
question 5   2      2      2      2    2     2.4         yes
question 6   2      2      2      2    2     2.4         yes

Also note that the "comment" has three options; fail if grade<3.0 , warning if grade<3.5 and pass if higher than 3.5.
The code I Have written is like this :

#include <stdio.h>
#define RIJ 6
#define KOLOM 5

int invoer[RIJ][KOLOM] , rij, kolom;
float ingeleverd ;

float functie(int* i ) ;
int main(void)
{

float uitgereikt=0  , percentage ;
char bestandpad[100]= "" ;

FILE *inputbestand;
printf("Dit programma verwerkt de data uit verschillende enquetes.\n");
printf("\n");
printf("Voer de locatie van het bestand in.\n");
scanf("%s", &bestandpad);
inputbestand=fopen(bestandpad, "r" );

    if (inputbestand == NULL)
        { printf("Bestand niet gevonden.");
            return 0;
        }

fscanf(inputbestand, "%f", &uitgereikt);
fscanf(inputbestand, "%f", &ingeleverd);
percentage = (ingeleverd/uitgereikt) *100 ;
//------- overige data inlezen in een array van 5 bij 6.

    for (rij=0; rij<RIJ; rij++)
       {
           for (kolom=0; kolom<KOLOM ; kolom++)
           {
                   fscanf(inputbestand, "%d", &invoer[rij][kolom]);
           }
       }
fclose(inputbestand);

printf("Aantal uitgereikte evaluaties: %0.f\n" , uitgereikt);
printf("Aantal ingeleverde evaluaties: %0.f\n" , ingeleverd);
printf("Response is: %.1f %% \n\n" , percentage);
printf("   \t  A\t B \t C \t D \t E \t cijfer \t opmerking\n\n");

            for (rij=0; rij<RIJ; rij++)
            {
                    printf("vraag %d   " ,rij+1);

                for (kolom=0; kolom<KOLOM ; kolom++)
                {
                    printf("%d       ", invoer[rij][kolom]);

                    if (kolom==KOLOM-1) {
                                            printf("%c \n", functie(invoer[rij]));
                                        }

                }
            }
}
float functie(int* i )
{
    int a ;
    float gem, totaal;

    for (a=0; a<5 ; a++)
    {totaal= totaal+ ( i[a] * (a+1)  ) ;}

    gem =  totaal/ingeleverd ;

    if      (gem<3.0)      {printf("%.1f \t \t Voldoet niet\n", gem);}
    else if (gem<3.5)      {printf("%.1f \t \t opmerking\n", gem);}
    else if (gem>3.5)      {printf("%.1f \t \t Voldoet\n", gem);}
  //  return  gem ;
}

It is working completely. I have one minor bug though. the results are not aligned properly. What do you guys think ? Except the bad indenting... I am also very sorry it is in Dutch. But I think you guys will understand alright..

the inputfile is like this:

    20
    19
    1
    2
    4
    5
    7
    0
    1
    8
    7
    3
    8
    5
    4
    2
    0
    2
    2
    4
    11
    0
    2
    3
    3
    5
    6
    0
    0
    4
    12
    3

I would think about breaking this up into more functions that do specific things. For example, I would break out the reading of the header information (the number of tests passed and the number returned) into one function, and the reading of the actual test data into another function:

void readHeader (FILE* inputbestand, float* uitgereikt, float* ingeleverd)
{
    fscanf(inputbestand, "%f", uitgereikt);
    fscanf(inputbestand, "%f", ingeleverd);
}

This brings up a question I have for you - why is ingeleverd a global variable? In fact, why do you have any global variables? They all look like they belong inside the main() function to me.

In any event, reading the table of data would then look something like this:

void readTable (FILE* inputbestand, int invoer[RIJ][KOLOM])
{
    //------- overige data inlezen in een array van 5 bij 6.
    int rij, kolom;
    for (rij = 0; rij < RIJ; rij++)
    {
        for (kolom = 0; kolom < KOLOM; kolom++)
        {
            fscanf(inputbestand, "%d", &invoer[rij][kolom]);
        }
    }
}

For what it's worth, I like to put spaces around operators (like comparison operators - <, >, etc.), as I find it much easier to read. That's just a style decision and might not suit you, but that's what I prefer.

I would also move the output into various functions. The introductory text could go into a function like this:

#define RESULT_OK 1    // <- These can go at the top of the file
#define RESULT_ERROR 0 // <- with the others
int printIntro (FILE** inputbestand, char bestandpad[100])
{
    int result = RESULT_OK;
    printf("Dit programma verwerkt de data uit verschillende enquetes.\n");
    printf("\n");
    printf("Voer de locatie van het bestand in.\n");
    scanf("%s", bestandpad);
    *inputbestand = fopen(bestandpad, "r" );

    if (*inputbestand == NULL)
    {
        printf("Bestand niet gevonden.");
        result = RESULT_ERROR;
    }
    else
    {
        result = RESULT_OK;
    }

    return result;
}

Although, perhaps a better function name would be getFileName() (or would it be getBestandpad()? I don't know Dutch for "get").

Since you were originally returning 0 from main() when the file could not be read, you need to return any errors from the above function and use that in main() to decide what to return. Which reminds me - you aren't returning a value on success in your main() function. So let's look at that next. After breaking out the various functions, it will look something like this:

int main (void)
{
    FILE* inputbestand;
    int invoer[RIJ][KOLOM];
    char bestandpad[100];

    int result = printIntro(&inputbestand, bestandpad);
    if (result == RESULT_OK)
    {
        float uitgereikt;
        float ingeleverd;
        readHeader(inputbestand, &uitgereikt, &ingeleverd);
        readTable(inputbestand, invoer);
        fclose(inputbestand);

        printStatistics(uitgereikt, ingeleverd);
        printResults(invoer, ingeleverd);
    }

    return result;
}

The function printStatistics() should print out the information about how many tests were passed and how many were returned:

void printStatistics (const float uitgereikt, const float ingeleverd)
{
    printf("Aantal uitgereikte evaluaties: %0.f\n" , uitgereikt);
    printf("Aantal ingeleverde evaluaties: %0.f\n" , ingeleverd);

    float percentage = (ingeleverd/uitgereikt) *100 ;
    printf("Response is: %.1f %% \n\n" , percentage);
}

Likewise, printResults() should print out the resulting table:

void printResults(int invoer[RIJ][KOLOM], const float ingeleverd)
{
    printf("   \t  A\t B \t C \t D \t E \t cijfer \t opmerking\n\n");
    int rij, kolom;
    for (rij=0; rij<RIJ; rij++)
    {
        printf("vraag %d   " ,rij+1);

        for (kolom=0; kolom<KOLOM ; kolom++)
        {
            printf("%d       ", invoer[rij][kolom]);
            if (kolom==KOLOM-1)
            {
                printf("%c \n", functie(invoer[rij], ingeleverd));
            }
        }
    }
}

It's not clear to me what the functie() function is doing. It looks like it's supposed to calculate an average, but it doesn't look to me like it's actually doing that. Also, it returns a float, but it's being used as a character in the printf() statement where it's called. That's almost certainly incorrect. And you've listed 3.0 and 3.5 as the breakpoints between fail, warning, and pass, but in the example, all the grades are 2.4, but marked as "pass". So I'd double-check to make sure you understand the requirement there.

In addition to the above, you should know that there are various ways that reading from the file could fail. You could have truncated data, or there might be the wrong kind of data in the file. I'm not sure if your teacher wants you to worry about that yet, or not. If you want to, you can check the return value of fscanf() and make sure that it read the argument you expected.


Excel VBA Filtering: AODB vs Autofilter vs SQLite I have an Excel sheet with data in about 100,000 rows, 16 columns. I have to extract data (one value from the 1 row I find) from this table a few thousand times. I have been using Autofilter to extract this value, but have also tried AODB/SQL statements to see if I can improve the speed of the queries.

The data is sorted by column 1. When I use Autofilter, it takes on average 0.3 seconds to get my value, no matter the location of the value. AODB takes between 0.1 to 0.6 seconds to find the value, depending on where the value is in the table (if the value is near the top of the table, it takes 0.1 seconds, while if the data is near the end of the table it can take up to 0.6 seconds). I had expected the AODB to be much faster than Autofilter, but it seems that on average they are about the same.

When I import this data into SQLite and use a tool such as SQLiteStudio, the query returns the value in less than 0.01s!

Can I change anything in my code to to speed up the Queries in VBA?

Codes are below. AODB method:

Sub AODB_method()
'Freeze Screen and other options to run code faster
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Application.DisplayStatusBar = False
Application.EnableEvents = False

 Dim cn As ADODB.Connection
 Dim rs As ADODB.Recordset
 Dim vv As Double
 Dim Time2 As Double

Time2 = Timer

Set cn = New ADODB.Connection
Set rs = New ADODB.Recordset

With ThisWorkbook
    If Application.Version < 12 Then
        cn.ConnectionString = _
          "Provider=Microsoft.Jet.OLEDB.4.0;" & _
          "Data Source=" & .FullName & ";" & _
          "Extended Properties=Excel 8.0"
    Else
        cn.ConnectionString = _
          "Provider=Microsoft.ACE.OLEDB.12.0;" & _
          "Data Source=" & .FullName & ";" & _
          "Extended Properties=Excel 8.0"

     End If

     cn.Open

    Set rs = cn.Execute("SELECT [Cl] FROM [Table1$] WHERE [Wind]=150 AND [Weight]=200000 AND [Altitude] = 20000 AND [ISA] = 0")

    vv = rs.Fields(0).Value

    rs.Close
    cn.Close
End With

'Turn Options on again
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
Application.DisplayStatusBar = True
Application.EnableEvents = True

Debug.Print "Connection Time: " & Timer - Time2
End Sub

Autofilter method:

Sub Autofilter_method()

'Freeze Screen and other options to run code faster
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Application.DisplayStatusBar = False
Application.EnableEvents = False
Dim vv As Double
Dim Time2 As Double

Time2 = Timer

With ThisWorkbook.Worksheets("Table1")
        .AutoFilterMode = False
        With .Range("A1:H1")
            .AutoFilter
            .AutoFilter Field:=4, Criteria1:=0
            .AutoFilter Field:=2, Criteria1:=200000
            .AutoFilter Field:=3, Criteria1:=20000
            .AutoFilter Field:=1, Criteria1:=-150

        End With

    End With

vv = Range("H" & finallastrow("Table1", "H")).Value

'Turn Options on again
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
Application.DisplayStatusBar = True
Application.EnableEvents = True

Debug.Print "Connection Time: "; Timer - Time2
End Sub

Some Notes:

I am using Excel 2013. The Jet OLEDB provider seemed faster but I can't get the value to be saved in variable vv (I get an error).
When Wind is -150 (Column1, -150 is near the top of the table), the AODB query runs in 0.1s, while a wind value of +150 takes 0.6 seconds.

Thanks for your help!

For anyone reading, I ended up moving my spreadsheets to Microsoft Access, and used ADODB/SQL Queries from Excel to retrieve values from the Access database. This helped speed up my code by approx 10x, so definitely worth the move.


Could anyone say if this is a reliable shuffling code? I am trying to prove the famous "Monty Hall Paradox":

  The Monty Hall problem is a probability puzzle named after Monty Hall, the original host of the TV show Let’s Make a Deal. It’s a famous paradox that has a solution that is so absurd, most people refuse to believe it’s true.

private static void suffle(ulong numberOfTries)
    {
        int correctHits = 0;
        int wrongHits = 0;

        var prizeIndex = new Random();

        for (ulong i = 1; i <= numberOfTries; i++)
        {
            int[] doors = { 1, 2, 3 };

            var prizeDoor = doors[prizeIndex.Next(0, doors.Length)] ;

            var selectedDoor = doors[prizeIndex.Next(0, doors.Length)];

            int discardedDoor = doors[prizeIndex.Next(0, doors.Length)];

            while (discardedDoor == prizeDoor || discardedDoor == selectedDoor)
            {
                discardedDoor = doors[prizeIndex.Next(0, doors.Length)];
            }

            var correctGuess = selectedDoor == prizeDoor;

            if (correctGuess) correctHits++;
            else wrongHits++;

            Console.WriteLine($"Prize Door: {prizeDoor}\nSelected Door: {selectedDoor}\nDiscarded Door: {discardedDoor}\nRight Guess? {correctGuess}\n\n");
        }

        Console.WriteLine($"Right Guesses: {correctHits} / {numberOfTries}\nWrong Guesses: {wrongHits} / {numberOfTries}");
    }

your code is pretty neat and readable. Shuffling algorithm looks reliable to me, however I have a few comments to your C# code.

You pass the argument numberOfTries as ulong type, but correctHits and wrongHits are int. It could lead to wrong output if the numberOfTries is large enough to overflow int.MaxValue.
Variable doors are initialized in each loop. It could be initialized once in higher scope, e.g. before the for loop.
I can see repetetive code doors[prizeIndex.Next(0, doors.Length)]. This piece of code could be refactored to isolated method. For example:

private static int GetDoorNumber() => _doors[_prizeIndex.Next(0, _doors.Length)];
You can save couple of code lines introducing ternary operator. Then you do not need variable correctGuess.

correctHits += (selectedDoor == prizeDoor) ? 1 : 0;

You can calculate wrongHits as a difference of numberOfTries and correctHits. But your solution works too.


Trying to improve and better understand Intel SSE intrinsics I've been writing a collection of signal processing function optimized with SSE intrinsics (mostly for audio processing). Here is a linear interpolation function I wrote. It works well and is quite fast (much better than straight lerp), but I'm using the full 8 registers available, and I'm wondering if there's something I'm not seeing that would improve its efficiency.

void SSE_vInterpLinear (const float *sourceX, const float *sourceY, float *resultY, const int lengthX)
{
    assert(lengthX % 4 == 0);

    __m128 *mXphases = (__m128*)sourceX;
    __m128 *mYresult = (__m128*)resultY;
    __m128 mX0, mY0, mY1, mYtemp, mXtemp, mtemp;
    __declspec(align(16)) float pos[4];

    for (int i = 0; i < lengthX; i+=4) {
        // floor the values in X to get indexing positions at X0, and add 1 for X1 index positions
        mX0 = _mm_floor_ps(*mXphases);
        _mm_store_ps(pos, mX0);
        mY0 = _mm_set_ps(sourceY[(int)pos[3]], sourceY[(int)pos[2]], sourceY[(int)pos[1]], sourceY[(int)pos[0]]);
        mY1 = _mm_set_ps(sourceY[(int)pos[3]+1], sourceY[(int)pos[2]+1], sourceY[(int)pos[1]+1], sourceY[(int)pos[0]+1]);

        mYtemp = _mm_sub_ps(mY1, mY0);
        mXtemp = _mm_sub_ps(*mXphases, mX0);
        mtemp = _mm_mul_ps(mYtemp, mXtemp);
        *mYresult = _mm_add_ps(mY0, mtemp);

        mXphases++;
        mYresult++;
    }
}

One improvement would be to restrict the number of (random) memory accesses. First notice is that for each sourceY[fixed] there's also a memory read from sourceY[fixed+1];

I believe these should be at least combined to single 64-bit memory accesses.

A more crucial improvement would be at higher level: is the interpolation really random, or could the xvalues be say localized to say 8-15 successive indices? Then I'd pursuit for a technique that reads them to 2 or 4 xmm registers and tries to shuffle the values in place an in parallel.