Counting rows in a CSV file that correspond to a database row, each with a million records I have two DataTables:

dt: is populated from a CSV file with over 1.7 million rows
dataStructure.Tables["AccountData"]: is populated from a database query also roughly a million rows

I use the following code to iterate through and compare the data from each set of rows. The code takes over 48 hours to complete. I have changed the properties of the application to x64 to allow it to use more Process Memory. It now uses roughly 2.5GB.

My question is, is there a more efficient way of doing this that would decrease run time?

//set is_legal column value for each row
foreach (DataRow row in dt.Rows)
{
    var acctNum = row[0].ToString().Replace("\"", "");
    foreach (DataRow queryRow in dataStructure.Tables["AccountData"].Rows)
    {
        var queryAcctNum = queryRow[0].ToString();
        if (acctNum.Equals(queryAcctNum))
        {
            row[12] = "Y";
            Console.WriteLine("Yes count: " + cnt);
        }
        else
        {
            row[12] = "N";
        }
    }
    cnt++;
};

How dataStructure.Tables["AccountData"] is being populated:

//Read each row from the table and output the results into the data set
while (readFile.Read())
{
    //Create a row to hold data
    DataRow datarow = dataStructure.Tables["AccountData"].NewRow();

    datarow["AccountNumber"] = readFile.GetString(0).Trim();
    datarow["LegalStatus"] = readFile.GetString(1);

    //add the row to the data table AccountData
    dataStructure.Tables["AccountData"].Rows.Add(datarow);
}

Your inner loop appears to be unnecessary. Why not create a lookup:

var knownAccountNumbers = new HashSet<string>(
    dataStructure.Tables["AccountData"].Rows
        .Cast<DataRow>()
        .Select(row => row[0].ToString()));

Now your loop is simply:

foreach (DataRow row in dt.Rows)
{
    var accountNumber = row[0].ToString().Replace("\"", "");
    row[12] = knownAccountNumbers.Contains(accountNumber) ? "Y" : "N";
}

I think I remember reading once that the memory usage of a HashSet is 12 bytes per entry + size of the entry. So you're looking at 12MB + 1,000,000 * (2 * accountNumber.Length). So basically nothing in the grand scheme of things. However, you are gaining constant time lookups which should be a huge benefit to this kind of work.

You should take more care when naming things. Don't abbreviate e.g. acctNum -> accountNumber.


NodeJS script to dump data from Redis to Elasticsearch I have about a million records in Redis which I want to dump into Elasticsearch periodically. I just want to make sure that my script is decent enough in terms of speed and no memory leaks.

'use strict';

const redis = require('redis');
const bluebird = require('bluebird');
const request = require('request');
const elasticsearch = require('elasticsearch');
const fs = require('fs');
const _ = require('lodash');
const async = require('async');
const sh = require('shorthash');
const sleep = require('sleep');
const config = require('../config');

bluebird.promisifyAll(redis.RedisClient.prototype);
bluebird.promisifyAll(redis.Multi.prototype);

let client = redis.createClient({
  host: config.redis.url,
  port: config.redis.port
});

let ES = elasticsearch.Client({
  host: config.elasticsearch.url,
  requestTimeout: 30000000
});

var keys = fs.readFileSync('no-keys').toString().split('\n');
keys = keys.filter((e) => e);

let chunkedKeys = _.chunk(keys, 1000);
console.log('We have ' + chunkedKeys.length + ' keys');

_.each(chunkedKeys, (chunkedKey) => {
  client.mget(chunkedKey, (mgetError, replies) => {
    if (mgetError) {
      console.error(mgetError);
    }
    console.log('MGET complete from Redis');
    console.log('We have ' + replies.length + ' documents');
    async.mapLimit(replies, 5, (reply, callback) => {
      try {
        let content = JSON.parse(reply);
        let k = sh.unique(content.url);

        let body = [{index: {_index: config.elasticsearch.index, _type: 'article', _id: k, _timestamp: (new Date()).toISOString() }}];

        body.push(content);
        callback(null, body);
      } catch(e) {
        console.error(e);
        callback(e, []);
      }
    }, (err, results) => {
      if(err) {
        console.log(err);
      }
      let mergedResult = _.flatten(results.filter((e) => e));

      console.log('Export complete with ' + mergedResult.length);

      ES.bulk({body: mergedResult}, () => {
        console.log('Import complete');
      });
    });
  });
});

I can see two problems with your script:

You are doing sync stuff in async. It is not recommended. Async is recommended for asynchronous operations. You are using it to do synchronous stuff - in the async.mapLimit block. Neither JSON.parse nor sh.unique is asynchronous. It is critical that you under the difference. Please read async docs on synchronous operations
You are buffering all your keys into memory. I guess that is a smaller problem, but it is less efficient than using a readStream.

Please take a look at my implementation and feel free to use any parts of it

'use strict';

const redis = require('redis');
const bluebird = require('bluebird');
const request = require('request');
const elasticsearch = require('elasticsearch');
const fs = require('fs');
const _ = require('lodash');
const async = require('async');
const sh = require('shorthash');
const sleep = require('sleep');
const config = require('../config');
const readline = require('readline'); //handy wrapper for readStream

bluebird.promisifyAll(redis.RedisClient.prototype);
bluebird.promisifyAll(redis.Multi.prototype);

let client = redis.createClient({
    host: config.redis.url,
    port: config.redis.port
});

let ES = elasticsearch.Client({
    host: config.elasticsearch.url,
    requestTimeout: 30000000
});

const readLineStream = readline.createInterface({ //this stream will serve keys one per line
    input: fs.createReadStream('no-keys')
});

let keysBuffer = [];
readLineStream.on('line', (line) => { // you were splitting with '\n' I guess your input is key per-line
    if (line) { //check for empty lines
        keysBuffer.push(line);
        if (keysBuffer.length === 1000) {
            migrateKeys(keysBuffer);
            keysBuffer = [];
        }
    }
});

readLineStream.on('end', ()=>{
    if(keysBuffer.length >0){
        migrateKeys(keysBuffer); // remember to flush your local buffer
    }
});

function migrateKeys(chunkOfKeys) {
    client.mget(chunkOfKeys, (mgetError, replies) => {
        if (mgetError) {
            console.error(mgetError); // Consider returning early
        }
        console.log('MGET complete from Redis');
        console.log('We have ' + replies.length + ' documents');

        let parsedReplies = [];
        replies.forEach((reply)=> {
            try {
                let content = JSON.parse(reply);
                parsedReplies.push([{
                    index: {
                        _index: config.elasticsearch.index,
                        _type: 'article',
                        _id: sh.unique(content.url),
                        _timestamp: (new Date()).toISOString()
                    }
                }, content]); // no need to filter out replies with parse errors
            } catch (e) {
                console.error(e);
            }
        }); //using async for sync code is not recommended

        console.log('Export complete with ' + parsedReplies.length);

        ES.bulk({body: parsedReplies}, () => {
            console.log('Import complete');
        });
    });
}


Recursive LinkedList. Which implementation is better and why? class Node:

    def __init__(self, data):
        self.data = data
        self.next = None

class LinkedList:

  def __init__(self):
      self.head = None

def insert(self, data):
    self.head = self.insertHelper(self.head, data)

def insertHelper(self, head, data):
    if(head == None):
        return Node(data)
    else:
        head.next = self.insertHelper(head.next, data)
    return head

  or

def insertR(self, data):
    self.insertRHelper(self.head, data)

def insertRHelper(self, head, data):
        if(head == None):
            self.head = Node(data)
        elif(head.next == None):
            head.next = Node(data)
        else:
            self.insertRHelper(head.next, data)

Any reason why one of these implementations would be better? Personally think the insertR version is easier to look at.

Both of these implementations are bad.

The first is probably worse.

every node in the linked list, from self.head to the last node's head.next is unconditionally written to, which makes every cache-line/cache-page/virtual-memory-page the linked list touches "dirty" and need to be flushed/written.
self.head is passed to insertHelper(), which recursively calls itself N times, eventually (assuming N > 1) returning the value that it was given self.head, to be stored back into the self.head member.  This is a LONG critical section.  If any threading support was added, locks would need to be held way longer than necessary.

Recursion

Both use recursion, and Python does not do tail-call-optimization.  This means that if your linked-list is very long, you will get a stack overflow.  There are manual ways to make your function behave in a tail-call-optimized fashion, even if Python won't do it for you, but they are complex and it is by far simpler to just use a loop.

PEP-8

Both insertHelper and insertRHelper are helper functions, which should not be exposed a public methods.  They should begin with a leading underscore to indicate they are not part of the public interface.
The members next, head, and probably data should also not be public, so should be named with a leading underscore.
Classes are named using BumpyWords; functions, members and variables should all be snake_case.  These means insertHelper should be named _insert_helper, and so on.
if(...): is not Pythonic.  The parenthesis are unnecessary.

Tail

The best solution would be to maintain a ._tail member in the LinkedList:

class LinkedList:
    """Description of class here"""

    def __init__(self):
        self._head = None
        self._tail = None

    def insert(self, data: Node) -> None:
        """Description of the insert function here"""

        if self._tail:
            self._tail._next = Node(data)
            self._tail = self._tail._next
        else:
            self._head = self._tail = Node(data)

Now, insertions at the tail of the LinkedList are \$O(1)\$.

Naming

As mentioned by Matthieu M. in the comments:

  I would also consider renaming here: insert should be append, to make it clear where the insert occurs, and offer a path to proposing different insertions such as prepend, or insert with an index.


Search Array of Strings in Array of String, receive Int-Array of indexes I'm looking for an lib which can search an array of strings with multible search-strings (array). I want to get an int-array of all indexes/matches.

private int[] getIndexArray(String[] pArray, String[] pSearchStrings) {
    int[] indexes = new int[0];
    for (String str : pArray) {
        if (ArrayUtils.contains(pSearchStrings, str))
            indexes = ArrayUtils.add(indexes, ArrayUtils.indexOf(pArray, str));
    }
    return indexes;
}

Is there a better way of doing it?

If you use pSearchStrings[] for all pArray[], you can transform pSearchStrings in a Pattern and use matcher method to avoid to load each time ArrayUtils.contains(..)

EDIT

private static int[] getIndexArray(String[] pArray, String[] pSearchStrings) {
    StringBuilder sb = new StringBuilder((int) estimateSizeOfSearchStrings +16);
    for (String searh : pSearchStrings) {
         sb.append(".*");
         sb.append(search)
         sb.append(".*|");
    }
    Pattern patt = Pattern.compile(sb.toString().subString(0,sb.length() -1);
    // If pSearchStrings are always the same, you can put static 'patt' outside this method
    // Or pass pre-compiled 'patt' as a parameter.
    int [] indexes = new int [pSearchStrings.length];
    int i = 0;
    for (String str : pArray) {
            if (patt.matcher(str).find())
            indexes[i++] = ArrayUtils.indexOf(pArray, str);
    }
    return indexes;
}


Knight moves in a 8*8 chess board The problem that I have is to find the minimum number of ways to go from one box to another box of a chess board. Now, I have created a code which is not very elegant but works. I am interested in if the code will give me the correct answer. Here is the code:

import java.util.*;
class Main{
  public static void main(String []sad){
    String str[] = new String[]{"2 1", "1 2","-1 -2","-1 2","1 -2","2 -1","-2 1","-2 -1"};
    String start = "8,8";
    Set<String> set = new HashSet<String>();
    String goal = "1,1";
    int s0 = Integer.parseInt(start.split(",")[0]);
      int s1 = Integer.parseInt(start.split(",")[1]);
      int s2 = Integer.parseInt(goal.split(",")[0]);
      int s3 = Integer.parseInt(goal.split(",")[1]);
    int dist = 0;
    Stack<String> stk = new Stack<String>();
    stk.push(start);
    set.add(start);
    while(!stk.empty()){
      dist++;
      String ele = stk.pop();
      for(int i=0;i<str.length;i++){
        int d0 = Integer.parseInt(ele.split(",")[0]) + Integer.parseInt(str[i].split(" ")[0]);
        int d1 = Integer.parseInt(ele.split(",")[1]) + Integer.parseInt(str[i].split(" ")[1]);
        if(d0<1 || d0>8 || d1<1 || d1>8)
          System.out.println("No Pushing allowed");
        else{
        String sr = d0+","+d1;
        System.out.println(sr);
        if(sr.equals(goal)){
          System.out.println("YES "+dist);
          return;}
          if(!set.contains(sr))
          {
            set.add(sr);
            stk.push(sr);
          }
      }
      }
    }
  }
}

Thanks.

First of all, you're using a wrong algorithm. A depth-first finds some path, but not necessarily the shortest one. What you actually need is a breadth-first search. You can just use a queue instead of a stack to fix it.

Storing everything as a string is bad idea. You code'll be much better if you use appropriate data types. The position on the board or a shift vector for one move is not string. I'd suggest to create a separate Position class that holds a pair of integers to represent a position on the board and has a shiftBy and an isValid method (to get the next position after one move and to check if a position is valid).

I'd also recommend to separate the code that reads the data and prints the result from the part that actually runs the computations.

Fixing the indentation and spacing of code according to the Java coding convention would make it more readable.

The names of the variables are also not very descriptive. For instance, what does ele mean? What is str (which is, in fact, not really a string)? I have no clue. It should be something like currentPosition and shiftVector because that's what it actually is.


Project Euler Problem #4 - Palindromic number The problem is as follows:

  A palindromic number reads the same both ways. The largest palindrome made from the product of two 2-digit numbers is 9009 = 91 × 99. Find the largest palindrome made from the product of two 3-digit numbers.

Using Project Euler to stretch my C# learning, I solved problem #4 with the code below. I ran this in a console app to get the answer. How can I improve my code?

class PalindromNumber
{
    public string GetPalindromeNumber(int maxNumber = 999)
    {
        bool breakOut = false;
        int test=0;
        int left = 0;
        int right = 0;
        int biggestNumber = 0;
        string returnString=string.Empty;
        for (left=maxNumber; left >= 0; left--)
        {
            for(right=maxNumber; right >= 0; right--)
            {
                test = left * right;

                string testNumberAsString = Convert.ToString(test);
                string reverse = string.Empty;
                for (int index = testNumberAsString.Length; index > 0; index--)
                {
                    reverse += testNumberAsString[index-1];
                }

                breakOut = (testNumberAsString == reverse && Convert.ToString(left).Length == 3 && Convert.ToString(right).Length == 3);

                if (breakOut )
                {
                    break;
                }
            }

            if (test>biggestNumber)
            {
                biggestNumber = test;
                returnString = $"Palindrome: {test}, Left: {left}, Right: {right}";
            }
        }

        return returnString;
    }
}

The other answers are all giving you different ways to reorganize your loops. They're all not bad in their own way, but the better solution in C# is to not write any loops in the first place.

Your program could be written like this:

static void Main()
{
    var query = from first in ThreeDigitNumbers()
                from second in ThreeDigitNumbers()
                let product = first * second
                where IsPalindromicNumber(product)
                select product;
    Console.WriteLine(query.Max());
}

Notice how every part of this program reads like the specification of the problem. The problem is "give the maximum of the palindromic numbers that are formed as the product of two three-digit numbers", and clearly that's what this program does.

Writing helper methods IsPalindromicNumber and ThreeDigitNumbers is left as an exercise.

Now, once that program is written and correct, then is the time to start asking questions about whether it is sufficiently fast, and so on. We could notice for instance that we check both 113 x 115 and 115 x 113, even though they obviously have the same result.  So we could then modify the query to

from second in ThreeDigitNumbersGreaterThan(first)

and then write that helper method. And so on.

(Exercise: Notice how introducing an optimization can introduce a bug if you're not careful. What bug did I just suggest that you write?)

But the takeaway here is write your program so that it looks like the specification. The specification does not say "write a bunch of loops". It says "find the maximum..." so there should be a call to Max in there somewhere. Write small methods that each do one thing well, and do exactly what they say on the tin.


Storing and Retrieving data with User Forms in Excel 2010 I'm currently designing a check list for my office using Excel that requires the user to save anywhere from 4-10 documents that need to be saved in the same folder as this check list. I thought it might be easier to consolidate everything into one Excel file by using User Forms. I noticed that it's not safe to use a User Form to store data, but instead using it as a means to display or manipulate data or cells from within the spreadsheet.

Unfortunately, I cannot store all the raw data from these files into a single cell as they can be quite large files. I thought maybe it'd be best to have an ActiveX Textbox to store this data into instead, but I'm wondering if I this might cause issues later down the road or if some one else's computer isn't compatible.

Visually what I've done...

This is the secondary spreadsheet (will be hidden) that stores the data in ActiveX Text Boxes

User form below that is displaying that data, stored from the secondary spreadsheet in the ActiveX Text Boxes

TL;DR

Everything is working as intended, but my primary question here is... Is this the best and safest (safest as in, lowest chance of losing data... I am not worried about security as everything is stored on a server) way to have all this data being stored in a single document by using embedded TextBoxes? If not, what would be a good alternative?

Code run when the User Form is closed

Private Sub btnSaveAndClose_Click()

    Sheets("Data").TextBox1.Value = PO850Data.Value
    Sheets("Data").TextBox2.Value = Invoice810Data.Value
    frmEDIData.Hide

End Sub

Code run when the User Form is opened

Private Sub UserForm_Initialize()

    btnSaveAndClose.SetFocus
    PO850Data.Value = Sheets("Data").TextBox1.Value
    Invoice810Data.Value = Sheets("Data").TextBox2.Value

End Sub

EDIT 1:

EXAMPLE EDI 850 DOCUMENT - (X12 Format)

ISA*01*0000000000*01*0000000000*ZZ*ABCDEFGHIJKLMNO*ZZ*123456789012345*101127*1719*U*00400*000003438*0*P*>
GS*PO*4405197800*999999999*20101127*1719*1421*X*004010VICS
ST*850*000000010
BEG*00*SA*08292233294**20101127*610385385
REF*DP*038
REF*PS*R
ITD*14*3*2**45**46
DTM*002*20101214
PKG*F*68***PALLETIZE SHIPMENT
PKG*F*66***REGULAR
TD5*A*92*P3**SEE XYZ RETAIL ROUTING GUIDE
N1*ST*XYZ RETAIL*9*0003947268292
N3*31875 SOLON RD
N4*SOLON*OH*44139
PO1*1*120*EA*9.25*TE*CB*065322-117*PR*RO*VN*AB3542
PID*F****SMALL WIDGET
PO4*4*4*EA*PLT94**3*LR*15*CT
PO1*2*220*EA*13.79*TE*CB*066850-116*PR*RO*VN*RD5322
PID*F****MEDIUM WIDGET
PO4*2*2*EA
PO1*3*126*EA*10.99*TE*CB*060733-110*PR*RO*VN*XY5266
PID*F****LARGE WIDGET
PO4*6*1*EA*PLT94**3*LR*12*CT
PO1*4*76*EA*4.35*TE*CB*065308-116*PR*RO*VN*VX2332
PID*F****NANO WIDGET
PO4*4*4*EA*PLT94**6*LR*19*CT
PO1*5*72*EA*7.5*TE*CB*065374-118*PR*RO*VN*RV0524
PID*F****BLUE WIDGET
PO4*4*4*EA
PO1*6*696*EA*9.55*TE*CB*067504-118*PR*RO*VN*DX1875
PID*F****ORANGE WIDGET
PO4*6*6*EA*PLT94**3*LR*10*CT
CTT*6
AMT*1*13045.94
SE*33*000000010
GE*1*1421
IEA*1*000003438

These can be much larger documents, I'd say we deal with 10x this size on a regular basis. Each TextBox will store one of these document's data.

See this related question:
https://stackoverflow.com/questions/7589579/is-it-possible-to-store-elements-of-array-permanently-in-vba-in-excel

Summary:
To store data persistently in the workbook using VBA, you have a few options:

Store it in another worksheet. If you don't want the user to be able to see the worksheet, Sheets("ArrayValuesWorksheet").Visible = xlVeryHidden
Create a defined name which has the desired data as its value. Names("MyArrayData") = myArray
Store it in ThisWorkbook.CustomDocumentProperties -- see https://stackoverflow.com/questions/17406585/vba-set-custom-document-property
Store it in a CommandBar, hopefully the user won't find it
Use a helper XLL which can call Excel4(xlDefineBinaryName,...) and Excel4(xlGetBinaryName)

I believe storing in a hidden sheet is the most common.


Inserting tables with foreign key How can I improve this method that adds data to three tables in the database?

The tables are:

UserTable
UserInfoTable
ContactTable

UserInfoTable and ContactTable have a one-to-many relationship, with ContactTable having UserInfoID as a foreign key.

public static boolean addUser(User user) {
Connection connection = getConnection();
try {
PreparedStatement insertUser = connection.prepareStatement(Insert_User);
insertUser.setString(1, user.getUsername());
insertUser.setString(2, user.getPassword());
insertUser.setString(3, user.getUserType());
insertUser.executeUpdate();
PreparedStatement insertUserInfo = connection.prepareStatement(Insert_UserInfo);
insertUserInfo.setString(1, user.getFirstName());
insertUserInfo.setString(2, user.getMiddleName());
insertUserInfo.setString(3, user.getLastName());
insertUserInfo.setString(4, user.getGender());
insertUserInfo.setString(5, user.getBirthdate());
insertUserInfo.executeUpdate();
//I need to get the UserInfoID to insert to ContactTable. I really hate to do this but I can't think of any way to insert the foreign key.
PreparedStatement getUserInfoID = connection.prepareStatement(GET_USER_INFO_ID);
getUserInfoID.setString(1, user.getFirstName());
getUserInfoID.setString(2, user.getMiddleName());
getUserInfoID.setString(3, user.getLastName());
getUserInfoID.setString(4, user.getGender());
getUserInfoID.setString(5, user.getBirthdate());
ResultSet rs = getUserInfoID.executeQuery();
PreparedStatement insertContact = connection.prepareStatement(Insert_Contact);
while (rs.next()) {
insertContact.setInt(1, rs.getInt(1));
insertContact.setString(2, user.getEmailAddress());
insertContact.setString(3, user.getAddress());
insertContact.setString(4, user.getContactNumber());
insertContact.executeUpdate();
}
} catch (SQLException sqle) {
sqle.printStackTrace();
return false;
}
return true;
}

I originally want to set the connections's autocommit to false so the connection will rollback if an insert statement fails, but since I need to get the UserInfo ResultSet to get the UserInfoID, I can't do it anymore. How can I do this better?

I think you want to use transactions. Within a transaction, you can certainly do INSERT + INSERT + SELECT the UserInfoID + the final INSERT. Commit the transaction at the end, after all successful, rollback on any exception.

Rather than using the low level database APIs like managing Connection, PreparedStatement, ResultSet, yourself, I strongly recommend to use a framework for this, such as Spring's JDBC template or Apache's DbUtils. Once you step on that road life will get easier and you won't look back.


Sending a file in servlet response, using a class inside a method I am processing file inside my method - basically preparing it for the download.

But I decided to split actions for setting headers and reading bytes.
As Java does not support method declaration inside methods, I decided to go with class inside method.

public static String downloadFile(final String fileBody, HttpServletResponse response) {
    class FileDownloader {

        private static final String HEADER_KEY = "Content-Disposition";
        private static final String HEADER_VALUE = "attachment; filename=\"%s\"";
        private static final String TEMPLATE_FILE_NAME = "filename.html";

        private void setHeader(HttpServletResponse response) {
            response.setHeader(HEADER_KEY, String.format(HEADER_VALUE, TEMPLATE_FILE_NAME));
        }

        public void processResponse(HttpServletResponse response) throws IOException {
            byte[] bytes = fileBody.getBytes();
            IOUtils.copy(new ByteArrayInputStream(bytes), response.getOutputStream());
            setHeader(response);
        }
    }

    try {
        new FileDownloader().processResponse(response);
        return StringUtils.EMPTY;
    } catch (IOException e) {
        e.printStackTrace();
        return null;
    }
}

But I am not sure whether the idea I've come up with is good or bad. Basically there is no reason for method separation because they are not doing a lot of work.

Yeah..... no. Not a good idea.... ;-)

You are using a class to do the job of a method. The real question is why you feel you need to declare the method inside your other method (for the record, it is highly unconventional to declare a formal class inside a method too... an anonymous class is common, but a formal class is odd)

You really just want to have a private-static method that does the work for you... something like:

    private static final String HEADER_KEY = "Content-Disposition";
    private static final String HEADER_VALUE = "attachment; filename=\"%s\"";
    private static final String TEMPLATE_FILE_NAME = "filename.html";

    private static void processResponse(HttpServletResponse response, String fileBody) throws IOException {
        byte[] bytes = fileBody.getBytes();
        IOUtils.copy(new ByteArrayInputStream(bytes), response.getOutputStream());
        response.setHeader(HEADER_KEY, String.format(HEADER_VALUE, TEMPLATE_FILE_NAME));
    }

then your code becomes:

public static String downloadFile(final String fileBody, HttpServletResponse response) {
    try {
        processResponse(response, fileBody);
        return StringUtils.EMPTY;
    } catch (IOException e) {
        e.printStackTrace();
        return null;
    }
}

which in turn, probably means the functional extraction is too much... how about just:

public static String downloadFile(final String fileBody, HttpServletResponse response) {
    try {
        byte[] bytes = fileBody.getBytes();
        IOUtils.copy(new ByteArrayInputStream(bytes), response.getOutputStream());
        response.setHeader(HEADER_KEY, String.format(HEADER_VALUE, TEMPLATE_FILE_NAME));
        return StringUtils.EMPTY;
    } catch (IOException e) {
        e.printStackTrace();
        return null;
    }
}


Search through JavaScript object using a whitelist/filterlist I am trying to build a small search function where I can search through some JavaScript objects even if there are arrays in it. I got it working but I would like to ask if someone can find a better approach than the one I made. Also, I assume I have some cases I did not think of.

My current search function looks as follows:

function searchFor(needle, haystack, filter, inside){
    if (inside == undefined) inside = 1;
    if (filter){
        for (var c = 0; c < filter.length; c++) {
            var splittedFilter = filter[c].split('.');
            for (var s = 0; s < splittedFilter.length; s++) {
                var obj = haystack[splittedFilter[s]];
                if (obj){
                    if (Array.isArray(obj) && obj.length > 0){
                        for (var i = 0; i < obj.length; i++) {
                            var result = searchFor(needle, obj[i], [splittedFilter[inside]], inside+1);
                            if (result) return true;
                        }
                    } else if( typeof obj == "object"){
                        var result = searchFor(needle, obj, [splittedFilter[inside]], inside+1);
                        if (result) return true;
                    } else {
                        if (obj.toLowerCase().indexOf(needle.toLowerCase()) != -1){
                            return true;
                        }
                    }
                }
            }
        }
    }
    return false
}

The filterlist:

var filter = ['company', 'friends.name'];

The haystack:

var hay =  [
  {
    "id": "57db96f4acfaec3218383063",
    "name": {
      "first": "Nora",
      "last": "Cooke"
    },
    "company": "MITROC",
    "email": "nora.cooke@mitroc.com",
    "friends": [
      {
        "id": 0,
        "name": "West Duke"
      },
      {
        "id": 1,
        "name": "Williams Kelley"
      },
      {
        "id": 2,
        "name": "Amelia Kirk"
      }
    ]
  },
  {
    "id": "57db96f4d5cae409054d3a5b",
    "name": {
      "first": "Dickson",
      "last": "Moses"
    },
    "company": "VISUALIX",
    "email": "dickson.moses@visualix.org",
    "friends": [
      {
        "id": 0,
        "name": "Patty Carr"
      },
      {
        "id": 1,
        "name": "Bowers Wilkerson"
      },
      {
        "id": 2,
        "name": "Fox Kidd"
      }
    ]
  }
];

The execution of the search function:

hay.forEach(function(element) {
    if (searchFor('Duke', element, filter)) {
        document.getElementById('result').innerHTML = document.getElementById('result').innerHTML + '<br />' + JSON.stringify(element);
    }
}, this);

needle is the string to search for
haystack is one object
filter is an array of filters, like: var filter = ['company'];
inside is to determine how far in the recursive loop the function is

I made a live example here.

You could use a simplified version for lookup all needed values and the check against the needle.

function search(needle, haystack, filter) {
    return filter.some(function (f) {
        function iter(o, i) {
            var item;

            if (!(ff[i] in o)) {
                return false;
            }

            item = o[ff[i]];
            i++;

            if (i === ff.length) {
                return item.indexOf(needle) !== -1;
            }

            if (Array.isArray(item)) {
                return item.some(function (a) {
                    return iter(a, i);
                });
            }
            return iter(item, i);
        }

        var ff = f.split('.');

        return iter(haystack, 0);
    });
}

var filter = ['company', 'friends.name'],
    hay = [{ "id": "57db96f4acfaec3218383063", "name": { "first": "Nora", "last": "Cooke" }, "company": "MITROC", "email": "nora.cooke@mitroc.com", "friends": [{ "id": 0, "name": "West Duke" }, { "id": 1, "name": "Williams Kelley" }, { "id": 2, "name": "Amelia Kirk" }] }, { "id": "57db96f4d5cae409054d3a5b", "name": { "first": "Dickson", "last": "Moses" }, "company": "VISUALIX", "email": "dickson.moses@visualix.org", "friends": [{ "id": 0, "name": "Patty Carr" }, { "id": 1, "name": "Bowers Wilkerson" }, { "id": 2, "name": "Fox Kidd" }] }, { "id": "57db96f42215bea719d83036", "name": { "first": "Kerr", "last": "Gray" }, "company": "TUBESYS", "email": "kerr.gray@tubesys.io", "friends": [{ "id": 0, "name": "Rose Pierce" }, { "id": 1, "name": "Matthews Moore" }, { "id": 2, "name": "Brenda Ayala" }] }, { "id": "57db96f435d0f064afc90768", "name": { "first": "Elsie", "last": "Dejesus" }, "company": "KLUGGER", "email": "elsie.dejesus@klugger.us", "friends": [{ "id": 0, "name": "Gina Cruz" }, { "id": 1, "name": "Nieves Carrillo" }, { "id": 2, "name": "Chandler Sanford" }] }, { "id": "57db96f4597631fffff36b6b", "name": { "first": "Chan", "last": "Mcdaniel" }, "company": "AQUACINE", "email": "chan.mcdaniel@aquacine.name", "friends": [{ "id": 0, "name": "Hodge Sweeney" }, { "id": 1, "name": "Sweet Burnett" }, { "id": 2, "name": "Lynn Hunter" }] }];

console.log(hay.map(function (a) {
    return search('Sweet', a, filter);
}));

console.log(hay.map(function (a) {
    return search('Rose', a, filter);
}));
.as-console-wrapper { max-height: 100% !important; top: 0; }


Writing a class that represents a sorted list of filenames I have a question:

  Write a class that represents a sorted list of filenames. The filenames are sorted by most recently added first. Duplicates are not allowed.

And I wrote this solution:

public class RecentlyUsedList

{

    private readonly List<string> items;

    public RecentlyUsedList()

    {

        items = new List<string>();

    }

    public void Add(string newItem)

    {

        if (items.Contains(newItem))

        {

            int position = items.IndexOf(newItem);

            string existingItem = items[position];

            items.RemoveAt(position);

            items.Insert(0, existingItem);

        }

        else

        {

            items.Insert(0, newItem);

        }

    }

    public int Count

    {

        get

        {

            int size = items.Count;

            return size;

        }

    }

    public string this[int index]

    {

        get

        {

            int position = 0;

            foreach (string item in items)

            {

                if (position == index)

                    return item;

                ++position;

            }

            throw new ArgumentOutOfRangeException();

        }

    }

}

Now, other person asked me, do the code review of your own code and tell the answers of following questions:

  Three operations are required:

  How long is the list?
  Access filename at a given position
  Add a filename to the list; if it already exists in the list it gets moved to the top otherwise the new filename is simply prepended to the
  top.

I got confused and tried to look on Google but I didn't get much from there. What would be the best answers to these questions?

Empty lines as separation around code lines that are related are always a good idea. Your code has a little too much empty lines where it is not common in C# (or C/C++, Java, JavaScript):

public class RecentlyUsedList

{

This in fact decreases readability. Instead just write:

public class RecentlyUsedList
{

      private readonly List<string> items;

      public RecentlyUsedList()

      {

        items = new List<string>();

      }

Here the constructor is not necessary. Just do:

private readonly List<string> items = new List<string>();

Your Add(...) method can be simplified to this:

  public void Add(string newItem)
  {
    items.Remove(newItem);
    items.Insert(0, newItem);
  }

items.Remove(newItem) just returns false, if the item is not present, so it's safe to use in any case. There is no need to be concerned about the existing string because it is the same as the newItem.

public int Count... can be simplified to:

public int Count => items.Count;

  List<string> items

has an indexer it self, so you can use that when implementing the indexer:

  public string this[int index]
  {
    get
    {
      if (index < 0 || index >= items.Count)
      {
        throw new ArgumentOutOfRangeException();
      }
      return items[index];
    }
  }

Here I throw an exception if the index argument is out of range. You could let items handle that as well...

In fact your foreach-loop is potentially much slower than the List<T>[index] because you make a kind of search where List<T>[index] just performs a look up. So you hide an efficient behavior with a not so efficient one.


Dynamic field access in Haskell I have to transform a response into the internal Coordinate record, some of the fields are lists and I need to handle empty lists case.

Records:

newtype GeoResponse =
  GeoResponse
    { results :: [ResultResponse]
    }
  deriving (Eq, Show, Generic)

newtype ResultResponse =
  ResultResponse
    { locations :: [LocationResponse]
    }
  deriving (Eq, Show, Generic)

newtype LocationResponse =
  LocationResponse
    { latLng :: CoordinateResponse
    }
  deriving (Eq, Show, Generic)

data CoordinateResponse =
  CoordinateResponse
    { lat :: !Double
    , lng :: !Double
    }
  deriving (Eq, Show, Generic)

Mapper function:

toCoordinate :: GeoResponse -> Coordinate
toCoordinate (GeoResponse (ResultResponse [LocationResponse (CoordinateResponse lat lng)]:_)) =
  Coordinate lat lng
toCoordinate _ = Coordinate 0 0

So I didn't find a better way to get Coordinate without pattern match, but it looks a bit ugly to me.

Do you have any suggestion to improve my code?

Note that, if you just remove the tag Response from all your types (including unifying the types CoordinateResponse and Coordinate which seem like they ought to be a single type), it's already a lot more readable:

toCoordinate :: Geo -> Coordinate
toCoordinate (Geo (Result [Location coord]:_)) = coord
toCoordinate _ = Coordinate 0 0

Also, the Location newtype doesn't seem to be serving any purpose, so maybe Location and Coordinate should just be the same type, too, which would remove a level.

Otherwise, what you've written is pretty idiomatic.  It's possible to use coerce to "cut through" all of the newtypes, so you could write:

import Data.Coerce

toCoordinate' :: GeoResponse -> Coordinate
toCoordinate' gr = case coerce gr of
  [CoordinateResponse lat lng]:_ -> Coordinate lat lng
  _                              -> Coordinate 0 0

or if CoordinateResponse and Coordinate were the same type:

toCoordinate' :: Geo -> Coordinate
toCoordinate' gr = case coerce gr of
  [coord]:_ -> coord
  _         -> Coordinate 0 0

That's obviously shorter, though it may hide what you're doing.  It seems important that you are pattern matching against the first of (possibly many) ResultResponse values but requiring that response to consist of a single LocationResponse, and that's not so obvious from this version.


Right-trimming text files below vbscript code took me 3hours just to complete trimming whitespaces of a 24MB file any tips to speed up the process below?

dim filename, path \'variables
     Set objFSO = CreateObject("Scripting.FileSystemObject")
     Set objFile = objFSO.GetFile(Wscript.ScriptFullName)
     strFolder = objFSO.GetParentFolderName(objFile)
     filename = replace(strFolder,"jobs\SCRIPTS","shared\file") & "\DCS.txt"
     converter(filename)
     objFile.Close
     function converter(filename)
     Const ForReading = 1
     Const ForWriting = 2
     Set objFSO = CreateObject("Scripting.FileSystemObject")
     PATH = filename
     Set objFile = objFSO.OpenTextFile(PATH, ForReading)
     Do Until objFile.AtEndOfStream
        strLine = objFile.Readline
        strLine = Trim(strLine)
        If Len(strLine)  0 Then
            strNewContents = strNewContents & strLine & vbcrlf
        End If
     Loop
     objFile.Close
     Set objFile = objFSO.OpenTextFile(PATH, ForWriting)
     objFile.Write strNewContents
     objFile.Close
     msgbox "done trim on " & filename
     end function

If you're going to be writing VBScript or WSScript files then you should be using VBEdit.

Allow it is not required you should still capitalize End, Function and Sub.
Wrapping each tasks in it's own Sub or Function will make it easier to read, debug and modify your code.
A Function that does not return a value should be a Sub.
Avoid editing text files line by line.  It is much more efficient to split the text into an array of lines, edit each element of the array, and then join the output.
Refactored Code
This code takes less than 1 second to trim each line of a 27 MB text file.
Dim FullFileName
FullFileName = GetSharedFileName

If FileSystemObject.FileExists(FullFileName) Then
    TrimFileContents FullFileName
    MsgBox "Done"
Else
    MsgBox "File not found: " & FullFileName
End If

Sub TrimFileContents(FullFileName)
    Dim Lines
    Lines = GetTextFileLines(FullFileName)

    Dim n

    For n = 0 To UBound(Lines)
        Lines(n) = Trim(Lines(n))
    Next

    OverWriteTextFile FullFileName, Join(Lines, vbNewLine)
End Sub

Function GetSharedFileName
    GetSharedFileName = Replace(GetParentFolderName,"jobs\SCRIPTS","shared\file") & "\DCS.txt"
End Function

Function FileSystemObject
    Set FileSystemObject = CreateObject("Scripting.FileSystemObject")
End Function

Function GetParentFolderName

    GetParentFolderName = FileSystemObject.GetFile(Wscript.ScriptFullName).ParentFolder.Path

End Function

Function GetTextFileLines(FullFileName)
    Const ForReading = 1
    Dim Text
    With FileSystemObject.OpenTextFile(FullFileName, ForReading)
        Text = .ReadAll
        .Close
    End With

    If InStr(Text, vbNewLine) = 0 Then
        GetTextFileLines = Split(Text, vbLf)
    Else
        GetTextFileLines = Split(Text, vbNewLine)
    End If

End Function

Sub OverWriteTextFile(FullFileName, Text)
    Const ForWriting = 2

    With FileSystemObject.OpenTextFile(FullFileName, ForWriting)
        .Write Text
        .Close
    End With

End Sub


A class to store the number of seconds since midnight The purpose of this code is to to implement the time as the number of seconds since midnight. The class  should have one data field (an int  with the number of seconds since midnight) instead of three. The Driver class will test the Time2 class . This program should ask the user to input the number of hours, minutes, and seconds past midnight, creating a Time2 object  and using the mutator methods . The program  should then use the toString() method  to print out the time.

I am unsure if I could put throw new IllegalArgumentException into a single statement if I need 3. But I have the code working.

import java.util.Scanner;

public class Driver {

public static void main(String[] args) {

    Scanner input = new Scanner(System.in);

    Time2 time = new Time2();

    System.out.print("Enter hours:");
    int hours = input.nextInt();
    System.out.print("Enter minutes:");
    int minutes = input.nextInt();
    System.out.print("Enter seconds:");
    int seconds = input.nextInt();

    time.setHour(hours);
    time.setMinute(minutes);
    time.setSecond(seconds);
    time.toString();
}

public static class  Time2 {

private  int  hour;
private  int  minute;
private  int  second;

public Time2() {this(0, 0, 0);}
public Time2(int  hour) {this(hour, 0, 0);}
public Time2(int  hour, int  minute) {this(hour, minute, 0);}

public Time2(int  hour, int  minute, int  second) {
    if(hour<0||hour>=24)
        throw new IllegalArgumentException("hour must be 0-23");
    if (minute < 0 || minute >= 60)
        throw new IllegalArgumentException("minute must be 0-59");
    if (second < 0 || second >= 60)
        throw new IllegalArgumentException("second must be 0-59");
    this.hour = hour;
    this.minute = minute;
    this.second = second;
}

public Time2(Time2 time) {this(time.getHour(), time.getMinute(), time.getSecond());}

public void setTime(int  hour, int  minute, int  second) {
    if (hour<0||hour>=24)
        throw new IllegalArgumentException("hour must be 0-23");
    if (minute < 0 || minute >= 60)
        throw new IllegalArgumentException("minute must be 0-59");
    if (second < 0 || second >= 60)
        throw new IllegalArgumentException("second must be 0-59");
    this.hour = hour;
    this.minute = minute;
    this.second = second;
}

public void setHour(int  hour) {
    if (hour < 0 || hour >= 24)
        throw new IllegalArgumentException("hour must be 0-23");
    this.hour = hour;
}

public void setMinute(int  minute) {
    if (minute < 0 && minute >= 60)
        throw new IllegalArgumentException("minute must be 0-59");
    this.minute = minute;
}

public void setSecond(int  second) {
    if (second <= 0 || second > 60)
        throw new IllegalArgumentException("second must be 0-59");
    this.second = second;
}

public int  getHour() {return hour;}
public int  getMinute() {return minute;}
public int  getSecond() {return second;}

public String  toUniversalString() {
    return String .format( "%02d:%02d:%02d", getHour(), getMinute(), getSecond());
}

public String  toString() {
    return String .format("%d:%02d:%02d %s",
                          ((getHour() == 0 || getHour() == 12) ? 12 : getHour() % 12), getMinute(),
                          getSecond(), (getHour() < 12 ? "AM" : "PM"));
}
}
}

Your validation is too strict, as a minute can sometimes have 61 seconds. Limiting it to 60 seconds is what causes computer crashes during leap seconds.

To avoid code duplication, the Time(hour, minute, second) constructor should just do:

public Time(int hour, int minute, int second) {
    this.setHour(hour);
    this.setMinute(minute);
    this.setSecond(second);
}

Your spacing is a bit weird here:

return String .format( "%02d:%02d:%02d", getHour(), getMinute(), getSecond());

It would be more conventional to write:

return String.format("%02d:%02d:%02d", getHour(), getMinute(), getSecond());


Is there a better way of defining custom calendars? I need to define custom calendars and, in particular, test a DateTime for being a holiday. My current code is shown below. Is there a more concise/better way of doing this, preferably without sacrificing performance?

let private isHoliday2011 (dt:DateTime) =
  match dt.DayOfWeek with
  | DayOfWeek.Saturday | DayOfWeek.Sunday ->
    match (dt.Month, dt.Day) with
    | (3,5) -> false
    | _ -> true
  | _ ->
    match dt.Month with
    | 1 ->
      match dt.Day with
      | 3 | 4 | 5 | 6 | 7 | 10 -> true
      | _ -> false
    | 2 ->
      match dt.Day with
      | 23 -> true
      | _ -> false
    | 3 ->
      match dt.Day with
      | 7 | 8 -> true
      | _ -> false
    | 5 ->
      match dt.Day with
      | 2 | 9 -> true
      | _ -> false
    | 6 ->
      match dt.Day with
      | 13 -> true
      | _ -> false
    | 11 ->
      match dt.Day with
      | 4 -> true
      | _ -> false
    | _ -> false

If you need to check for holidays repeatedly, I would go for Set and flatten pattern matching blocks for readability:

let holidays = set [ 1, 3; 1, 4; 1, 5; 1, 6; 1, 7; 1, 10 // month, day
                     2, 23
                     3, 7; 3, 8
                     5, 2; 5, 9
                     6, 13
                     11, 4
                    ]

let private isHoliday2011 (dt: DateTime) =
  match dt.DayOfWeek, dt.Month, dt.Day with
  | DayOfWeek.Saturday, m, d | DayOfWeek.Sunday, m, d -> m <> 3 || d <> 5
  | _, m, d -> Set.contains (m, d) holidays


How can the below code can be refactored with design pattern? In one of our project with 3 tier architecture (tightly coupled), there's a bad code smell and it doesn't follow the DRY principle. I want to refactor it with design possible design pattern. I don't want to write this project from scratch.

Here's one of the class in BLL:

namespace BAL
{
   public class NotificationSettings
   {
    public readonly bool SignUPFBPost;
    public readonly bool SignUPEmails;
    public readonly bool SignUPPushNotification;
    public readonly bool SignUPFCActivity;
    public readonly bool SignUPBubbleNotification;
    public readonly bool SignUPGlobeNotification;

    public NotificationSettings()
    {
        NotificationSettingsBAL objNotificationSettings = new NotificationSettingsBAL();
        DataTable dtblNotifList = objNotificationSettings.GetNotificationList();
        SignUPFBPost = (Convert.ToInt16(dtblNotifList.Rows[0]["FBWallPost"]) == 1) ? true : false;
        SignUPEmails = (Convert.ToInt16(dtblNotifList.Rows[0]["Emails"]) == 1) ? true : false;
        SignUPPushNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["PushNotification"]) == 1) ? true : false;
        SignUPFCActivity = (Convert.ToInt16(dtblNotifList.Rows[0]["FCActivity"]) == 1) ? true : false;
        SignUPBubbleNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["BubbleNotification"]) == 1) ? true : false;
        SignUPGlobeNotification = (Convert.ToInt16(dtblNotifList.Rows[0]["GlobNotification"]) == 1) ? true : false;
    }
   }
}

We're getting various user related settings db from database and checking at different places as per the need and displaying messages or sending push notification.

if (objSqlResult.IsSuccess)
{
    FavorBAL.Favor objFavorStatus = (FavorBAL.Favor)objFavorBAL.Status;

    switch (objFavorStatus)
    {
        case FavorBAL.Favor.Initiated:
            break;
        case FavorBAL.Favor.Accepted:
            strTitle = "Accept";
            strMessage = MsgSuccess.GetMsg(Successes.FBPostFavorAccept).Replace("{%RecMemberName%}", objMemberBAL.FirstName + " " + objMemberBAL.LastName).Replace("{%ReqMemberName%}", Convert.ToString(dtblFavor.Rows[0]["ReqMemberName"]));
            strEmailMessage = MsgSuccess.GetMsg(Successes.FBPostFavorAccept).Replace("{%RecMemberName%}", objMemberBAL.FirstName + " " + objMemberBAL.LastName).Replace("{%ReqMemberName%}", "you");
            Common.DisplayMessageFavor(divMsg, MsgSuccess.GetMsg(Successes.FriendFavorAccept), Common.ErrorMsgType.Success);
            if (new NotificationSettings().FavorAcceptFBPost)
            {
                Response.Write(Javascript.ScriptStartTag + "PostOnFBWall(\"" + strMessage + "\",\"" + Config.FacebookAppUrl + "\",\"" + Config.WebSiteUrl + "images/logo.png\",\"Favor in Progress!\",\"\",\"Message\");" + Javascript.ScriptEndTag);
            }
            if (new NotificationSettings().FavorAcceptEmails)
            {
                IsNotificationMailAllowed = true;
            }
            if (new NotificationSettings().FavorAcceptPushNotification)
            {
                objPushNotification.Push(strMessage, strUDID);
            }
            break;
            //Other Cases
    }
}

How can we refector the code with or without a design pattern? There's replication of second code listing or similar to it. I am reading one of the article on interface to make it loosely coupled so that system can adapt changes easily.

I would look at replacing the BLL code with the following. With the main focus being replacing the Convert.ToInt16 repetition with a method. The other area of concern is the inability to mock your BAL code. To overcome this I have written a quick and dirty interface which allows better unit testing.

namespace BAL
{
    // interfaces allow the class ot be easily mocked
    internal interface ICustomDataColumnTryParse
    {
        bool TryParseBool(string columnName);
    }

    // interfaces allow the class ot be easily mocked
    internal interface INotificationSettingsDomain
    {
        DataTable GetNotificationList();
    }

    // sample implementation of the interface
    internal class NotificationSettingsBAL : INotificationSettingsDomain
    {
        public DataTable GetNotificationList()
        {
            // do whatever you need to return information from your data repository
            throw new System.NotImplementedException();
        }
    }

    internal class NotificationSettings : ICustomDataColumnTryParse
    {
        // properties are more widely accepted for consumers of this class over
        // traditional fields / variables, but it really depends on 'what' you want
        // to use these values for?
        internal bool SignUPFBPost { get; private set; }
        internal bool SignUPEmails { get; private set; }
        internal bool SignUPPushNotification { get; private set; }
        internal bool SignUPFCActivity { get; private set; }
        internal bool SignUPBubbleNotification { get; private set; }
        internal bool SignUPGlobeNotification { get; private set; }

        private DataTable m_notificationListDataTable;

        internal NotificationSettings()
            : this(new NotificationSettingsBAL())
        { }

        // using an interface allows mocking of the 'NotificationSettingsBAL' object
        public NotificationSettings(INotificationSettingsDomain notificationSettingsDomain)
        {
            // prefer suffixes over prefixes for naming conventions
            m_notificationListDataTable = notificationSettingsDomain.GetNotificationList();

            SignUPFBPost = TryParseBool("FBWallPost");
            SignUPEmails = TryParseBool("Emails");
            SignUPPushNotification = TryParseBool("PushNotification");
            SignUPFCActivity = TryParseBool("FCActivity");
            SignUPBubbleNotification = TryParseBool("BubbleNotification");
            SignUPGlobeNotification = TryParseBool("GlobNotification");
        }

        // this can be a method on this class, extension method (my preference) or a static method
        // i have opted for an internal class method coded against an interface so the data table
        // isn't passed around.
        public bool TryParseBool(string columnName)
        {
            // this can allow for additional logic if we wanted to get a different row index
            var row = m_notificationListDataTable.Rows[0];
            var value = row[columnName];

            if (value == null)
            {
                return false;
            }

            int valueAsInt = 0;
            int.TryParse(value.ToString(), out valueAsInt);
            return valueAsInt == 1;
        }
    }
}

As for replacing the switch statement you might be over-using an architectural pattern. I would only choose something like a strategy pattern if the business logic is likely to change or that additional switch statements will be added / removed with a high frequency in the future.


Disabling selection in textbox I have a form that displays a list of days in a given month that looks like so:

All the text boxes are set as readonly, which is the desired functionality.
While setting enabled to false works, it gives the control a blurred out look, which is difficult to override. In order to complete them, I want to keep the user from being able to select any text.

I have bound the KeyUp and MouseUp events in the following code:

private void dateDisplay_MouseUp(object sender, MouseEventArgs e)
{
    this.dateDisplay.SelectionStart = this.dateDisplay.Text.Length;
}

private void dateDisplay_KeyUp(object sender, KeyEventArgs e)
{
    this.dateDisplay.SelectionStart = this.dateDisplay.Text.Length;
}

where dateDisplay is the left textbox.

And this what the other textbox looks like with text selected, without being wired up to any events:

This code prevents the user from selecting any text inside the text boxes by forcing the cursor to end of the text box. Is there a better way of doing this?

Another possible solution is to use a DataGridView.  You may define a column to be read only and it will appear as normal, i.e. not grayed out when the control is not enabled.  Plus I think a DataGridView would be a better solution overall given that you apparently want to displays rows of data.


Max heap implementation in Kotlin I wrote this class a while ago to learn Kotlin and heaps, so any tips (on either my code style or my heap algorithm) would be appreciated!

class MaxHeap<T : Comparable<T>>() {
    private val items: MutableList<T?> = mutableListOf()
    var size: Int = 0
        private set

    fun push(item: T) {
        set(size++, item)
        siftUp(size - 1)
    }

    fun peek() = get(0)

    fun pop(): T? {
        // If there are no items in the heap, null is returned
        if (size == 0) {
            return null
        }

        // Get the max item, replace it with the last item in the heap,
        // then sift it down until the heap property is restored
        val max = peek()
        set(0, get(--size))
        items.removeAt(size) // Remove original reference to the last item
        siftDown(0)
        return max
    }

    private fun siftUp(i: Int) {
        assert(i >= 0 && i < size)

        // The root node has no parent, so we cannot sift up any more
        if (i == 0) {
            return
        }

        // If the current item is larger than the parent, swap them and
        // then repeat on the item, which is now in the parent position
        val iParent = parentIndex(i)
        if (get(i)!! > get(iParent)!!) {
            swap(i, iParent)
            siftUp(iParent)
        }
    }

    private fun siftDown(i: Int) {
        assert(i >= 0 && i < size)

        // Cannot sift down leaf nodes
        if (i >= size / 2) {
            return
        }

        // Find the larger child value.
        // Left child is guaranteed to exist because this is not a leaf node,
        // and because we add items to the heap by appending them to the end,
        // the left mode must be filled before the right node.
        val iLeft = leftChildIndex(i)
        val iRight = rightChildIndex(i)
        var iLargerChild = iLeft
        if (iRight < size && get(iLeft)!! < get(iRight)!!) {
            iLargerChild = iRight
        }

        // If this item is less than the larger child, sift it down
        if (get(i)!! < get(iLargerChild)!!) {
            swap(i, iLargerChild)
            siftDown(iLargerChild)
        }
    }

    private fun set(i: Int, value: T?) {
        if (i < items.size) {
            items[i] = value
        } else {
            items.add(value)
        }
    }

    private fun get(i: Int): T? {
        if (i < items.size) {
            return items[i]
        } else {
            return null
        }
    }

    private fun swap(i: Int, j: Int) {
        val temp = items[i]
        items[i] = items[j]
        items[j] = temp
    }

    private fun parentIndex(i: Int) = (i - 1) / 2
    private fun leftChildIndex(i: Int) = i * 2 + 1
    private fun rightChildIndex(i: Int) = i * 2 + 2
}

This looks nice, though I don't actually know kotlin.

One suggestion I'd make is that when you have multiple relational conditions in a statement like this:

assert(i >= 0 && i < size)

It's slightly more readable if you arrange the terms in increasing order of their values, like this:

assert(0 <= i && i < size)


Project Euler #7 - 10001st prime I just finished Project Euler #7 in Swift, and since there is not any version yet on Code Review, I would like to have some comments on what I did to try to improve it.

By listing the first six prime numbers: 2, 3, 5, 7, 11, and 13, we can see that the 6th prime is 13.
What is the 10 001st prime number?

import Foundation

func isPrime(number:Int) -> Bool {

    if number == 1 {
        return false
    }
    else if number < 4 {
        return true
    }
    else if number % 2 == 0 {
        return false
    }
    else if number < 9 {
        return true
    }
    else if number % 3 == 0 {
        return false
    }
    else {
        let maxPrime = Int(ceil(sqrt(Double(number))))

        for var i = 5; i <= maxPrime; i += 6 {
            if number % i == 0  || number % (i + 2) == 0 {
                return false
            }
        }
    }

    return true
}

func getNumberForXthPrime(prime:Int) -> Int {

    var xThPrime = prime - 1 // We skip the prime 2 with the += 2
    var number = 1

    while xThPrime > 0 {

        number += 2
        if isPrime(number) {
            xThPrime--
        }
    }

    return number
}

func printTimeElapsedWhenRunningCode(operation:(xThPrime:Int) -> Int) {
    let startTime = CFAbsoluteTimeGetCurrent()
    let number = operation(xThPrime: 10_001)
    println(number)
    let timeElapsed = CFAbsoluteTimeGetCurrent() - startTime
    println("Time elapsed : \(timeElapsed) s")
}

printTimeElapsedWhenRunningCode(getNumberForXthPrime)

The code executes in 0.0181439518928528 s.

for var i = 5; i <= maxPrime; i += 6 {
    if number % i == 0  || number % (i + 2) == 0 {
        return false
    }
}

We can implement some Swift syntax here:

for i in stride(from:5 through:maxPrime by: 6) {
    if number % i == 0 || number % (i + 2) == 0 {
        return false
    }
}

I don't know how this would compare in terms of performance however.  I suspect it's the same or better however.


Engineering a minimalist image interface without templates I'm practicing different ways of class design, so I'd like feedback on the design characteristics. Of course code practice comments are also welcome.

It was a conscious decision to omit templates. My next version will use a class template to store image data, similar to CImg and boost::GIL. I intend to make more classes with increasing complexity (templates, abstract base class, variadic templates). Until I reach a detailed yet highly abstract image class.

Header:

#ifndef AIMG
#define AIMG

#include <vector>
#include <string>

class AImg
{
public:
  const std::vector<short> dimsizes; //or std::array of length 4?
  const int N;     //# of elements ( = product of dimension sizes)
  unsigned int Bytes {1}; //bytes per element
  std::string tag; //meta info like "rgb"
  std::string cast;//used for returning data of type cast
  void* data;      //pointer to some array/container/byte-blob

  AImg(const std::vector<short>&); //you have to specify image size when creating. But may be empty of data.
  void setdata(void*);
  void* getdata();

  //example of pseudo-template method:
  void plusone(std::string);
  int operator()(unsigned int); //variable args would allow better indexing

private:
  int getN();
  //checkcast() ?
};

//public fcns
AImg::AImg(const std::vector<short>& input): dimsizes(input), N(getN()), cast("") {}
int AImg::operator()(unsigned int idx){
   if (cast=="int")
    {return ((int*)data)[idx];} else return 0;
  }
void  AImg::setdata(void* input){data=(void*)input;}
void* AImg::getdata(){return data;}
void  AImg::plusone(std::string cast){
        if (cast=="int")
        {
          int* idata = (int*)data;
          for (int i = 0;i<N;i++){idata[i] += 1;}
        }
        else if (cast == "float")
        {
          float* fdata = (float*)data;
          for (int i = 0;i<N;i++){fdata[i] += 1.0;}
        }
        //...other types
        // careful: N is based on elements of some user-defined type. Not all types have the same precision (#bytes).
        // So you'd need to normalize N based on the type size ( sizeof() ) to properly iterate over the data, or forbid certain types
      }

//Private fcns
int AImg::getN(){ //doesn't STL have a std::prod() ?!?!
  int prod = 1;
  for(short n : dimsizes) prod *= n;
  return prod;
}

#endif // AIMG

Main:

#include <iostream>
#include "AImg.h"

using std::cout;  using std::cin;
using std::endl;  using std::vector;

int main()
{
  //create image object of size 2x2
AImg img(vector<short> {2,2});
  //create image data: simple int array
int d[4] {1,2,3,4};
img.setdata(d); //make sure length of d equals N, i.e. fill img up completely
img.plusone("int");
int* d2 = (int*)img.getdata(); //need to explicitly re-cast in c++
cout<<d2[0]<<d2[1]<<d2[2]<<d2[3]<<endl;
img.cast = "int";
cout<<img(0)<<img(1)<<img(2)<<img(3)<<endl;
}

Personally I think a class design like this is risky to use, so should I add more safety checks? It also seems harder to maintain and extend compared to a class using templates.

It also seems hard to extend the model to allow for inhomogenous (sub)pixels. For example, the value of a pixel at (1,1,1) could be an int but the value of the pixel at (1,1,2) could be an unsigned char.

A few notes about things and stuff (I know, my introductions are always original):

About that comment:

//doesn't STL have a std::prod() ?!?!

The standard library does have a function to compute the accumulated product of a range. It's called std::accumulate. You can use it like this:

int n = std::accumulate(
    std::begin(dimsizes), std::end(dimsizes),
    1, std::multiplies<int>{}
);

Agreed, it's a bit verbose. But the function is quite generic since it may work with any well-formed binary operation. Note that if you are using C++14, you can use std::multiplies<> instead of std::multiplies<int> and let the type be deduced.
Also, you mentioned std::array. If you know for sure that you will only store 4 values and not more, then please, by all means, use an std::array<short, 4> instead of an std::vector<short>.
Your case isn't really consistent. For local variables, the generally accepted practice is to use snake_case everywhere and to drop every capital letter.
Ok, I hate to say this, but void AImg::plusone(std::string cast) without templates really hurts me. I know that you plan to change that later, but the current version still hurts me ç___ç Also, since you're not altering the std::string, it would be better to take a const std::string& parameter instead.
Instead of that:

int AImg::operator()(unsigned int idx);

Array subcript tends to use the... well, the array subcript operator[] instead of the parenthesis. Unless you have to feed it several parameters, in which case operator() is sometimes preferred, but that's not your case.

int AImg::operator[](unsigned int idx);

I don't think that the method setdata really needs a cast from void* to void*:

void AImg::setdata(void* input) { data = input; }


Temperature conversion in JavaScript I've made a simple console application using JavaScript, NodeJS and yargs.

index.js

This is the startup file.

#!/usr/bin/env node
'use strict';

const yargs = require('yargs');
const argv = yargs
.option('celsius', {alias: 'c', default: false})
.option('fahrenheit', {alias: 'f', default: false})
.option('kelvin', {alias: 'k', default: false})
.help('h')
.argv;

let value = argv._[0];

if (!value) {
  console.log('You need to pass a value to be converted.');
} else {
  const convert = require('./convert')

  if (argv.celsius) {
    console.log(convert.fromCelsius(value));
  } else if (argv.fahrenheit) {
    console.log(convert.fromFahrenheit(value));
  } else if (argv.kelvin) {
    console.log(convert.fromKelvin(value));
  } else {
    console.log("You need to pass a valid flag.")
  }
}

convert.js

This is required by index.js in order to transform values. Each scale has it own file.

'use strict';

let fixFloat = (value) => parseFloat(value).toFixed(2);

let fromCelsius = (value) => {
  const conversion = require('./celsius');
  let kelvin = fixFloat(conversion.toKelvin(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  return `${value}°C = ${fahrenheit}°F = ${kelvin}K`;
};

let fromFahrenheit = (value) => {
  const conversion = require('./fahrenheit');
  let celsius = fixFloat(conversion.toCelsius(value));
  let kelvin = fixFloat(conversion.toKelvin(value));
  return `${value}°F = ${celsius}°C = ${kelvin}K`;
};

let fromKelvin = (value) => {
  const conversion = require('./kelvin');
  let celsius = fixFloat(conversion.toCelsius(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  return `${value}K = ${celsius}°C = ${fahrenheit}°F`;
};

module.exports = {
  fromCelsius,
  fromFahrenheit,
  fromKelvin,
};

celsius.js

'use strict';

let toKelvin = (value) => value + 273.15;
let toFahrenheit = (value) => (value * (9 / 5)) + 32;

module.exports = {
  toKelvin,
  toFahrenheit,
};

fahrenheit.js

'use strict';

let toCelsius = (value) => (value - 32) * (5 / 9);
let toKelvin = (value) => (value + 459.67) * (5 / 9);

module.exports = {
  toCelsius,
  toKelvin,
};

kelvin.js

'use strict';

let toCelsius = (value) => value - 273.15;
let toFahrenheit = (value) => 1.8 * toCelsius(value) + 32;

module.exports = {
  toCelsius,
  toFahrenheit,
};

You can see it running by installing it via npm

npm i -g thermo.js
thermo 10 -celsius
=> 10°C = 50.00°F = 283.15K

Is my implementation good? This is my first non-web JavaScript program. I want to make the code more idiomatic. I think I messed up some of ES6 variables declaration.

Generalizing code to write less of it

Adding support for a new time scale

To test your code for maintainability let's go through a hypothetical situation. What if a new temperature scale were introduced - say Felsius?

To add support for the up-and-coming scale you would need to add a new file, felsius.js, which will contain 3 conversion functions: toKelvin toCelsius toFahrenheit. Next add toFelsius functions in each of the existing temperature scale files.

Alright, lots of maths... but awesome. We did it! Now simply to add:

let fromFelsius = (value) => {
  const conversion = require('./felsius');
  let celsius = fixFloat(conversion.toCelsius(value));
  let fahrenheit = fixFloat(conversion.toFahrenheit(value));
  let kelvin = fixFloat(conversion.toKelvin(value));
  return `${value}°ϵ = ${celsius}°F = ${fahrenheit}°F = ${kelvin}K`;
};

And add:

let felsius = fixFloat(conversion.toFelsius(value));
`${felsius}°ϵ`;

To each fromX function.

Finally done! Let's reflect:

  Hmmmm... That seemed like an awful lot of work - possibly prone to errors too.

Unfortunately we don't have time for any more reflection because the project manager just informed us of a huge user base that firmly believes in the Rankine scale.

Possible modification

Having a specialized conversion function between each temperature scale requires \$n^2\$ conversion functions (where \$n\$ is the number of temperature scales you support). You could get by with \$2n\$ if you used a standard temperature scale as the root of all of your conversions. For example fahrenheit.js using Celsius as the standard:

'use strict';

let fromCelsius = (value) => (value * (9 / 5)) + 32;
let toCelsius = (value) => (value - 32) * (5 / 9);
let toString = (value) => `{value}°F`

module.exports = {
  fromCelsius,
  toCelsius,
  toString,
};

Now convert.js

'use strict';

let fixFloat = (value) => parseFloat(value).toFixed(2);

const temperatureScales = [
  {
    name: 'Fahrenheit',
    file: './farhenheit.js'
  },
  {
    name: 'Celsius',
    file: './celsius.js'
  },
  {
    name: 'Kelvin',
    file: './kelvin.js'
  },
  {
    name: 'Felsius',
    file: './felsius.js'
  },
];

// add conversion code for each temperature scale
module.exports = temperatureScales.reduce((m, fromScale) => {
  const fromTemp = require(fromScale.file);

  // generate the `fromFooBar` function
  m['from' + fromScale.name] = (value) => {
    const standard = fromTemp.toCelsius(value);

    let str = fromTemp.toString(fixFloat(value));

    // generate the rest of the result string
    return temperatureScales.reduce((str, toScale) => {
      if (toScale == fromScale) { // don't recompute the initial temperature
        return str;
      }

      const toTemp = require(toScale.file);
      const temperatureValue = fixFloat(toTemp.fromCelsius(standard));
      const temperatureString = toTemp.toString(temperatureValue);
      return str + " = " + temperatureString;
    }, str);
  };

  return m;
}, module.exports);

Eliminating some of the redundancies makes it much easier to add Rankine support. Simply add { name: 'Rankine', file: './rankine.js' } to the temperatureScales list. Then add three functions: toCelsius fromCelsius toString to ./rankine.js.

Often redundancies in code lead to more work when maintaining. However, the generalized code might be less readable than the redundant version (nested reduce functions - why would anyone do that??).

Nice work overall! For your purpose the redundancies are probably fine -- just some food for thought.


Chaining Tasks with async/await I've been trying to learn how to use the TPL for quite some time now.
Basically I'm trying to fire off a group of Tasks, wait for them to complete and then fire off another group of tasks, waiting for them to complete before firing off the tasks that update form elements on the UI Thread.

Here's what I got. I think I'm probably overcomplicating it however it does seem to work as I want it too.
Please could I have some advice on how I could simplify this based on what I want to achieve?

private async Task PrepareData()
{
    Task[] FirstTasks = new Task[] { TaskOne(), TaskTwo() };    // Do First group of Tasks
    await Task.WhenAll(FirstTasks); // Wait for First group of Tasks

    Task[] SecondTasks = new Task[] { TaskThree(), TaskFour() }; // Do Second group of Tasks
    await Task.WhenAll(SecondTasks).ContinueWith(c => // Wait for Second group of Tasks
        {
            GuiTaskOne();
            GuiTaskTwo();
        }, TaskScheduler.FromCurrentSynchronizationContext());  // Then update the form on the UI Thread
}

private async Task TaskOne()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskTwo()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskThree()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task TaskFour()
{
    someVariable = await Task.Run(() => DoSomething());
}

private async Task GuiTaskOne()
{
    someControl.Text = await Task.Run(() => DoSomething());
}

private async Task GuiTaskTwo()
{
    someControl.Text = await Task.Run(() => DoSomething());
}

Since await is effectively a continuation anyway, you could simplify your function to:

private async Task PrepareData()
{
    Task[] FirstTasks = new Task[] { TaskOne(), TaskTwo() };    // Do First group of Tasks
    await Task.WhenAll(FirstTasks); // Wait for First group of Tasks

    Task[] SecondTasks = new Task[] { TaskThree(), TaskFour() }; // Do Second group of Tasks
    await Task.WhenAll(SecondTasks); // Wait for Second group of Tasks

    GuiTaskOne();
    GuiTaskTwo();
}

I also suspect you meant to await the GUI tasks too.


A function that returns all letters in the Alphabet except the ones in a pre-defined list This function should return a string that is comprised from all the letters in the Alphabet except the letters in a pre-defined list in alphabetical order.

Assuming all letters in the pre-defined list are all lowercase;

I came up with two ways to do this and I would like to know which way is more efficient or more 'Pythonic'.
Thanks in advance.

The first way:

import string

def get_remaining_letters(list_of_characters):
    # casting the Alphabet to a list so that it becomes mutable

    list_of_all_letters = list(string.ascii_lowercase)
# looping for each character in the pre-defined list
    for char in list_of_characters :
        if char in list_of_all_letters: # to avoid possible errors caused by the .remove in case of a duplicate charcter in the pre-defined list
            list_of_all_letters.remove(char)
    return ''.join(list_of_all_letters) # returns a string

# ----------------------------------------------------------- #

pre_defined_list = ['a', 'b', 'b']

test = get_remaining_letters(pre_defined_list)
print(test)

output :

>>> cdefghijklmnopqrstuvwxyz

The second way:

import string

def get_remaining_letters(list_of_characters):

    if list_of_characters == []:
        return string.ascii_lowercase
    else:
        remaining_letters = ''
        for char in string.ascii_lowercase:
            if char not in list_of_characters:
                remaining_letters += char
        return remaining_letters

pre_defined_list = ['a', 'b', 'b']

test = get_remaining_letters(pre_defined_list)
print(test)

output :

>>> cdefghijklmnopqrstuvwxyz

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide and Flake8 http://flake8.pycqa.org/en/latest/ as a tool for style enforcement.

Docstrings: Python documentation strings or docstrings are a way of documenting your classes/function/modules and are usually accessed through help(). You should include docstrings to your functions indicating what they do and what they return.
Type hints: Type Annotations are a new feature added in PEP 484 that allow for adding type hints to variables. They are used to inform someone reading the code what the type of a variable should be. As you're taking a list as input and will return a string, you should have some documentation indicating that. Use type hints.

function should look like:

def get_remaining_letters(letters: list) -> str:
    """Return lowercase alphabetic letters that are not present in letters."""

Blank lines: instead of this # ----------------------------------------------------------- # an empty line is sufficient and note that according to PEP0008 empty lines as well as comments should be used sparingly (when necessary) to separate logical sections as well as explain the not-so-obvious parts of the code.
main guard: use if __name__ == '__main__': guard at the end of the script to allow outside modules to import this module without running it as a whole. You will be calling your functions from there.

if __name__ == '__main__':
    pre_defined_list = ['a', 'b', 'b']
    test = get_remaining_letters(pre_defined_list)
    print(test)

Evaluation to empty list: In Python empty sequences evaluate to False.
instead of if list_of_characters == []: it is written: if not list_of_characters:
Adding to strings: Python strings are immutable which means that each time you're adding to a string like in remaining_letters += char a new string is created and assigned the new value which is inefficient and should be replaced with list comprehension syntax which is shorter and more efficient or Pythonic as you call it.

An improved version of the code:

import string

def get_remaining_letters(letters: list) -> str:
    """Return lowercase alphabetic letters not present in letters."""
    return ''.join(letter for letter in string.ascii_lowercase if letter not in letters)

if __name__ == '__main__':
    chosen = [letter for letter in 'sdfmmmsvvd']
    print(get_remaining_letters(chosen))


Find the object that contains title1-2 I use jQuery and underscore.js, I have title1-2 and I would like to have the action corresponding.

this.items = {
    menuItems: [
        { title: 'title1',
            data: [
                { title: 'title1-1',
                    action: 'action1-1'
                },
                { title: 'title1-2',
                    action: 'action1-2'
                }
                ]
        },
        { title: 'title2',
            data: [
                { title: 'title2-1',
                    action: 'action2-1'
                },
                { title: 'title2-2',
                    action: 'action2-2'
                }
            ]
        }
    ]
};

Currently, I have the following code to do this:

var item = _.find(_.flatten(_.pluck(this.items.menuItems, 'data')), function (item) { return item.title === 'title1-2'; });
console.log(item.action);

Is there a better way to find it?

Actually the only improvement I think you can make is that you use the chaining sugar, looks a lot better:

var item = _.chain(items.menuItems)
  .pluck('data')
  .flatten()
  .find(function(a){
    return a.title === 'title1-2'
  })
  .value()


Package for testing network connection I wrote this package for monitoring the state of a network connection in the terminal because my net connection would go down for a few minutes at a time and come back up. I think it could be used in other applications.

I'm looking for any advice on how to improve or extend this package in any way possible.

Package repo

My concerns are as follows:

Is the package "Pythonic"?
Does the package contain any obvious bugs/errors/flaws?

#!/usr/bin/env python

'''
This module is for checking a networks status.
'''

import socket
from time import sleep as _sleep
from functools import wraps
import errno
import os
import signal

TIMEOUT = 2

class NetworkStatus(object):
    def timeout(seconds=1, error_message=os.strerror(errno.ETIME)):
        def decorator(func):
            def _handle_timeout(signum, frame):
                return
                signal.signal(signal.SIGALRM, _handle_timeout)
                signal.alarm(seconds)
            def wrapper(*args, **kwargs):
                try:
                    result = func(*args, **kwargs)
                finally:
                    signal.alarm(0)
                return result

            return wraps(func)(wrapper)

        return decorator

    def set_timeout(self, t_out):
        '''
        Update the global variable TIMEOUT.
        '''
        global TIMEOUT
        TIMEOUT = t_out

    def sleep(self,delay):
        '''
        Sleep function with cleanup.
        '''
        try:
            _sleep(delay)
        except KeyboardInterrupt:
            exit()

    @timeout(TIMEOUT)
    def check(self, **kw):
        '''
        Check if the network is up.
        '''
        self.sleep(TIMEOUT)
        try:
            socket.gethostbyname(kw.get('remote_server', 'www.google.com'))
            return True
        except Exception:
            return False

    def online(self, **kw):
        '''
        return True is the network is UP.
        '''
        return self.check(**kw) is True

    def offline(self, **kw):
        '''
        return False if the network is DOWN.
        '''
        return self.online(**kw) is False

    def if_online(self, func, *args, **kw):
        if self.online():
            func(*args, **kw)

    def if_offline(self, func, *args, **kw):
        if self.offline():
            func(*args,**kw)

    def mainloop(self):
        '''
        Continuously check if the network is UP or DOWN.
        '''
        while True:
            up = self.check()
            if up:
                print 'Network is up'
            else:
                print 'network is down.'

if __name__ == '__main__':
    NetworkStatus().mainloop()

Importing sleep as _sleep is an awkward work around. It'd be much easier to do a plain import time, then in your sleep function refer to time.sleep. No need for odd aliases and nothing gets overwritten.

This function does nothing:

def _handle_timeout(signum, frame):
    return
    signal.signal(signal.SIGALRM, _handle_timeout)
    signal.alarm(seconds)

It will immediately return, and the following two lines will never be executed.

If TIMEOUT can be set then it's not a constant, and it should instead be a class level variable. Place it directly after the class definition, then you can directly assign it easily without needing to resort to the global keyword. You also don't need to pass it as an argument to @timeout, you can just use NetworkStatus.timeout directly there.

class NetworkStatus(object):

    timeout = 2

In online you're returning if self.online(**kw) is True, but this is the same as returning self.online(**kw) already is True. And check only returns true or false. There's no reason to have two separate functions here at all, just rename the vague check function to is_online and you're returning a boolean directly from there. Then you also have offline which makes little sense. If you're concerned about readability, then if not network.is_online() is plenty readable without having a specific function.

Further down you also have if_online and if_offline... what are these wrappers for? They seem overly generic to achieve one small goal (only run a function if the network is online/offline. It'd make more sense to have these specific to the function, rather than passing the function as an argument. Did you have a lot of functions that needed this construct? It doesn't look like you've used it here.

I'd rename mainloop to just run, or run_test. mainloop is a generic name that tells us nothing. Also, no need to store the result in up. You can evaluate directly:

if self.is_online():

Now that I'm here I notice that you never passing any keyword arguments as kw**. Indeed you never use them, instead just using string literals:

socket.gethostbyname(kw.get('remote_server', 'www.google.com'))

This is very bad practice! Those values should be constants or class attributes. You should also remove all the unnecessary kw** parameters. Why let the user pass in values that'll never be used for anything?


Stack implementation in Java Here's my implementation of a Stack in Java, all feedback welcome.

import java.util.Iterator;

public class Stack<T> implements Iterable<T> {
    private Node head = null;
    private int size;

    @Override
    public Iterator<T> iterator() {
        return new StackIterator();
    }

    private class Node<T>{

        public Node(T data){
            this.data = data;
            this.next = null;
        }
        T data;
        Node next;
    }

    private class StackIterator implements Iterator<T>{

        private Node current = head;
        @Override
        public boolean hasNext() {
            return current != null;
        }

        @Override
        public T next() {
            T item = (T) current.data;
            current = current.next;
            return item;
        }
    }

    public Stack(){
        size = 0;
        head = null;
    }

    public boolean isEmpty(){
        return head == null;

    }

    public void push(T item){
        Node p = new Node(item);
        if(head == null){
            head = p;
            size++;
            return;
        }
        p.next = head;
        head = p;
        size++;

    }

    public T pop(){
        Node current = head;
        if(current.next == null){
            head = null;
        }else{
            try {

                head = head.next;
                size--;
            }catch (Exception e){
                System.out.println("Popping off an empty stack:" + e);
            }
        }

        return (T)current.data;
    }

    public void trace(){
        Node current = head;
        while(current != null){
            System.out.println(current.data);
            current = current.next;
        }
    }
}

}catch (Exception e){
    System.out.println("Popping off an empty stack:" + e);
}

Why? You should be able to know if your stack is empty before you even try to pop it. And in that case, any consumer of that Stack class is going to expect an exception to be thrown.

Actually, something's fishy about that catch block.

Stack<int> foo = new Stack<int>();
foo.pop();

What does that do? head is null, you've made sure... twice.

Once here:

private Node head = null;

And another here:

public Stack(){
    size = 0;
    head = null;
}

The constructor seems redundant.

Now, with head being null, foo.pop() will throw a rather surprising NPE, despite that try/catch block seemingly handling the "stack is empty!" case.

You should avoid having that NPE thrown, and instead throw an EmptyStackException, for which your consumer code will be thankful.


Resolve (domainless) machine name from DNS alias So I have a URI with a DNS alias, and I need to dynamically replace the alias with the actual machine. I started out with this:

var builder = new UriBuilder("http://server-alias/path/to/resource");
builder.Host = Dns.GetHostEntry(builder.Host).HostName;
return builder.Uri; // http://server1.mydomain.net/path/to/resource

However, what I need is the URI with just the machine name, http://server1/path/to/resource. Of course I can use this, but string manipulation seems seems a bit kludgey:

builder.Host = Dns.GetHostEntry(builder.Host).HostName.Split(new[] { '.' }, 2)[0];

Is there a better way to get just the machine name from a DNS alias, without the domain?

However, what I need is the URI with just the machine name, http://server1/path/to/resource. Of course I can use this, but string manipulation seems seems a bit kludgey:
builder.Host = Dns.GetHostEntry(builder.Host).HostName.Split(new[] { '.' }, 2)[0];

Also this seems to be a little bit strange, I don't know another way, only a cleaner way.
private Uri GetUriFromAlias(String alias)
{
    UriBuilder builder = new UriBuilder(alias);
    IPHostEntry entry = Dns.GetHostEntry(builder.Host);

    builder.Host = GetMachineName(entry);
    return builder.Uri;
}
private String GetMachineName(IPHostEntry entry)
{
    return entry.HostName.Split(new[] { '.' }, 2)[0];
}


Handling failed catches I know that is generally expected that you should not swallow exceptions. In this code, an Infragistic's UltraWinGrid is being configured. Is there a better way to handle the failed catches?  Is this an exception to the rule?

private void HideExcludedColumns(UltraGridBase grid)
{
    if (_scExclusions == null) return;
    foreach (var strKey in _scExclusions)
    {
        //Set the appropriate column of the grid to hidden.
        foreach (var band in grid.DisplayLayout.Bands)
        {
            try
            {
                band.Columns[strKey].Hidden = true;
                break;
            }
            catch { }   //go to the next band.
        }
    }
}

I agree with almaz - a blanket catch is bad juju in just about every case. Need to know what exception(s) wind up being there. If the column doesn't exist, there should be a better, non-exceptional way of doing that:

    private void HideExcludedColumns(UltraGridBase grid)
    {
        if (_scExclusions == null) return;
        foreach (var strKey in _scExclusions)
        {
            //Set the appropriate column of the grid to hidden.
            foreach (var band in grid.DisplayLayout.Bands)
            {
                var columnIndex = band.Columns.IndexOf(strKey);
                if (columnIndex > -1)
                {
                    band.Columns[columnIndex].Hidden = true;
                    break;
                }
            }
        }
    }


Add neighboring stones of the same color to form chains, with logging for isolated stones I'm trying to check four things, one of those things might happen, maybe two, three or four things, but if none of them occur, then I want to run some code. I can't do an else statement because that only applies to one and I can't do else if because more than one can happen.

At the moment I've resorted to this:

function checkChains(x, y, color, array)
{
    var count = 0;

    if ((x + 1) != (boardSize) && (stones[x + 1][y] == color))
    {
        addToChain(x, y, x + 1, y, array);
    }
    else
    {
        count++;
    }

    if ((x != 0) && (stones[x - 1][y] == color))
    {
        addToChain(x, y, x - 1, y, array);
    }
    else
    {
        count++;
    }

    if ((y + 1) != (boardSize) && (stones[x][y + 1] == color))
    {
        addToChain(x, y, x, y + 1, array);
    }
        else
    {
        count++;
    }

    if ((y != 0) && (stones[x][y - 1] == color))
    {
        addToChain(x, y, x, y - 1, array);
    }
    else
    {
        count++;
    }

    if (count == 4)
    {
        console.log("none!");
    }
}

It's pretty ugly, can this be tidied?

You don't really need your elses. Move count++ to the if, and change your last if to !count.
Rather than using !=, you should really use < and <=. This means you you can use x + 1 < boardSize rather than (x + 1) != (boardSize).
You should learn operator precedence, == happens before &&. And so you can remove even more parentheses.

And so you can use:

function checkChains(x, y, color, array)
{
    var count = 0;

    if (x + 1 < boardSize && stones[x + 1][y] == color)
    {
        addToChain(x, y, x + 1, y, array);
        count++;
    }

    if (x > 0 && stones[x - 1][y] == color)
    {
        addToChain(x, y, x - 1, y, array);
        count++;
    }

    if (y + 1 < boardSize && stones[x][y + 1] == color)
    {
        addToChain(x, y, x, y + 1, array);
        count++;
    }

    if (y > 0 && stones[x][y - 1] == color)
    {
        addToChain(x, y, x, y - 1, array);
        count++;
    }

    if (!count)
    {
        console.log("none!");
    }
}


Simple address book in C++ - follow-up After posting my original address book program I took the responses and edited my code. I have used every suggestion given, the code works great, and now I am looking for further review and suggestions from the Code Review community.

Parent class (Person):

//parent class
#ifndef PERSON
#define PERSON
#pragma once

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

class Person
{
public:
    //input person variable
    std::istream& read(std::istream&, Person &p1);
    //output person variable
    std::ostream& print(std::ostream& os, const Person &p1);
    //comparison operator
    bool operator<(const Person&) const;
private:
    std::string name;
    std::string address;
};

#endif

Person member functions:

#include "Person.h"

//input for address book person element
std::istream& Person::read(std::istream& in, Person &p1) {
    std::cin >> p1.name;
    std::cin.ignore();
    std::getline(std::cin, p1.address);
    return in;
}

//operator for address book person element
std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address << "\n\n";
    return os;
}

bool Person::operator<(const Person& other) const {
    return name < other.name;
}

Child class (Address Book):

//child class
#ifndef ADDRESS_BOOK
#define ADDRESS_BOOK
#pragma once

#include "Person.h"

class Address_book : public Person
{
    friend std::istream& operator>>(std::istream&, Address_book&);
    friend std::ostream& operator<<(std::ostream&, Address_book&);
public:
    //set vector size to match appropriate number of entries
    void num_of_ent(int, Address_book&);
    void Address_book::sort(Address_book&,int);

private:
    std::vector<Person> add_book;
};

#endif

Address book member functions:

#include "Address_book.h"

//sets vector size, creates person elements of vector
void Address_book::num_of_ent(int entries, Address_book& abook) {
    for (int i = 0; i < entries; ++i) {
            Person p;
        abook.add_book.push_back(p);
    }
}

//fills address book
std::istream& operator>>(std::istream& in, Address_book& abook) {
    for (int i = 0; i < abook.add_book.size(); ++i) {
        abook.add_book[i].read(in, abook.add_book[i]);
    }
    return in;
}

//sort address book
void Address_book::sort(Address_book& abook) {
    std::sort(abook.add_book.begin(), abook.add_book.end());
}
//prints contents of address book
std::ostream& operator<<(std::ostream& os, Address_book& abook) {
    for (int i = 0; i < abook.add_book.size(); ++i) {
        abook.add_book[i].print(os, abook.add_book[i]);
    }
    return os;
}

Main:

#include <iostream>
#include <vector>
#include <string>
#include "Address_book.h"
#include "Person.h"

void pause();

int main() {

    int entries = 0;

    std::cout << "Enter number of entries to go in addressbook: ";
    std::cin >> entries;

    Address_book abook1;
    abook1.num_of_ent(entries, abook1);

    //input into addressbook
    std::cout << "Enter the name, followed by the address:\n";
    std::cin >> abook1;
    //sort addressbook
    abook1.sort(abook1, entries);

    //output addressbook
    std::cout << "\n";
    std::cout << abook1;

    pause();
}

void pause() {
    std::string pause;
    std::cout << "Press any key followed by enter to continue...";
    std::cin >> pause;
}

Misuse of inheritance

The huge, glaring problem I see with this code can be shown in one line:

class Address_book : public Person

This looks to me like a completely broken architecture. As a rule, public inheritance should follow the Liskov Substitution Principle (LSP), and this isn't even close1.

Before Barbara Liskov formulated the substitution principle, the usual phrasing was that public inheritance should reflect an "Is-a" relationship. The LSP attempts to...tighten that a little to deal with a few odd cases like circles vs. ellipses, where you can say a circle is a specific case of an ellipse, but (since you can't always substitute a circle where an ellipse is required) you shouldn't normally use public inheritance.

In this case, however, we don't need the more careful formulation. An address book isn't a person, and isn't really even anything like a person. We don't need to look much further to see that this is use (abuse?) of inheritance is problematic at best.

Consistent interface

I'd also change the interface a little bit. Right now, you have read and write member functions for Person, and operator<< and operator>> for an address book.

I'd prefer to change this so the public interface is more consistent. Since the (more or less) standard way to do formatted I/O is to use operator<< and operator>>, I'd use those for both classes.

Resizing

Right now, to resize your vector, you have code like:

//sets vector size, creates person elements of vector
void Address_book::num_of_ent(int entries, Address_book& abook) {
    for (int i = 0; i < entries; ++i) {
            Person p;
        abook.add_book.push_back(p);
    }
}

I'm not fond of the name (at first blush, the name sounds more like it should be retrieving the current number of entries). I'd also prefer to use vector's resize() member instead. Finally, a member function that's going to modify an object should normally modify the object of which it's a member, not have some other object passed to it to modify.

void Address_book::resize(int entries) {
    add_book.resize(entries);
}

Size type

That brings us to the next point: the type for the size of a container is preferably container_type::size_type or if that's not easily available, you can usually use size_t as a fair approximation. So that means the resize should really be changed to:

void Address_book::resize(size_type entries) {
    add_book.resize(entries);
}

...where size_type is something like:

typedef vector<Person>::size_type size_type;

...or:

using size_type = vector<Person>::size_type;

formatting of output

A fairly minor point is that by convention writing an object to the stream should only write the content of that object to the stream, not write a delimiter that separates that object from the next. For example, here:

//operator for address book person element
std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address << "\n\n";
    return os;
}

...we really want to get rid of the trailing new-lines, and leave that to the client code.

std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address;
    return os;
}

Then we might use it something like this:

std::copy(my_book.begin(), my_book.end(),
          std::ostream_iterator<Person>(std::cout, delimiter));

...and to get the same format, we'd have delimiter = "\n\n";, but if we wanted the output single-spaced, it'd be "\n", and so on.

1. Nor do I see any indication that this might be an exception to the rule.