Parsing an IP address (Bit manipulation) Given a string representing of an IP address, such as "127.43.23.59", return a 32 bit integer representation of this string. The 32 bit integer is separated into four bytes, where each byte is one of the numbers in the string, separated by dots. If you  & 0xFF with the returned integer, it should return 59. If you & 0xFF00 with the returned integer, it should return 23, etc.

public class IP{
    public static void main(String[] args)
    {
        IP address = new IP();
        int x = address.IPAddress("127.43.29.56");
    }
    public int IPAddress(String str)
    {
        int byte1 = 0;
        int numbers[] = new int[4];
        int index = 0;
        int finalByte = 0;
        for(int i = 0; i < str.length(); i++)
        {
            if(str.charAt(i) != '.')
                byte1 = 10*byte1 + (str.charAt(i) - 48);
            else
            {
                numbers[index++] = byte1;
                byte1 = 0;
            }
        }
        numbers[index] = byte1;
        return finalByte = (numbers[0]<<24) | (numbers[1]<<16)|
                    (numbers[2]<<8) | (numbers[3]);
    }
}

First impressions

Some basic issues that immediately jump in the eye:

A method that doesn't need data in member fields can be static. The IPAddress method looks like a good candidate for a static utility method in an IPAddressUtils class.
The Java convention is camelCase for method names (violated by IPAddress)
Poor naming: the IPAddress method takes a dotted decimal notation and returns a 32-bit integer representation. The name "IPAddress" does very little to describe that
finalByte is unused, unnecessary
Unusual declaration: instead of int numbers[], the convention is int[] numbers
Unusual formatting

Simplification

You don't need an int[] to calculate the value. You need just one value, and shift it to the left by 8 bits for each parsed byte value.

Instead of the number 48 for '0', you can just use '0' to make the meaning obvious.

Suggested implementation

Applying the above suggestions, here's a utility class:

public class IPAddressUtils {

    public static int toInt(String dottedDecimal) {
        int byte1 = 0;
        int value = 0;
        for (char c : dottedDecimal.toCharArray()) {
            if (c != '.') {
                byte1 = 10 * byte1 + (c - '0');
            } else {
                value <<= 8;
                value += byte1;
                byte1 = 0;
            }
        }
        value <<= 8;
        value += byte1;
        return value;
    }
}


Thread Safe Bank Transaction: Deposit, Withdrawal, Check Balance and Transfer in Java I have made a class which is a thread-safe implementation supporting Deposit, Withdrawal, Check Balance Querying and Transferring of money.
I have a class Account:

public class Account {
    private int balance;
    private String address;
    private String userId;
    final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
    Lock transferLock = new ReentrantLock();
    public Account(String userId){
        this.userId = userId;
    }

    public void add(int amount){
        balance += amount;
    }
    public void sub(int amount){
        this.balance -= amount;
    }
    public int getBalance(){
        return balance;
    }
}

I use a ReentrantReadWriteLock for Depositing and Withdrawing which acquires a Write Lock.
Read lock is acquired when checking an Account's balance.
A 'transferLock' is used to lock the transferring of funds from Account A to Account B.

Here are the main functions:

 public boolean deposit(String userId, int amount){
        Account account = getUserAccount(userId);
        Lock accountWriteLock = account.lock.writeLock();
        accountWriteLock.lock();

        account.add(amount);

        accountWriteLock.unlock();
        return true;
    }

    public boolean withdraw(String userId, int amount){
        Account account = getUserAccount(userId);
        Lock accountWriteLock = account.lock.writeLock();
        accountWriteLock.lock();

        if(account.getBalance() < amount){
accountWriteLock.unlock();
            return false;
        }
        account.sub(amount);

        accountWriteLock.unlock();
        return true;
    }

    public int checkBalance(String userId){
        Account account = getUserAccount(userId);
        Lock accountReadLock = account.lock.readLock();
        accountReadLock.lock();

        int balance = account.getBalance();

        accountReadLock.unlock();
        return balance;
    }

    public boolean transfer(String fromAccountUserId, int amount, String toAccountUserId) throws Exception {
        Account fromAccount = getUserAccount(fromAccountUserId);
        Account toAccount = getUserAccount(toAccountUserId);
        Random rand = new Random();
        Lock fromAccountTransferLock = fromAccount.transferLock;
        Lock toAccountTransferLock = toAccount.transferLock;
        while (true) {
            if (fromAccountTransferLock.tryLock()) {
                fromAccount.lock.writeLock();
                if (toAccountTransferLock.tryLock()) {
                    try {
                        withdraw(fromAccountUserId, amount);
                        deposit(toAccountUserId, amount);
                        break;
                    } catch (Exception e) {
                        return false;
                    } finally {
                        toAccountTransferLock.unlock();
                    }
                }
                fromAccountTransferLock.unlock();
                Thread.sleep(rand.nextInt(1001));
            }
        }
        return true;
    }

Assume that getUserAccount(String userId) retrieves the appropriate Account object.

What I Would Like Reviewed
  1. Is my code thread-safe?
  2. Any improvements you think would be good?

Notes
  1. I prevent Deadlock in Transferring from Account A to Account B by allowing the holding thread to release its lock.
  2. I prevent LiveLock in transfer(...) by sleeping the thread for a random number of milliseconds between [0,1000] ms.

EDIT:
Here is the getUserAccount() code:

public Account getUserAccount(String userId){
        return new Account(userId);
    }

Let's break this down.

You have two locks for single object that guarding same fields.

It's generally bad practice, and it doesn't make sense as second lock(transferLock) is used only in single operation. It won't guard for other operations.
It could possibly work if you have a static read-only lock for transfer with the following semantics: add/sub/get are global read lock and transfer is global write lock, then yu should acquire read lock on transfer on regular operations and write-lock on transfer op, but that would allow on;y single transfer at a time, and that's definitely not what you are trying to achieve.

Your locks leak outside of your main class (reed aren't private/protected).

This could lead to uncontrolled use from outside, say future versions of code could misuse locks. I can see that they have package level visibility, that's semi ok, but Account#add,Accound#sub, Account#getBallance are public and not guarded by locks, so that destroys the purpose of haveing locks package-private.
I'd suggest put all usage of locks inside instance methods mentioned above, and add another method:
class Account{
public void transfer(Account other, int amount) { ... }
}

Then all logic related to locking would be in a single, controlled place.

Lock ordering. If you are using granular locks, so no single lock for all accounts and you need to lock several objects you should have STRICT lock order.

You are trying to achieve this with tryLock and while loop, this COULD actually work if you have used your rw lock, but it would be possible to deadlock as you are acquiring other locks then. And it's not really performant to wait in spin-loop here, you can wait forever.
I'd suggest to user your rw-lock but first, sort two accounts by accountId, then take write locks in order of account ids from lower to higher.
//pseudocode
if (acc1.id<acc2.id) {
  acc1.lock.writeLock().lock()
  acc2.lock.writeLock().lock()
}else{
  acc2.lock.writeLock().lock()
  acc1.lock.writeLock().lock()
}

It could be actually combined with tryLock, but it could take a while to acquire such locks.

Minor thing, it's preferable to call unlock() inside finally block, so no objects would stick in a locked state.


Filtering a recursive directory listing, discarding subfolders I needed a function that would take a list of folders and removed all sub folders, so that only the top level folders stayed in the list.

For example, given the list:

c:\stuf\morestuff
c:\stuf\morestuff\sub1
c:\otherstuf
c:\otherstuf\sub1
c:\otherstuf\sub2

I wanted the list to be reduced to:

c:\stuf\morestuff
c:\otherstuf

So I came up with this solution:

// remove empty strings and sub folders
private static void CleanUpFolders(List<string> uniqueFolders)
{
    uniqueFolders.RemoveAll(
        delegate(string curFolder)
        {
            // remove empty
            if (string.IsNullOrEmpty(curFolder))
                return true;

            // remove sub paths
            if (uniqueFolders.Exists(
                delegate(string s)
                {
                    if (!string.IsNullOrEmpty(s) &&
                        curFolder.StartsWith(s) &&
                        string.Compare(s, curFolder) != 0)
                        return true;
                    return false;
                } ))
                return true;

            return false;
        }
    );
}

This seems to work (not very well tested though) but I was left wondering about some things:

is there an issue with using variables inside anonymous methods that were declared outside?
any potential issues with nested anonymous methods?
any other issues or best practices worth mentioning?

is there an issue with using variables inside anonymous methods that were declared outside?

No, this is what anonymous methods are designed for! It is a very useful trick to keep up your sleeve. Read up on Closures. There are all sorts of things you can do with them.

Obviously there are issues with doing anything when you don't understand it fully, but the way you are using them in your code is what these things are all about!

  any potential issues with nested anonymous methods?

Same thing.

  any other issues or best practices worth mentioning?

Unless you are still using c# 2, the syntax has been simplified to use what is known as a lambda. Instead of using

delegate(string curFolder) { ..code.. }

you can just go :

curFolder => ..code..

As RemoveAll takes a Predicate, you can also lose the return key word. As long as the statement evaluates to True or False, it will take that as the return.

You have some code that is basically going :

if x == true
    return true
return false

This can be simplified to :

return x

With those two things, your code could be simplified to :

 uniqueFolders.RemoveAll(
        curFolder =>

            string.IsNullOrEmpty(curFolder) ||
                   uniqueFolders.Exists( s=>
                        !string.IsNullOrEmpty(s) &&
                        curFolder.StartsWith(s) &&
                        string.Compare(s, curFolder) != 0)

    );

Its a bit of a mouthfull. You may want to factor out a new method.

uniqueFolders.RemoveAll( curFolder => IsNotRootFolder(uniqueFolders, curFolder ) );

bool IsNotRootFolder ( uniqueFolders, curFolder )
{
   return string.IsNullOrEmpty(curFolder) ||
          uniqueFolders.Exists( s=>
            !string.IsNullOrEmpty(s) &&
            curFolder.StartsWith(s) &&
            string.Compare(s, curFolder) != 0)
}


Efficient Sum of Primes I wrote this simple implementation of the sum of primes for the code eval

Challenge Description:
Write a program which determines the sum of the first 1000 prime numbers.

I would like to know of a better - more efficient - way of implementing the solution.
import java.lang.Math;

public class SumOfPrimes {

    public static boolean isPrime(int n) {
        if (n < 2) {
            return false;
        }
        if (n == 2) {
            return true;
        }
        if (n % 2 == 0) {
            return false;
        }
        for (int i = 3; i <= Math.sqrt((double) n); i += 2) {
            if (n % i == 0) {
                return false;
            }
        }
        return true;
    }

    public static void main(String[] args) {
        long sum = 0;

        for (int i = 1, count = 0; count < 1000; i++) {
            if (isPrime(i)) {
                sum += i;
                count++;
            }
        }
        System.out.print(sum);
    }

}

As for the primality test I only know of these other:
for (int divisor = 2; divisor < n; divisor++) {
    if (n % divisor == 0) {
        return false;
    }
}
return true;

and
for (int divisor = 2; divisor <= n/2; divisor++) {
    if (n % divisor == 0) {
        return false;
    }
}
return true;

You've written a very literal and straightforward implementation to solve the challenge: test numbers for primality, and accumulate them until you have 1000 of them.

However, when you want to find many prime numbers, an algorithm such as the sieve-of-eratosthenes is more efficient than repeated trial division.  Here is a benchmark that illustrates the difference.  Your program follows the same basic outline as @Legato's solution, except that you haven't implemented some optimization hacks.  If you just want to sum the first 1000 primes, there isn't much of a difference in execution time, but your program will not scale up as gracefully as a sieve algorithm.


File generator in shell script I'm a student, and most of my lab work includes writing programs (usually .cpp) run them, copy the source code and output to a word file and either mail a pdf to the concerned teacher or submit in print.
(I still don't get the point of submitting a program in print or pdf file, but this is what we're asked to do)

So I wrote a little shell script to help me automate this stuff.

The packages I'm using are:

enscript
ps2pdf
pdftk

Here is the script code:

#!/bin/bash

TARGET='/home/angg/code'
cd $TARGET

find . -type f -name '*.cpp' | while read CPPFILE
do
    TITLE=$(basename $CPPFILE .cpp)

    g++ $TITLE.cpp

    echo $CPPFILE | xargs enscript --color=1 -C -Ecpp -B -t $TITLE -o - | ps2pdf - $TITLE.pdf

    ./a.out > $TITLE.txt && enscript -B $TITLE.txt -o - | ps2pdf - $TITLE.output.pdf

    pdftk $TITLE.pdf $TITLE.output.pdf cat output $TITLE.final.pdf

    rm $TITLE.output.pdf
    rm $TITLE.pdf

done

today=`date +%j-%M`

pdftk *.pdf cat output $today.pdf

I referred this link to convert all cpp files to pdf files, to add output at the end of each cpp file, I created a separate pdf for output of each program and merged the two pdfs together using pdftk
and then removing pdfs not needed now

I need a review regarding the code and if I can shorten it. Also, I'm new to all these packages used and would want to know if there's any redundancy I could avoid.

Automated suggestions

If you don't have shellcheck installed, grab it, or use the online service.  It reports a bunch of questionable constructs:

202986.sh:4:1: warning: Use 'cd ... || exit' or 'cd ... || return' in case cd fails. [SC2164]
202986.sh:6:38: note: read without -r will mangle backslashes. [SC2162]
202986.sh:8:22: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:10:9: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:12:10: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:12:61: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:12:84: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:14:15: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:14:41: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:14:68: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:16:11: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:16:22: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:16:51: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:18:8: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:19:8: note: Double quote to prevent globbing and word splitting. [SC2086]
202986.sh:23:7: note: Use $(..) instead of legacy `..`. [SC2006]
202986.sh:25:7: note: Use ./*glob* or -- *glob* so names with dashes won't become options. [SC2035]
202986.sh:25:24: note: Double quote to prevent globbing and word splitting. [SC2086]

Prefer lower-case for variable names

We normally use upper-case names for environment variables intended to change the behaviour of programs we use.  It's better to use lower-case for ordinary shell variables.

Check for program failure

At present, if compilation fails, we'll still proceed, likely with old code.  Instead, we should abort execution if any of the steps fail.  It's pretty easy to ask the shell to do this for us (though it's worth learning the exceptions); we can also ask it to check we don't expand any undefined variables, too:

set -eu

Useless use of xargs

I've never seen this pattern before:

echo "$filename" | xargs program

Unless your filename contains whitespace (the rest of the code implies it doesn't), that's exactly

program "$filename"

Unusual date conversion

Do you really mean to give the result file a name given by day-within-year and minute-within-hour?  That's a surprising combination, and certainly deserves a justifying comment if it is really what you want.

Avoid temporary files

Pipeline processing can produce results with lower latency than storing intermediates to temporary files.  We can pipeline one input into pdftk and create the other using a process substitution in roughly this manner:

enscript "$CPPFILE" | ps2pdf \
    | pdftk - <(./a.out | enscript | ps2pdf) \
            cat output "$TITLE.final.pdf"

Consider using Make

If you make a change to a single file, you need to re-run the script (re-doing all the compilation and output processing) to update the final output.  Make is more intelligent: you can write pattern rules that enable you to re-build and re-run only what's out of date, without wasting time and energy on the bits that haven't changed.

It can also clean up the temporary files, and you could write a "clean" rule to clean up all the result files.

If you do write a Makefile, be sure to bring it here for review!


Revert knockout model original state back on update cancel? I am using knockout in my project. I have some models like EmployeeModel, ServiceModel etc which user can update. On UI I give the user a Cancel control button by which he can revert the changes. On cancel I have to revert the model change back to its original state. For this I found these two good approaches by @rniemeyer:

Protected Observable

I can't use protected observables because I am using ko.mapping plugin which will create a knockout models base on server side viewmodels. Yes, I can use mapping option like Create, but I feel its not a good to keep separate observables state. I want solution which will be reusable and can apply to whole model.

And guess what I got, second good approach by same author @rniemeyer:

Simple editor pattern

I can't use this also because of the same reason I mentioned above. So I wrote 2 functions copy and reset which will do the job for me:

Copy

Here, source is the object whose current state we want to preserve, and dest keeps the current state.

function copy(source, dest) {

    for (var key in source) {

        if (source.hasOwnProperty(key)) {

            //no need to keep knockout added properties
            if (key === "__ko_mapping__")
                continue;

            //recursive call to copy if property is an object
            else if (typeof source[key] === "object") {
                dest[key] = {};
                copy(source[key], dest[key]);
            }

            else {
                //no need to copy the computed observable value
                if (!ko.isComputed(source[key])) {
                    var value = ko.utils.unwrapObservable(source[key]);
                    dest[key] = value;
                }
            }
        }
    }

    //remove all function
    for (var key in dest) {
        if (typeof dest[key] === "function")
            delete dest[key];
    }
}

Reset

Here, source is the previous dest and dest is the previous source.

function reset(source, dest) {

    for (var key in source) {

        if (source.hasOwnProperty(key)) {

            //recursive call to copy if it is an object
            if (source[key] !== null && typeof source[key] === "object") {
                reset(source[key], dest[key]);
            }

            else {
                if (ko.isObservable(dest[key]))
                    dest[key](source[key]);
                else
                    dest[key] = source[key];
            }
        }
    }
}

I want you to please review these function and let me know if these function require any improvement.

From a KISS perspective, I would simply load the model again, then you would not need any of this code.

Other than that the code is clean and maintainable, some nitpicking though:

You declare var key twice in copy, that is not required
You are not using hasOwnProperty(key) when you delete all the functions in copy
You could have merged source.hasOwnProperty(key) and key === "__ko_mapping__" with a &&
You access source[key] a lot, you could have considered a var value = source[key] and then work with value


Massive IP2host scanner I'm writing a "IP2host" resolver. Basically all it does is a NSLookup and output's all domain's on the IP. I'm trying to run this on a world scan that's 40M+ lines of IP's.

I have used basic threading but just wondering if I could do anything better to make my code more efficient.

#! python !#
import requests, sys, os, re, getopt, threading

ips = open(sys.argv[1], "r").readlines()

class ip2host(threading.Thread):
    def __init__ (self, ip):
        threading.Thread.__init__(self)
        self.ip = ip.rstrip("\n")
    def run(self):
        try:
            url = "http://ip-www.net/" + self.ip
            response = requests.get(url, timeout=10)
            text =  response.content
            sites = re.findall('">(.*?)</a>', text)
            for w in sites:
                if "You are using" in w: sites.remove(w)
            for w in sites:
                if "Google Chrome" in w: sites.remove(w)
            for w in sites:
                if "Tweet" in w: sites.remove(w)
            for w in sites:
                if "wa-com.com" in w: sites.remove(w)
            for site in sites:
                print site
                os.system("echo "+site+" >>output.txt")
        except:
            pass

print "Starting scan...\n"
for ip in ips:
    try:
        scan = ip2host(ip)
        scan.start()
        time.sleep(0.05)
    except:
        pass

I appreciate all input and replies. I'm just trying to learn.

Proper parallelism is hard in any language. You should definitely take the time to at least skim through all of the python docs for both the threading module AND the multiprocessing module. (see details below)

Also, in python there's an added wrinkle in that there is a Global Interpreter Lock. The simplified explanation is that Threads from the threading module run asynchronously, but not actually in parallel while multiprocessing Processes run in true parallel. For code like yours where you're querying the web and waiting for IO, threading helps because you can get something else done while waiting. But you might also get a speedup from dividing the load with multiprocessing (and have several threads inside each of those). Mulitprocessing processes take longer to setup so they're not as good when you're waiting on IO (as you are with response.get() ), but they run in true parallel--different processes in the system.

I'm not an expert on tuning between the two yet. I'll just warn you that when there are identically named communication objects like Queue and Event inside both the threading and multiprocessing module, and you have to use the correct one for the boundary your communication is crossing or it won't work.

Usually you want try statements to encapsulate the minimum amount of code possible so you can better predict what to do with the errors. As such response = response.get(... is probably the only line currently inside that actually needs to be in the try statement.

Edit: this section originally used dict keys which are only good for exact matches.

Removing specific responses can be done in one loop instead of going back over the loop many times. Also, the column of if statements can be made into a single statement by looking at a list. Something like:

removelist = ["You are using", "Google Chrome", "Tweet", "wa-com.com"]

for site in sites:
    for x in removelist:
        if x in site:
            sites.remove(site)
            break

Writing to a text file via a command line passed into os.system is not the most efficient method of doing that. First because you're involving the os module as a middle man when you don't need to, and second because you're doing all of this from up to 40M+ threads at once.

Instead write the output from each thread to a python Queue.

for site in sites:
    sitequeue.put(site)

Then have a single thread (or your main thread) read from the Queue and write to the file. Something like

with open('output.txt') as f:
    while not all_done_event.is_set():
        f.write(sitequeue.get())

Next you probably don't want 40M+ threads going at once, but time.sleep(0.5) is not the best way to control the number of threads launched. Instead determine the approximate maximum number of threads you can launch before the first one gets a response, and use a Semaphore object to control when new threads are launched. This keeps you always at the optimal number until you are done rather than using a guess about how long each thread will run. Something like:

maxthreads = 15
threadpool = BoundedSemaphore(value=maxthreads)
for ip in ips:
    threadpool.acquire()
    scan = ip2host(ip)
    scan.start()

and have threadpool.release() at the end of the ip2host.run() function to signal that the next thread can be started. Again, you may also want to split ips groups and wrap the threading objects in a multiprocessing Pool class object to get true parallelism on top of threading.

Hope this helps!


Get Workbook Method(s) I'm re-writing my Module of standard methods. These 3 are used to retrieve Workbooks as Workbook Objects. The standard I'm aiming for here is "Third-party Library/Add-in". So:

Do these functions adhere to the principle of least surprise?
Have I missed any potential error cases?

As always, general feedback is also welcome.

Public Function GetWorkbook(ByVal strFileName As String, Optional ByVal strFilePath As Variant) As Workbook

        If Not (IsMissing(strFilePath) Or strFilePath.TypeName = "String") Then
            PrintErrorMessage "File Path was not supplied as a string", stopExecution:=True
        End If

    Dim wbIsOpen As Boolean

        wbIsOpen = IsWorkbookOpen(strFileName)
        If wbIsOpen Then
            Set GetWorkbook = Workbooks(strFileName)
        Else
            If Not IsMissing(strFilePath) Then
                Set GetWorkbook = OpenWorkbook(strFilePath & strFileName)
            Else
                PrintErrorMessage "Workbook (" & strFileName & ") is not open, and no filepath was supplied", stopExecution:=True
            End If
        End If

End Function

Public Function IsWorkbookOpen(ByVal strTargetName As String) As Boolean

    Dim wbTest As Workbook

        On Error Resume Next
            Set wbTest = Workbooks(strTargetName)
            IsWorkbookOpen = (wbTest.Name = strTargetName)
        On Error GoTo 0

End Function

Public Function OpenWorkbook(ByVal strFileName As String, strFilePath As String) As Workbook

    Dim strFullFilename As String
        strFullFilename = strFilePath & strFileName

    Dim wbOpenedSuccessfully As Boolean

        On Error Resume Next
            Set OpenWorkbook = Workbooks.Open(strFullFilename)
            wbOpenedSuccessfully = IsWorkbookOpen(strFileName)
        On Error GoTo 0

        If Not wbOpenedSuccessfully Then
            PrintErrorMessage "Failed to open workbook """ * strFullFilename & """", stopExecution:=True
        End If

End Function

I will not mention again that I find the outdenting of Dim statements hurt readability. Instead I'll focus on this part:

On Error Resume Next
    Set wbTest = Workbooks(strTargetName)
    IsWorkbookOpen = (wbTest.Name = strTargetName)
On Error GoTo 0

What error are you expecting to trap here?

Workbooks(strTargetName)

This will raise runtime error 9 - Subscript out of range. By then you already know everything you need to know.

(wbTest.Name = strTargetName)

This will raise runtime error 91 - Object variable or With block variable not set - but you already know if that call is going to work before you even try it.

This would be cleaner - slightly overkill with error-handling, but cleaner:

Public Function IsWorkbookOpen(ByVal wbFullFileName As String) As Boolean

    On Error GoTo CleanFail
    IsWorkbookOpen = Not Workbooks(wbFullFileName) Is Nothing

CleanExit:
    Exit Function
CleanFail:
    Err.Clear
    Resume CleanExit
End Function

Notice the difference between strTargetName and wbFullFileName: "str" says "that's a string". "wb" says "that's something about a workbook". Both are Hungarian, only one of them is doing it right. Avoid prefixes that indicate the type.


Save space occupied by 2D array My goal is to save space occupied by 2D array (sea) that has 3 different values.

Ocean sea; //Ocean class has member 'private int[][] oceanMatrix;'

public final static int EMPTY = 1;
public final static int SHARK = 2;
public final static int FISH = 3;

by compressing it and place in two one dimensional array.

int[] runType = new int[10]; // each cell stores 1 or 2 or 3

int[] runLength = new int[10]; // corresponding index cell stores its number of occurence of that respective runType.

I am using Runlength encoding idea and wrote below code, which is yet to be fit into a Java class.

I need feedback/critiques on this logic of the below code and coding style, so that I can optimize and make it better. Constraint is to not use any existing Java library class except (array's length member).

private static int[] doubleTheSize(int[] arg){
        int[] temp = new int[2*arg.length];
        for(int c = 0; c < arg.length; c++){
            temp[c] = arg[c];
        }
        return temp;
    }

public static void main(String[] args) {
            Ocean sea;
            int[] runType = new int[10];
        int[] runLength = new int[10];
        int index = 0;
        boolean firstCell = true;
 //i&j are width & height of a 2d array read from commandline
        for(int row = 0; row < j ; row++){
            for(int col = 0; col < i ; col++){
                if(firstCell){
                    firstCell=false;
                    runType[index] = sea.cellContents(0, 0);
                    runLength[index]++;
                    continue;
                }
                switch(sea.cellContents(row, col)){
                case Ocean.EMPTY:
                    if(runType[index] == Ocean.EMPTY){
                        runLength[index]++;
                    }else{
                        index++;
                        if(index==runType.length){
                            runType = doubleTheSize(runType);
                            runLength = doubleTheSize(runLength);
                        }
                        runType[index]=Ocean.SHARK;
                        runLength[index]++;
                    }
                    break;
                case Ocean.SHARK:
                    if(runType[index] == Ocean.SHARK){
                        runLength[index]++;
                    }else{
                        index++;
                        if(index==runType.length){
                            runType = doubleTheSize(runType);
                            runLength = doubleTheSize(runLength);
                        }
                        runType[index]=Ocean.SHARK;
                        runLength[index]++;
                    }
                    break;
                case Ocean.FISH:
                    if(runType[index] == Ocean.FISH){
                        runLength[index]++;
                    }else{
                        index++;
                        if(index==runType.length){
                            runType = doubleTheSize(runType);
                            runLength = doubleTheSize(runLength);
                        }
                        runType[index]=Ocean.FISH;
                        runLength[index]++;
                    }
                    break;
                }// end switch
            }//end inner for loop
        }//end outer for loop
}//end main()

I wrote the above code for that.  Please help me review this code for optimization. One point that I recognized is, firstCell check that I did for every cell doesn't make sense. I need suggestion to better this logic because I need to enter in firstCell i{} block for only (0,0) cell.

A few notes about the original code.

If the ocean mostly empty it's actually a sparse matrix. If you are allowed to use more arrays or another primitive data structures check the Spare matrix Wikipedia article about it, it contains other, maybe more effective storage structures.
The second switch case contains the same logic three times. You could extract it out to a method to remove duplication:

private static int encode(int[] runType, int[] runLength, int currentType, int index) {
    if (runType[index] == currentType) {
        runLength[index]++;
    } else {
        index++;
        if (index == runType.length) {
            runType = doubleTheSize(runType);
            runLength = doubleTheSize(runLength);
        }
        runType[index] = currentType;
        runLength[index]++;
    }
    return index;
}

And use it in the switch:

switch (sea.cellContents(row, col)) {
case Ocean.EMPTY: {
    final int currentType = Ocean.EMPTY;
    index = encode(runType, runLength, currentType, index);
    break;
}
case Ocean.SHARK: {
    int currentType = Ocean.SHARK;
    index = encode(runType, runLength, currentType, index);
    break;
}
case Ocean.FISH: {
    int currentType = Ocean.FISH;
    index = encode(runType, runLength, currentType, index);
    break;
}

After that it's easier to notice that the whole switch-case could be eliminated since the body of every case statement is very similar to each other and depends on only the sea.cellContents(row, col) value.

final int currentType = sea.cellContents(row, col);
index = encode(runType, runLength, currentType, index);

int[] runLength = new int[10]; // corresponding index cell stores its number of occurence of that respective runType.

I'd put the comment before the line it comments:

// corresponding index cell stores its number of occurrence of that respective runType.
int[] runLength = new int[10];

It's easier to work with because it requires less horizontal scrolling.
A comment says the following:

// i&j are width & height of a 2d array read from commandline

Then you can use width and height as variable names for better readabilty (you don't have to remember what i or j mean nor check the comment for it) and could get rid of the comment.
The temp variable in doubleTheSize could be called doubledArray or newArray, arg is oldArray or originalArray. It would express the purpose of them.
Comments on the closing curly braces are unnecessary and disturbing. Modern IDEs could show blocks.

    }// end outer for loop
}// end main()

“// …” comments at end of code block after } - good or bad?


Python Clean way to get whether a number is above/below two values and increment accordingly Consider (simplified)

low_count = 0
high_count = 0
high = 10
low = 5
value = 2

What is a clean way to check a number value versus a min low and a max high, such that if value is below low low_count increments, and if value is above high high_count increments? Currently I have (code snippet)

            high_count = 0
            low_count = 0
            low = spec_df['Lower'][i]
            high = spec_df['Upper'][i]
            #Calculate results above/below limit
            for result in site_results:
                if result<low:
                    low_count+=1
                else:
                    if result>high:
                        high_count+=1

Thanks!

I would write it as

low_count = sum(map(lambda x: x < low, site_results))
high_count = sum(map(lambda x: x > high, site_results))

but admit I'm spoiled by Haskell.

Edit: As suggested by @Nick Burns, using list comprehension will make it even clearer:

low_count = sum(x < low for x in site_results)
high_count = sum(x > high for x in site_results)


React components to render two kinds of form elements I have two React components. The only difference is the input tag vs select tag. I'm trying to move the bulk of the duplicate code to a higher order component but I can't wrap my head around it. Any suggestions?

import React from "react";
import Style from "./style.scss";

export class FloatingLabelInput extends React.Component {
    render() {
        const { label, ...props } = this.props;
        const clsOK = props.value ? Style.ok : "";
        return (
            <div className={`${Style.wrapper}`}>
                <input className={`${Style.input} ${clsOK}`} ref={el => this.input = el} {...props} />
                <label className={`${Style.label}`} htmlFor={props.id}>{label}

        );
    }
}

export class FloatingLabelDropDown extends React.Component {
    render() {
        const { label, ...props } = this.props;
        const clsOK = props.value ? Style.ok : "";
        return (
            <div className={`${Style.wrapper}`}>
                <select
                    className={`${Style.input} ${clsOK}`}
                    ref={el => this.input = el} {...props} >
                    {this.props.children}
                </select>
                <label className={`${Style.label}`} htmlFor={props.id}>{label}

        );
    }
}

I think I have an understanding of what you are trying to do, but because select has children and and input does not it's a little tricky but this is what I came up with:

import React, { Component } from 'react'
import { render } from 'react-dom'

class FloatingLabel extends Component {
  render() {
    return (
      <div>
        <this.props.render option='Hello'  />
      </div>
    )
  }
}

class SelectDropDown extends Component {
  render() {
    return (
      <select>
        <option>{this.props.option}</option>
      </select>
    )
  }
}

class InputDropDown extends Component {
  render() {
    return (
      <input />
    )
  }
}

render(<FloatingLabel render={SelectDropDown} />, document.querySelector('#root'))

In this example you can create a <FloatingLabel /> component that can either take a SelectDropDown or a InputDropDown component as a prop. You can then render that prop in the FloatingLabel parent and pass whatever props that you need to pass down to the children.

Basically what you would be changing is the everything inside the div except for the label.


Deep nesting when looping over an object model coming from a 3rd part I am stuck with this horrible object model coming back from a 3rd party product.  It's six levels of objects deep, and I have to loop through the collection in each level, to get the values I need, in order to produce a small object model that I actually require.

The code ends up looking like this (with variable and type names changed). How can I clean up this mess when I can't modify the structure of the rootObject?

(This is .NET 3.5.)

var levelOneEnumerator = rootObject.GetEnumerator();
while (levelOneEnumerator.MoveNext())
{
    var levelOneItem = levelOneEnumerator.Current as Foo_LevelOneItem;
    if (levelOneItem == null) continue;

    var levelTwoItemsEnumerator = levelOneItem.LevelTwoItems.GetEnumerator();
    while (levelTwoItemsEnumerator.MoveNext())
    {
        var LevelTwoItemsItem = levelTwoItemsEnumerator.Current as Foo_LevelTwoItem;
        if (LevelTwoItemsItem == null) continue;

        var foobars = new List<FooBar>();

        var levelThreeItemsEnumerator = LevelTwoItemsItem.LevelThreeItems.GetEnumerator();
        while (levelThreeItemsEnumerator.MoveNext())
        {
            var levelThreeItem = levelThreeItemsEnumerator.Current as Foo_LevelThreeItem;
            if (levelThreeItem == null) continue;

            var levelFourItemsEnumerator = levelThreeItem.LevelFourItems.GetEnumerator();
            while (levelFourItemsEnumerator.MoveNext())
            {
                var levelFourItem = levelFourItemsEnumerator.Current as Foo_LevelFourItem;
                if (levelFourItem == null) continue;

                var levelFiveItemsEnumerator = levelFourItem.LevelFiveItems.GetEnumerator();
                while (levelFiveItemsEnumerator.MoveNext())
                {
                    var levelFiveItem = levelFiveItemsEnumerator.Current as Foo_LevelFiveItem;
                    if (levelFiveItem == null) continue;

                    var levelSixItemsEnumerator = levelFiveItem.LevelSixItems.GetEnumerator();
                    while (levelSixItemsEnumerator.MoveNext())
                    {
                        var levelSixItem = levelSixItemsEnumerator.Current as Foo_LevelSixItem;
                        if (levelSixItem == null) continue;

                        var levelSixKey = levelSixItem.Key;
                        var foobar = foobars.Where(x => x.Key == levelSixKey).FirstOrDefault();
                        if (foobar == null)
                        {
                            foobar = new FooBar
                                          {
                                              LevelSixKey = levelSixKey,
                                              TransDate = levelFiveItem.TransDate,
                                              PaidAmount = 0
                                          };
                            foobars.Add(foobar);
                        }

                        // * -1 because value should be positive, while product reports a negative (and vice versa)
                        foobar.PaidAmount += (levelFiveItem.PaidAmount ?? 0) * -1;
                    }
                }
            }
        }

        yield return new FooBarsCollection
                         {
                             Prop1 = levelTwoItemsItem.Prop1,
                             Prop2 = levelTwoItemsItem.Prop2,
                             Prop3 = levelTwoItemsItem.Prop3,
                             FooBars = foobars
                         };
    }
}

First of all, I would realise that

var levelOneEnumerator = rootObject.GetEnumerator();
while (levelOneEnumerator.MoveNext())
{
    var levelOneItem = levelOneEnumerator.Current as Foo_LevelOneItem;
    if (levelOneItem == null) continue;

is equivalent to the much shorter

foreach (var levelOneItem in rootObject.OfType<Foo_LevelOneItem>())
{

Then you will realise that you have several nested foreach loops which make for nicely-readable single-line nestings. So factor all the multi-line code into methods of its own. The end-result I got looks like this:

foreach (var levelOneItem in rootObject.OfType<Foo_LevelOneItem>())
    foreach (var levelTwoItem in levelOneItem.LevelTwoItems.OfType<Foo_LevelTwoItem>())
        yield return new FooBarsCollection
        {
            Prop1 = levelTwoItem.Prop1,
            Prop2 = levelTwoItem.Prop2,
            Prop3 = levelTwoItem.Prop3,
            FooBars = getFoobars(levelTwoItem)
        };

[...]

private static List<FooBar> getFoobars(Foo_LevelTwoItem levelTwoItem)
{
    var foobars = new List<FooBar>();

    foreach (var levelThreeItem in levelTwoItem.LevelThreeItems.OfType<Foo_LevelThreeItem>())
        foreach (var levelFourItem in levelThreeItem.LevelFourItems.OfType<Foo_LevelFourItem>())
            foreach (var levelFiveItem in levelFourItem.LevelFiveItems.OfType<Foo_LevelFiveItem>())
                foreach (var levelSixItem in levelFiveItem.LevelSixItems.OfType<Foo_LevelSixItem>())
                    processLevelSixItem(foobars, levelFiveItem, levelSixItem.Key);

    return foobars;
}

private static void processLevelSixItem(List<FooBar> foobars, Foo_LevelFiveItem levelFiveItem, Foo_LevelSixItemKey levelSixKey)
{
    var foobar = foobars.Where(x => x.Key == levelSixKey).FirstOrDefault();
    if (foobar == null)
    {
        foobar = new FooBar
        {
            LevelSixKey = levelSixKey,
            TransDate = levelFiveItem.TransDate,
            PaidAmount = 0
        };
        foobars.Add(foobar);
    }

    // * -1 because value should be positive, while product reports a negative (and vice versa)
    foobar.PaidAmount += (levelFiveItem.PaidAmount ?? 0) * -1;
}

Of course, you could further change this into a much more LINQy expression involving either SelectMany or the from query syntax, but to be honest, in your particular case I would leave it like this. It is very clear. In case you still want the query syntax, here is just getFoobars to give you the idea:

private static List<FooBar> getFoobars(Foo_LevelTwoItem levelTwoItem)
{
    var foobars = new List<FooBar>();

    var query =
        from levelThreeItem in levelTwoItem.LevelThreeItems.OfType<Foo_LevelThreeItem>()
        from levelFourItem in levelThreeItem.LevelFourItems.OfType<Foo_LevelFourItem>()
        from levelFiveItem in levelFourItem.LevelFiveItems.OfType<Foo_LevelFiveItem>()
        from levelSixItem in levelFiveItem.LevelSixItems.OfType<Foo_LevelSixItem>()
        select new { LevelFiveItem = levelFiveItem, Key = levelSixItem.Key };

    foreach (var info in query)
        processLevelSixItem(foobars, info.LevelFiveItem, info.Key);

    return foobars;
}


Needs code review for class implementation could you please review the code for the implementation of the class (link: General review of Robot class)

require_relative 'robot'
class Game

  def initialize
    # initialize values
    @commands = %w{PLACE MOVE LEFT RIGHT REPORT}
    @facings = %w{NORTH SOUTH EAST WEST}
    @pos_X, @pos_Y = nil, nil
    @facing, @robot, @r = nil, nil, nil
    command = get_initial_command
    setup(command)
  end

  def setup(command)
    params = command.gsub(/[PLACE]/, '').split(',')
    if (params[0] != nil && params[1] != nil && params[2] != nil)
      @pos_X  = params[0].to_i
      @pos_Y  = params[1].to_i
      @facing = params[2]
      @r = Robot.new(@pos_X, @pos_Y, @facing)
      if is_valid_position
        @robot = @r
      else
        Game.new
      end
      command = get_commands
    else
      Game.new
    end
  end

  def get_initial_command
    command = gets.strip
    if command.include?("PLACE")
      return command
    else
      command = get_initial_command
    end
  end

  def get_commands
    is_command = false
    command = gets.strip
    @commands.each do |c|
      if command.include?(c)
        is_command = true
        break
      end
    end
    if is_command
      translate_command(command)
      command = get_commands
      return command
    else
      command = get_commands
    end
  end

  def translate_command(command)
    case command
    when command.include?('PLACE')
      puts 'place the command'
      setup(command)
    when 'MOVE'
      @robot.move if is_valid_move
    when 'RIGHT'
      @robot.right
    when 'LEFT'
      @robot.left
    when 'REPORT'
      @robot.report
    when 'EXIT'
      Kernel.abort(false)
    end
  end

  def is_valid_move
    if ((@robot.instance_variable_get("@pos_X") == 0 && @robot.instance_variable_get("@facing") == 'SOUTH') ||
        (@robot.instance_variable_get("@pos_X") == 5 && @robot.instance_variable_get("@facing") == 'NORTH') ||
        (@robot.instance_variable_get("@pos_Y") == 0 && @robot.instance_variable_get("@facing") == 'WEST') ||
        (@robot.instance_variable_get("@pos_Y") == 5 && @robot.instance_variable_get("@facing") == 'EAST'))
          return false
    else
      return true
    end
  end

  def is_valid_position
    is_proper_facing = false
    @facings.each do |f|
      if f.include?(@r.instance_variable_get("@facing"))
        is_proper_facing = true
        break
      end
    end
    if (@r.instance_variable_get("@pos_X") < 0 || @r.instance_variable_get("@pos_X") > 5 ||
      @r.instance_variable_get("@pos_Y") < 0 || @r.instance_variable_get("@pos_Y") > 5 || !is_proper_facing)
      return false
    else
      return true
    end
  end
end

a = Game.new

A couple of issues I found or suggestions I have:

avoid unnecessary or redundant comments (like # initialize values within initialize)
use symbols for your commands and facings
don't use upper case letters in your variable names (pos_x or just x instead of pos_X)
no need to check for != nil, you can simply do params[0] && params[1] &&params[2]
I'd convert simple ifs to use the ternary operator instead: is_valid_position ? @robot = @r : Game.new
find a better name for @r
I'm not sure why you're using instance_variable_get instead of just @robot.pos_X

Try to refactor your longer methods. Take setup as example, personally, I'd like it to look somewhat like this:

def setup(command)
  begin
    params = get_params(command)
  rescue ParamsInvalidError
    restart
  end

  new_robot(params)
  restart unless is_valid_position?
end


(Leetcode) Escape a large maze This is a Leetcode problem -

  In a 1 million by 1 million grid, the coordinates of each grid square
  are (x, y) with \$0\$ \$<=\$ x\$,\$ y \$<\$ \$10^6\$.

  We start at the source square and want to reach the target square.
  Each move, we can walk to a 4-directionally adjacent square in the
  grid that isn't in the given list of blocked squares.

  Return True if and only if it is possible to reach the target square
  through a sequence of moves.

  Example 1 -

Input: blocked = [[0,1],[1,0]], source = [0,0], target = [0,2]
Output: False

# Explanation:
# The target square is inaccessible starting from the source square because we can't walk outside the grid.

  Example 2 -

Input: blocked = [], source = [0,0], target = [999999,999999]
Output: True
# Explanation:
# Because there are no blocked cells, it's possible to reach the target square.

  Note -

  0 <= blocked.length <= 200
  blocked[i].length == 2
  0 <= blocked[i][j] < \$10^6\$
  source.length == target.length == 2
  0 <= source[i][j], target[i][j] < \$10^6\$
  source != target

Here is my solution to this challenge -

from collections import deque

# Bidirectional BFS

def is_escape_possible(blocked: List[List[int]], source: List[int], target: List[int]) -> bool:

    if len(blocked) < 2:
        return True
    n = 10 ** 6
    block_set = set(map(tuple, blocked))

    #initialize two deque

    bfs_que_s = deque([tuple(source)])
    bfs_que_t = deque([tuple(target)])

    # visited sets for the two points

    visited_s = set([tuple(source)])
    visited_t = set([tuple(target)])

    # wave set means the outline of BFS

    wave_s = set([tuple(source)])
    wave_t = set([tuple(target)])

    # BFS into the next layer, if the current queue is empty, return False, which means this point is sealed by blocks

    def bfs_next(bfs_que, visited, wave):
        size = len(bfs_que)
        if not size:
            return False
        wave.clear()
        for _ in range(size):
            c_r, c_c = bfs_que.popleft()
            for dx, dy in [(0, 1), (1, 0), (0, -1), (-1, 0)]:
                n_r = c_r + dx
                n_c = c_c + dy
                if 0 <= n_r < n and 0 <= n_c < n and (n_r, n_c) not in block_set and (n_r, n_c) not in visited:
                    bfs_que.append((n_r, n_c))
                    visited.add((n_r, n_c))
                    wave.add((n_r, n_c))
        return True

    # mark the points have escaped or not

    escape_s = False
    escape_t = False

    # when waves share some same values, they met in the search, return True

    while not wave_s & wave_t:
        if not escape_s:
            if not bfs_next(bfs_que_s, visited_s, wave_s):
                return False

            # when wave's length > blocks' length, the point must have escaped the blocks

            if len(wave_s) > len(blocked):
                escape_s = True
        if not escape_t:
            if not bfs_next(bfs_que_t, visited_t, wave_t):
                return False
            if len(wave_t) > len(blocked):
                escape_t = True

        #both of the points are escaped

        if escape_s and escape_t:
            return True
    return True

Here is my idea of the challenge -

If we conduct two independent searches to find out whether either of the two points is blocked or not, we may search for many duplicated and unnecessary points in the case that two points can reach each other. So we can start BFS from both directions simultaneously, one layer frontward and one layer backward; when they meet, we can return True.

Here is my Leetcode result -

So, I would like to know whether I could make this program shorter and more efficient.

There are few redundancies, both time and memory wise.

You don't need to maintain an entire visited set. It is only necessary for a general graph. For the grid, the wave computed at the previous iteration serves equally good.
Upon completion of bfs_next, both wave and bfs_que contain the same set of points. You don't need to update wave in the loop. Do wave = set(bfs_que) once, after the loop terminates. Since set is a built-in primitive, this should run faster.
I am not sure that dequeue is a right tool. The for _ in range(size) loop effectively drains it every time, and there is no need to store the new wavefront in the same structure with the old one. Consider using two lists instead.

Also, for the clarity of the algorithm, I'd seriously consider reworking bfs_next into a generator, yielding the next wavefront.

All that said, I am not convinced that len(wave) > len(blocked) means escape. In fact, correct me if I am wrong, when 9 blockers cut off the corner

   |*
   | *
   |  *
   |  3*
   | 323*
   |32123*
   |210123*
   |32123  *
   | 323    *
   +----------

the length of the third wavefront is 10. However, there is no escape.


Doubly linked list in Java I did not add generics to my code because I am new to Java. I am on my way in learning data structures and algorithms. I just want to make sure that my code is clean and efficient.

package Double_LinkedList;

class Node{
    int data;
    Node prev;
    Node next;

    void setData(int data){
        this.data = data;
    }

    int getData(){
        return data;
    }

    void setPrev(Node prev){
        this.prev = prev;
    }

    void setNext(Node next){
        this.next = next;
    }

    Node getPrev(){
        return prev;
    }

    Node getNext(){
        return next;
    }

    Node(int data, Node prev, Node next){
        this.data = data;
        this.next = next;
        this.prev = prev;
    }

    Node(int data){
        this.data = data;
    }

    Node(){    }
}

public class DoubleLinkedList{

    private Node head = null;
    private Node tail;

    private int length;

    public Node getHead(){
        return head;
    }

    public Node getTail(){
        return tail;
    }

    public int getData(Node node){
        return node.getData();
    }
    public int getLength(){
        return length;
    }
    public void insertFront(int data){

        Node newNode = new Node(data, null, head);

        if(head != null){
            head.setPrev(newNode);
        }
        head = newNode;
        if(tail == null){
            tail = newNode;
        }
        length++;
    }

    public void insertEnd(int data){

        Node newNode = new Node(data);
        newNode.setNext(null);

        if(tail != null){
            tail.setNext(newNode);
            newNode.setPrev(tail);
            tail = newNode;
        }
        if(head == null){
            head = newNode;
        }

        length++;
    }

    public void removeFront(){

        if(head != null){

            head.getNext().setPrev(null);
            head = head.getNext();

        }else if(head == null){

            System.out.println("List is empty");
            return ;

        }
        length--;
    }

    public void removeEnd(){

        if(tail != null){

            System.out.println("Tail not null");
            Node temp = tail.getPrev();
            temp.setNext(null);
            tail = temp;

        }else{
            System.out.println("Tail is null");
        }
        length--;
    }

    public void printList(){
        Node temp = head;

        while(temp != null){
            System.out.print(temp.getData()+" ");
            temp = temp.getNext();
        }
        System.out.println("");
    }
}

Bug

    public void insertEnd(int data){

        Node newNode = new Node(data);
        newNode.setNext(null);

        if(tail != null){
            tail.setNext(newNode);
            newNode.setPrev(tail);
            tail = newNode;
        }
        if(head == null){
            head = newNode;
        }

        length++;
    }

If tail is null at the beginning, it will stay null.  Like with head in insertFront, you should always set tail in insertEnd.  As is, if you call insertEnd and then insertFront, it will point tail at the one inserted at the front.

    public void insertEnd(int data){
        Node newNode = new Node(data, tail, null);

        if (tail != null) {
            tail.setNext(newNode);
        }
        tail = newNode;
        if (head == null) {
            head = newNode;
        }

        length++;
    }

This way, it always sets tail to newNode.

You also never set the previous node if tail is null.  But you explicitly set the next node to null.  This version always sets both.  It would also work to never set the next node and only set the previous node if there is one.  But please don't mix the two notions.  Either set everything or let everything that can default, default.  Either is readable on its own.  Mixed, it gets a bit confusing.

I tend to favor explicitly setting everything.

I also prefer more whitespace, so that keywords like if are separated from things like parenthetical expressions.  The exception being method calls.  That way if I see foo(, I know immediately that foo is a method and not some language construct.

Don't overcomplicate things

    public void removeFront(){

        if(head != null){

            head.getNext().setPrev(null);
            head = head.getNext();

        }else if(head == null){

            System.out.println("List is empty");
            return ;

        }
        length--;
    }

You check both if head is not null and if it is null.  You can check just one.  Further, since you return if it is null, you can do that first.

    public void removeFront() {
        if (head == null) {
            System.out.println("List is empty");
            return;
        }

        head = head.getNext();
        head.setPrev(null);
        length--;
    }

Now we take care of the exceptional case first, and we don't have to gate the rest of the method.  Note that it would usually make more sense to throw an exception here.  A Deque would throw a NoSuchElementException in that situation.

If we switch the order of the statements, we don't have to write head.getNext() twice.

It's no longer confusing that we decrement length outside the if structure but only actually reach that code in one branch.

Bug 2

    public void removeEnd(){

        if(tail != null){

            System.out.println("Tail not null");
            Node temp = tail.getPrev();
            temp.setNext(null);
            tail = temp;

        }else{
            System.out.println("Tail is null");
        }
        length--;
    }

You decrement length even if there's nothing there.

    public void removeEnd() {
        if (tail == null) {
            System.out.println("Tail is null");
            return;
        }

        System.out.println("Tail not null");

        tail = tail.getPrev();
        tail.setNext(null);
        length--;
    }

This should follow the same pattern as removeFront.

Implement an interface

Note that a Deque has a similar API to this.  Consider implementing that interface for compatibility reasons.  Of course, you may find that that is unnecessary for what you need.


Unit testing static method that imports environment file I'm (very) new to writing unit tests and I'm wondering how I should approach testing a static method that constructs our API endpoints based on what the properties are set to in our environment.ts file.

The environment.ts file changes with the environment, so I'm curious how I would accommodate for that in my test.

Does my method need to be refactored to make this easier to test? For example, instead of implicitly referencing environment.endpoint, instead pass-in environment as a argument? Would I mock the environment.ts file?

Any suggestions would be helpful.

import {environment} from '../../environments/environment';

export class Utilities {

  public static constructAPIEndpoint(): string {
    const hostname: string = environment.endpoint;
    const protocol: string = environment.local ? 'http:' : 'https:';

    return `${protocol}//${hostname}/`;
  }
}

environments.ts

export const environment = {
  production: false,
  local: false,
  hostname: 'localhost',
  showLogs: true,
  endpoint: 'foo-dev-graphapp.com',
  appInsights: {
    instrumentationKey: '123'
  }
};

jasmine test:

  import {environment} from '../../environments/environment';

  it('constructAPIEndpoint() it should construct an endpoint based on environment.ts', () => {
    const endpoint: string = Utilities.constructAPIEndpoint();

    /// THIS DOESN'T SEEM - having to recreate logic inside constructAPIEndpoint()
    const protocol: string = environment.local ? 'http:' : 'https';

        expect(endpoint).toEqual(`${protocol}//${environment.endpoint}`);

  });

If you really want to unit test that logic, you should fake its data. In order to do that, you need a way to inject that dependency instead of getting it straight from it.

You have 3 options:

Installing a framework to mock the import (as detailed in https://medium.com/@emandm/import-mocking-with-typescript-3224804bb614)
Have a constructor that is autocalled for its usage statically.
Similar to number 2: have a method with environment as parameter (so you can test it), and the current one calls the new one with the imported environment as parameter.


SPOJ problem: Life, the Universe, and Everything Your program is to use the brute-force approach in order to find the answer to Life, the Universe, and Everything. More precisely, rewrite small numbers from input to output. Stop processing input after reading in the number 42. All numbers at input are integers of one or two digits.

I was solving this problem and I submitted 3 solutions and I got "SEGMENTATION FAULT" for the first 2. The third one finally executed successfully, but I wanted to know where I made a mistake while solving.

1st I submitted:

#include <stdio.h>
  int main(void)
    {
      int i=0,j,a[9];

      while(a[i]!= 42)
       {
         ++i;
         scanf("%d",&a[i]);
       }
     for(j=0;j<i;j++)
     {
      printf("%d",a[j]);
       printf("\n");
      }
    return 0;
  }

2nd I submitted (after reading a comment that output has to be printed immediately after taking the input):

#include <stdio.h>
int main(void)
 {
  int i=0,j,a[9];

 scanf("%d",&a[i]);
 printf("%d\n",a[i]);

 while(a[i]!=42)
  {
   ++i;
   scanf("%d",&a[i]);
   if(a[i]!=42)
    printf("%d\n",a[i]);
  }
}

Third one (it was correct):

 #include <stdio.h>

  int main(void)
  {
   int i = 0;
   while (scanf("%d\n", &i) > 0 && i != 42)
   {
    printf("%d\n", i);
   }
    return 0;
   }

Can anyone point where my approach has gone wrong? When I executed the first 2 on www.ideone.com they both ran successfully over there but on SPOJ there was a runtime error (segmentation fault).

The working solution is quite good, actually. You check the return value of scanf(), and you use the while loop condition smartly to say exactly what you mean to accomplish.  The only thing wrong with it is the indentation.

Here is one reasonable way to format it.

#include <stdio.h>

int main(void)
{
    int i = 0;
    while (scanf("%d\n", &i) > 0 && i != 42)
    {
        printf("%d\n", i);
    }
    return 0;
}

The initialization of i to 0 and return 0 are actually not needed, but I don't have a problem with making them explicit.

As for your failed attempts, they fail because they attempt to store all of their input, which isn't necessary to do. When there are more than 9 inputs, the buffer overflows.


Convert number to words I wrote this converter code. It can convert a number from 1-100 into words. It works perfectly, but it seems to be too complicated to me.

Can you tell me whether it's okay or not?

#include <iostream>

int main()
{
    int number;
    int first_digit;
    int second_digit;
    std::cout << "Enter the number: ";
    std::cin >> number;
    first_digit = number/10;
    second_digit = number%10;
    if(number >= 11 && number <= 20)
    {
        switch(number)
        {
            case 11:
                std::cout<<"eleven";
                break;

            case 12:
                std::cout<<"twelve";
                break;
            case 13:
                std::cout<<"thirteen";
                break;
            case 14:
                std::cout<<"fourteen";
                break;
            case 15:
                std::cout<<"fifteen";
                break;
            case 16:
                std::cout<<"sixteen";
                break;
            case 17:
                std::cout<<"seventeen";
                break;
            case 18:
                std::cout<<"eighteen";
                break;
            case 19:
                std::cout<<"nineteen";
                break;
        }
    }
    else
    {
        switch(first_digit)
        {
            case 1:
                if(second_digit == 0)
                    std::cout<<"ten";
                break;
            case 2:
                std::cout<<"twenty";
                break;
            case 3:
                std::cout<< "thirty";
                break;
            case 4:
                std::cout<<"fourty";
                break;
            case 5:
                std::cout<<"fifty";
                break;
            case 6:
                std::cout<<"sixty";
                break;
            case 7:
                std::cout<<"seventy";
                break;
            case 8:
                std::cout <<"eighty";
                break;
            case 9:
                std::cout <<"ninety";
                break;
            case 10:
                std::cout <<"one hundred";
                break;
        }
        if(first_digit > 1 && number != 100) std::cout<<"-";
        switch(second_digit)
        {
            case 1:
                std::cout<<"one";
                break;
            case 2:
                std::cout<<"two";
                break;
            case 3:
                std::cout<<"three";
                break;
            case 4:
                std::cout<<"four";
                break;
            case 5:
                std::cout<<"five";
                break;
            case 6:
                std::cout<<"six";
                break;
            case 7:
                std::cout<<"seven";
                break;
            case 8:
                std::cout<<"eight";
                break;
            case 9:
                std::cout<<"nine";
                break;
            default:
                break;

        }
    }
    return 0;
}

I've never tackled such a problem before, so I'll proceed with this cautiously.

You don't need to declare those three variables and then assign to them.  Instead, initialize them right away where they're first used.

You can also move number right above the cin for slightly closer scope.

std::cout << "Enter the number: ";
int number;
std::cin >> number;
int first_digit = number/10;
int second_digit = number%10;

I suppose switch is an okay choice for this program.  Either way, the switch can still be condensed while making the program more modular (with functions).

Here's what 11-20 could look like within its own function:

std::string elevenThroughTwenty(unsigned int number)
{
    switch (number)
    {
        case 11: return "eleven";
        // ...
        case 20: return "twenty"; // you forgot the 20 in your code

        // throw an exception if number not in the switch
        // include <stdexcept> to use this
        default: throw std::logic_error("Not 11 through 20");
    }
}

Separating them as such helps with readability, organization, and conciseness.  You will also need proper defaults so that invalid numbers don't cause crippling errors.


DynamicList/LinkedList Queue Implementation here is my LinkedList Queue Implementation , seems like its almost like the linkedlist implementation with little differences mainly in the poll() method, i think that the offer() method is same like the linkedlist method add() , the point is that in the offer() method we are adding the elements to the tail, than we are moving them to the left, but their next is same like the add() method, where we are adding in the head and moving the elements to the right... Is i'm right? I'm glad to hear any suggestations...

import java.util.NoSuchElementException;

public class DynamicQueue {

private class Node {
    private Object item;
    private Node next;

    Node(Object item){
        this.item = item;
        this.next = null;
    }

    Node(Object item, Node prevNode){
        this.item = item;
        prevNode.next = this;
    }
}

private Node head;
private Node tail;
private int count;

public DynamicQueue() {
    this.head = null;
    this.tail = null;
    this.count = 0;
}

public int size() {
    return count;
}

public void offer(Object item) {
    if(tail == null) {
        tail = new Node(item);
        head = tail;
    }else {
        Node newNode = new Node(item,tail);
        tail = newNode;
    }
    count++;
}

public void clear() {
    head = null;
}

public Object peek() {
    if(head == null) {
        throw new NoSuchElementException("Overflow Exception");
    }else {
        return head.item;
    }
}

public void poll() {
    if(head == null) {
        throw new NoSuchElementException("Overflow Exception");
    }
    Node currentNode = head.next;
    head.next = null;
    head = currentNode;
    count--;

}

public static void main(String[] args) {
    DynamicQueue queue = new DynamicQueue();
    queue.offer("First");
    queue.offer("Second");
    queue.offer("Third");
    queue.offer("Fourth");
    queue.poll();
    System.out.println(queue.peek());
    queue.poll();
    System.out.println(queue.peek());
    queue.poll();
    System.out.println(queue.peek());

}

}

clear() is buggy

The following code causes a NoSuchElementException:

DynamicQueue queue = new DynamicQueue();
queue.offer("First");
queue.clear();
queue.offer("First");
queue.poll();

You can ealy fix it by adding tail = null; to your clear() method.

Use selfexplaing exception messages

An exception name (in this case NoSuchElementException) already describes the kind of error accurred. The cause should explain why this happened. A better message for exaple could simply be "this queue is empty".

Suggestion: add a return type to poll()

If poll() returns the current head, the user doesn't need to do call peek(). This wouldn't even change your main(String[]), since the user is not forced to use the returned object at all.

Of course, you could also add a generic type to your class instead of using Object inside your Node, but I guess that was not your intension, since you are already familiar with the Java Collections.


Stroke Algorithm in python Here is my next Numpy practice, I am not sure is it more accurate to call it "Stroke Algorithm" or "Draw edge stuff", xD.

Anyway I did in steps:

calculate each pixel's RGB absolute distance to pixels on its right and down side, the distance is simple measured by absolute different in each color value and sum them up.
if the absolute distance on both directions all larger than some particular value, then it is an edge pixel, and color it black, else white

Suggestions I am looking for

Any suggestions in Numpy, I have some duplicate code this time, and didn't find good way to remove them.

Full code:

# -*- coding: utf-8 -*-
import numpy as np
from PIL import Image

def stroke(image_path, output, level=80, edge_color=[255,255,255], blackground_color=[0,0,0]):
    img = Image.open(image_path)
    data = np.asarray(img, dtype="int32")

    w, h, k = data.shape
    dirright_data = np.concatenate((data[:, 1:], data[:,-1:]), axis=1)
    dirdown_data = np.concatenate((data[1:,:], data[-1:,:]), axis=0)

    disRight = np.absolute(np.sum(data - dirright_data, axis=2))
    disDown = np.absolute(np.sum(data - dirdown_data, axis=2))

    level = min(max(1,level), 255)
    D_right = np.asarray(disRight<=level, dtype="int32")
    D_down = np.asarray(disDown<=level, dtype="int32")

    D = (D_right+D_down) > 1
    neg_D = (D_right+D_down) <= 1
    data[D] = edge_color
    data[neg_D] = blackground_color

    img = Image.fromarray(np.asarray(np.clip(data, 0, 255), dtype="uint8"), "RGB")
    img.save(output)

if __name__ == "__main__":
    stroke("images/bob.png", "new_test.jpg")

I find your numpy code to be quite rational as far as my skills go, but your script have several things to improve on besides that.

Improvable in current scope :

Removing duplicate code thanks to a helper function
Painting the whole background to simplify
Removing (yet) unecessary line w, h, k = data.shape
Naming error, or logic error ? Background and edge color seem swapped. Changing <= level to > level
Some non-compliant and unexplicit variable names

Code showing this

# -*- coding: utf-8 -*-
import numpy as np
from PIL import Image

def get_dist_array(data, shifted_data, level):
    distance_map = np.absolute(np.sum(data - shifted_data, axis=2))
    return np.asarray(distance_map > level, dtype="int32")

def stroke(image_path, output, level=80, edge_color=[255,255,255], blackground_color=[0,0,0]):
    img = Image.open(image_path)
    data = np.asarray(img, dtype="int32")
    level = min(max(1,level), 255)
    edges_right = get_dist_array(data, np.concatenate((data[:, 1:], data[:,-1:]), axis=1), level)
    edges_down = get_dist_array(data, np.concatenate((data[1:,:], data[-1:,:]), axis=0), level)
    data[:] = blackground_color
    edges = (edges_right+edges_down) > 1
    data[edges] = edge_color
    img = Image.fromarray(np.asarray(np.clip(data, 0, 255), dtype="uint8"), "RGB")
    img.save(output)

if __name__ == "__main__":
    stroke("images/bob.png", "new_test.jpg")

Other things to consider worth improving :

Get rid of hardcoded file names. You could use argparse here
RGBA support ?


Custom UUID implementation on C++ The code's gonna be pretty simple and YES, I do know, what a proper UUID4 implementation is and what are possible approaches to implement RFC 4122.
But somewhere there exists a small piece of software, where possible dependencies should be minified if possible. Because of this and some other reason let's consider the following UUID generator:
#pragma once
#include <random>

static std::string generate_uuid(size_t len) {
  static const char x[] = "0123456789abcdef";

  std::string uuid;
  uuid.reserve(len);

  std::random_device rd;
  std::mt19937 gen(rd());

  std::uniform_int_distribution < > dis(0, sizeof(x) - 2);
  for (size_t i = 0; i < len; i++)
    uuid += x[dis(gen)];

  return uuid;
}

The output of the function is to be used in cases when some unique ID is required for a custom storage. Like "here we put this data, and there is its 16-byte ID", meaning it would be enough if collisions are just "not likely at all, like winning a lottery twice in a row".
As far as modern C++ has improved random utils, it seemed a not-so-bad custom solution, but now I'm not that sure.
What would you say? Thanks in advance

You forgot to include <string>, and misspelt std::size_t (twice).
Since this doesn't create a RFC-4122-compliant UUID, I would change the name so it doesn't get used where it shouldn't.  Perhaps create_random_id() or similar?
I don't like the name x for the digit string - I'd prefer something more descriptive like hex_chars or even just hex.

We could consider constructing a string that's already long enough, then setting all its characters:
std::string uuid(len, '\0');
for (auto& c: uuid) {
    c = x[dis(gen)];
}

Or perhaps replace the loop with a standard algorithm:
std::string uuid;
uuid.reserve(len);
std::generate_n(std::back_inserter(uuid), len,
                [&]{ return x[dis(gen)]; });

Though I don't think that's necessarily clearer.

Going a little over the top, we could make a general select-from-collection function and use that:
template<std::ranges::sized_range C, class G>
auto sample_from(C const& collection, G&& generator)
{
    std::uniform_int_distribution<typename C::size_type> dis{0, std::ranges::size(collection) - 1};
    return collection[dis(generator)];
}

⋮
    static const std::string_view hex_chars = "0123456789abcdef";
    std::generate_n(std::back_inserter(uuid), len,
                    [&]{ return sample_from(hex_chars, gen); });

We might even make a custom input-iterator for selecting random characters:
class random_char_source
{
    const std::string chars;
    std::mt19937 generator;
    std::uniform_int_distribution<std::size_t> distribution;

public:
    using iterator_category = std::input_iterator_tag;
    using value_type = char;
    using difference_type = int;
    using pointer = const char*;
    using reference = const char&;

    explicit random_char_source(std::string chars, std::random_device::result_type seed = std::random_device{}())
        : chars{std::move(chars)},
          generator{seed},
          distribution{0, this->chars.size() - 1}
    {}

    // it's an input iterator
    char operator*() { return chars[distribution(generator)]; }
    random_char_source& operator++() { return *this; }
};

That makes our ID generator very simple:
static std::string generate_random_id(std::size_t len)
{
    auto hex_chars = random_char_source{"0123456789abcdef"};

    std::string uuid;
    uuid.reserve(len);
    std::copy_n(hex_chars, len, std::back_inserter(uuid));
    return uuid;
}

However, for generating power-of-two numbers, and since we have a UniformRandomBitGenerator (the std::mt19937 object), we can extract bits directly:
static std::string random_id(std::size_t len)
{
    static const std::string_view hex_chars = "0123456789abcdef";
    using Generator = std::mt19937;

    Generator gen{std::random_device{}()};

    std::string uuid;
    uuid.reserve(len);

    while (uuid.size() < len) {
        auto n = gen();
        for (auto i = Generator::max();  i & 0x8 && uuid.size() < len;  i >>= 4) {
            uuid += hex_chars[n & 0xf];
            n >>= 4;
        }
    }

    return uuid;
}

You could probably make a simpler and more readable version of that.


Concatenate unlimited amount of arrays with predefined function I am working on learning more about arrays and loops! I have been trying to create a method which will allow me to take for example an unlimited amount of arrays and append or concatenate them to each other!

The method takes multiple arrays as arguments and creates a merged array in the order in which the arrays where passed through the parameter. My question is:

Is there a more efficient way to do this which I am maybe not aware of?

I have done my research but most ways usually use a set amount of arrays in order to achieve this. This is just something I am doing for the sake of learning, and these functions may not be useful in real life scenarios.

public static Object[] merge(Object[]... arrays) {
    int[] lenghts = new int[arrays.length];
    int wholeLenght = 0;
    int maxLenght = 0;

    for(int i = 0; i<arrays.length; i++){
        lenghts[i] = arrays[i].length;
    }
    for(int i = 0; i<lenghts.length; i++){
        wholeLenght += lenghts[i];
    }
    Object[] merged  = new Object[wholeLenght];

    for(int i = 0; i<arrays.length; i++){
        System.arraycopy(arrays[i], 0, merged, maxLenght, lenghts[i]);
        maxLenght+=lenghts[i];
    }
    return merged;
}

Version which allows generic types. A similar function can be easily made for
primitive a primitive type array of choice.

@SuppressWarnings("unchecked")
public static <T> T[] mergeGenerics(T[]... arrays) {
    int[] lenghts = new int[arrays.length];
    boolean allowOperation = true;
    int wholeLenght = 0;
    int maxLenght = 0;
    T[] merged = null;
    for (int i = 0; i < arrays.length; i++) {
        if (arrays[0].getClass().getComponentType() != arrays[i].getClass().getComponentType()) {
            allowOperation = false;
            System.err.println("The arrays are not all of the same type!!");
        }
    }
    if (allowOperation) {
        for (int i = 0; i < arrays.length; i++) {
            lenghts[i] = arrays[i].length;
        }
        for (int i = 0; i < lenghts.length; i++) {
            wholeLenght += lenghts[i];
        }
        merged = (T[]) Array.newInstance(arrays[0].getClass().getComponentType(), wholeLenght);

        for (int i = 0; i < arrays.length; i++) {
            System.arraycopy(arrays[i], 0, merged, maxLenght, lenghts[i]);
            maxLenght += lenghts[i];
        }
    }
    return merged;
}

Test The method maybe with:

        Object[] obj1 ={"A","B"};
        Object[] obj2 ={"C","D","E","F"};
        Object[] obj3 ={"G","H","I","J","K"};
        Object[] obj4 ={"L","M","N"};
        String[] obj5 ={"O","P","Q","R"};
        Object[] obj6 ={"S"};
        Object[] merge = Class.merge(obj1,obj2,obj3,obj4,obj5,obj6);

        for(int i = 0; i<merge.length; i++)
            System.out.print(merge[i]+" ");

The approach suggested by @Caridorc has the limitation that it requires the objects passed to be Iterables, which an array is not a subtype of due to certain limitations. Therefore, you either have to change your data type from an array to some Iterable extension or use a different approach.

A simple, naive solution would be to simple convert arrays to Lists and add all to a combined list like this:

private static <T> T[] merge2(T[] ... arrays) {
    final List<T> list = new ArrayList<>();
    for (T[] array : arrays) {
      list.addAll(Arrays.asList(array));
    }
    return list.toArray(arrays[0]);
}

which has certain runtime penalties as you create multiple new objects which furthermore on adding new lists via addAll(...) will increase the capacity of the list by 50% of its current length on exceeding the current size and therefore contain some unused space.

A further approach is based on your generic approach but contains only two loops. The first one to learn the total length, the second one to copy the data from the source array to the target array:

private static <T> T[] merge3(T[] ... arrays) {
    Class<?> componentType = arrays[0].getClass().getComponentType();
    int size = 0;
    for (T[] array : arrays) {
      size += array.length;
    }
    T[] result = (T[])arrays[0].getClass().cast(Array.newInstance(componentType, size));
    int pos = 0;
    for (T[] array : arrays) {
      System.arraycopy(array, 0, result, pos, array.length);
      pos += array.length;
    }
    return result;
}

This method is shorter and IMO easier to read then your implementation. A short comparison between the 4 algorithms reveals also, that it is faster then the other ones:

Original - Total: 51.402688 ms, Average: 5.1402688E-4 ms
Original Generics - Total: 42.150794 ms, Average: 4.2150794E-4 ms
Merge2 - Total: 87.06185 ms, Average: 8.706185E-4 ms
Merge3 - Total: 28.701731 ms, Average: 2.8701731E-4 ms

Note however, that I did a naive performance test and didn't take care of compiler optimizations or warm-up phases. The comparison therefore may be flawed and data messy.

Also note that I did not take care of checking if all of the passed arrays are of the same type. Mingling multiple arrays of different types might only work if the first passed array is of type object (or at least a supertype of any further arrays). In general, I wouldn't allow to pass arrays of different types though.

BTW: the double cast in merge3 is to get rid of Unchecked cast warnings


Simplify data packet assemble in C Below code just for assemble data into network packet. but the code logic has many repeat parts to convert integer for byte array. what's the best method for such purpose to simplify below logic?

typedef struct _INFO {
    int pid;
    int score;
} INFO;

void send_packet(int connfd,INFO* info,int len) {
    int buflen = len*8 + 8;
    unsigned char* buf = (int*)malloc(buflen);
    int p = 0;
    buf[p++] = 0x55;
    buf[p++] = 0x55;
    buf[p++] = 0x55;
    buf[p++] = 0x55;

    buf[p++] = buflen & 0xFF;
    buf[p++] = (buflen >> 8) & 0xFF;
    buf[p++] = (buflen >> 16) & 0xFF;
    buf[p++] = (buflen >> 24) & 0xFF;
    for (int i=0;i<len;i++) {
        buf[p++] = info[i].pid & 0xFF;
        buf[p++] = (info[i].pid >> 8) & 0xFF;
        buf[p++] = (info[i].pid >> 16) & 0xFF;
        buf[p++] = (info[i].pid >> 24) & 0xFF;

        buf[p++] = info[i].score & 0xFF;
        buf[p++] = (info[i].score >> 8) & 0xFF;
        buf[p++] = (info[i].score >> 16) & 0xFF;
        buf[p++] = (info[i].score >> 24) & 0xFF;
    }
    write(connfd, buf, buflen);
    free(buf);
}

what's the best method for such purpose to simplify below logic?

With a helper function as there is  repetitive code.

#include <stdint.h>

// Write 32-bit integer in little endian order to buf
unsigned char *u32tobuf(unsigned char *buf, uint32_t value) {
  buf[0] = value;
  buf[1] = value >> 8;
  buf[2] = value >> 16;
  buf[3] = value >> 24;
  return buf + 4;
}

I'd also 1) add some error checking, 2) drop the unneeded cast 3) prevent int overflow 4) use const.

// Return error status
int send_packet(int connfd, const INFO* info, int len) {
    uint32_t buflen = len*8lu + 8;
    unsigned char* buf = malloc(buflen);
    if (buf == NULL) {
      return 1; // or perhaps some enum
    }
    unsigned char* p = u32tobuf(buf, 0x55555555);
    p = u32tobuf(p, buflen);
    for (int i=0; i<len; i++) {
      p = u32tobuf(p, info[i].pid);
      p = u32tobuf(p, info[i].score);
    }
    ssize_t write_count = write(connfd, buf, buflen);
    free(buf);
    if (write_count != buflen) [
      return 1; // or perhaps some enum
    }
    return 0;
}


Task with feedback dialog helper class I'm currently working in a Windows.Forms project where its very common that I need to show a responsive feedback form while a task runs. These tasks take typically around 5 to 10 seconds to complete and the user can't do anything else until the task has successfully finished or failed.

To keep the UI responsive I've cooked up a little helper class. I'm pretty new to tasks and the async programming environment so I am not sure if I'm overkilling things here or going off in a completely wrong direction:

This is the small helper class:

internal sealed class TaskManagerWithFeedbackDialog<TController> where TController : IViewController, new()
{
    public static Task<TReturn> RunTask<TReturn>(Func<TReturn> func)
    {
        if (func == null)
        {
            throw new ArgumentNullException("func");
        }

        var manager = new _AsyncTaskManager<TReturn>(func);

        using (var view = new TController())
        {
            var task = manager.Start(view);
            view.ShowViewDialog();
            return task;
        }
    }

    private class _AsyncTaskManager<T>
    {
        private readonly Func<T> taskFunc;

        public _AsyncTaskManager(Func<T> func)
        {
            taskFunc = func;
        }

        public Task<T> Start(IViewController feedBackView)
        {
            var task = Task.Factory.StartNew(taskFunc);

            task.ContinueWith(t =>
            {
                feedBackView.CloseView();
            }, TaskContinuationOptions.OnlyOnFaulted);

            task.ContinueWith(t =>
            {
                feedBackView.CloseView();
                return t.Result;
            }, TaskContinuationOptions.NotOnFaulted);

            return task;
        }
    }
}

Where the implementation of IViewController.CloseView() would look like this:

public virtual void CloseView()
{
    if (View.InvokeRequired)
    {
        View.Invoke(new Action(View.Close));
    }
    else
    {
        View.Close();
    }
}

Another issue I am not sure about is where is the best place to handle ArregateException?. Right now I don't handle anything as I don't need to clean up anything if a task fails. I simply check the returned task for any errors. Is this the right thing to do? Do async tasks always fail 'silently'? If I needed to clean up, where would it be? Inside .ContinueWith( .... , TaskContinuationOptions.OnlyOnFaulted)?

Few minor things:

If TaskManagerWithFeedbackDialog is not a nested class then internal is redundant and can be dropped.
TaskManagerWithFeedbackDialog has - so far - just one single static method. Until you will add more methods make it a static class. No need to create any instance.
TController instance is disposed just after task started (see it's wrapped in using). When you will invoke CloseView() you will call it on an already disposed object. Is it just a factory? Call it *Factory instead of *Controller. Is it a controller? Don't dispose it immediately but when you close the view (and check why your Dispose() method does nothing).
Why RunTask() instead of StartNew() or - better - Run() as Task and TaskFactory methods? I'd favor consistency with framework classes.
Is there a specific reason you're using Task.Factory.StartNew() instead of Task.Run()? EDIT: OK, you're with .NET 4.0.
_AsyncTaskManager (which BTW shouldn't be prefixed, it's optional for private fields and to avoid for types) is superfluous. It's just one method, it has no state and it's not persisted or passed around. You don't need a class but a simple private method.
Task has a fluid interface, you can concatenate method calls return Task.Run(...).ContinueWith(...).ContinueWith(...).
In CloseView() you wait for dispatched action to terminate but you don't actually need to wait for UI to push thread back to the pool, in WinForms you may simply use BeginInvoke() because it doesn't need EndInvoke() to be called.

Ideally you wouldn't even need continuation tasks:

Task.Factory.StartNew(() => {
    try
    {
        return taskFunc();
    }
    finally
    {
        feedBackView.CloseView();
    }
});

About exception management: you're doing right, if it's the way you want to handle errors. You close your view but AggregateException goes unnoticed. Here you may want to log such errors or to propagate them to UI thread (for example) again using BeginInvoke. See MSDN for more details.


List abstraction using array Following this reference, below is the list abstraction implementation using array,

/* list.h */

/*
   List is an ordered collection(sequence) of homogeneous type elements(unique or duplicate).
   By definition, List is not suppose to have collection of heterogeneous type elements.
   All elements in a List are related.
   List is mutable
   Each element has a position.
   If an element is deleted, then still the remaining elements sit in new order.

   Array implementation of List

*/

#include<stddef.h>
#include<stdlib.h>
#include<string.h>

#define INITIAL_LIST_SIZE 50

#define NEW_LIST 0
#define DOUBLE_THE_LIST 1
#define HALF_THE_LIST 2

typedef struct List{
  int *array;
  int lastItemPosition;
  int size;
}List;

extern List *arrayList;

void insertItem(int newItem, int location);
List* createList(List *, int);

/* arrayImpl.c */
#include"list.h"

List *createList(List *list, int flag){

  List *lptr = (List *)malloc(sizeof(List));

  if(flag == NEW_LIST){
    lptr->array = malloc(INITIAL_LIST_SIZE*sizeof(int));
    lptr->array = memset(lptr->array, -1, INITIAL_LIST_SIZE*sizeof(int));
    lptr->lastItemPosition = -1;
    lptr->size = INITIAL_LIST_SIZE;
  }else if(flag == DOUBLE_THE_LIST){
    lptr->array = malloc(2*(list->size)*sizeof(int));
    lptr->array = memcpy(lptr->array, list->array, list->size*sizeof(int));
    lptr->lastItemPosition = list->lastItemPosition;;
    lptr->size = 2*(list->size);
  }else if(flag == HALF_THE_LIST){
    lptr->array = malloc(((list->size)/2)*sizeof(int));
    lptr->array = memcpy(lptr->array, list->array, (list->size/2)*sizeof(int));
    lptr->lastItemPosition = list->lastItemPosition;;
    lptr->size = (list->size)/2;
  }
  return lptr;

}

void insertItem(int newItem, int index){

  /* House keeping */
  if(arrayList->lastItemPosition + 1 == arrayList->size){
    arrayList = createList(arrayList, DOUBLE_THE_LIST);
  }

  /* Insert the element */
  arrayList->array[index] = newItem;
  if(index > arrayList->lastItemPosition){
     arrayList->lastItemPosition = index;
  }

  return;
}

void deleteItem(int index){

  arrayList->array[index] = -1;
  if(index == arrayList->lastItemPosition){
    arrayList->lastItemPosition--;
  }

  /* House keeping */
  if(arrayList->size > INITIAL_LIST_SIZE){
    if(arrayList->lastItemPosition == ((arrayList->size)/2)){
      arrayList = createList(arrayList, HALF_THE_LIST);
      arrayList = createList(arrayList, HALF_THE_LIST);
    }
  }
}

/* main .c */
#include"list.h"

List *arrayList = NULL;

int main(void){
  arrayList = createList((List *)NULL, NEW_LIST);

}

Is  Listfollowing abstraction and encapsulation?

Note: Encapsulation is about maintaining in-variants of ADT, after performing operations on ADT. Abstraction is a barrier between representation and usage.

This code is very easy to read, and very straightforward in its implementation. Nice work! Overall, it looks really good. I would say that it does follow abstraction and encapsulation fairly well. Below are some things that I think could be improved.
Functions Should Do 1 Thing
Your createList() function does 3 possible things:

Creates a new empty list with a set capacity
Creates a new list with double the capacity of an existing list, then copies the existing list to it
Creates a new list with half the capacity of an existing list, then copies the existing list to it

That's a little confusing. Also the name flag for the second parameter is not very helpful. If I were to keep the function as-is, I'd rename it to operation, or something like that to make it more clear. Usually a flag is a Boolean value of some sort. (It may be an int but is usually on or off.)
So I'd break createList() into 2 or 3 functions like this:
List* createList();
size_t setListSize(List* ptr, const size_t newSize);

Or you could break the second one into 2 functions, like increaseSize() and decreaseSize(), but I don't think that's necessary.
The new createList() function would just do what's inside of the if (flag == NEW_LIST) portion of your original function. For setListSize(), I'd pass in the new desired size. You can check inside the function if it's greater or less than the current size and do the appropriate thing. And the return value should be the size it was set to, which leads to my next point…
Handle Errors
None of your functions handle error cases. What if you attempt to increase the size of the list and you're out of memory? What if you want to reduce the size of the list, but can't allocate the smaller-sized array before doing the copy? With your current createList() function, it will return NULL, but none of the callers check that return value, so they could crash. Even worse, you allocate a List on the first line, then assume it worked and allocate the array. Then you assume that worked and start setting or copying to it. If any one of those steps fails, the next one will probably crash it.
If you return the size from setListSize(), the caller can check to make sure it worked and react appropriately if it didn't.
Modify the Existing List
Since your InsertItem() and DeleteItem() functions modify your list, the setListSize() function should too. Rather than create a new list, it would create the new array (either larger or smaller than the current one), do the copy, then free the existing one and replace it with the new one.