Editing /etc/network/interfaces I am trying to change static to dhcp in the /etc/network/interfaces file and vice versa using Python.

def setMode(mode,port,interfaces):
    #If the port's mode is not configured in the interface file, add a line for it, and return
    if "iface "+port+" inet " not in interfaces:
        newLine="\niface "+port+" inet "+mode
        interfaces="".join([interfaces,newLine])
        return interfaces

    #split the string by port to get to the line containing the port's ip mode
    #then split the third element of the resulting list by newline to isolate the end of that line
    split1=interfaces.partition(port+" inet ")
    split2=split1[2].partition("\n")
    #split the isolated line by space to get each word in it. the last element in the resulting list will be the current ip mode
    line=split2[0].split(" ")
    #if the current mode matches the new mode, return
    if line[len(line)-1] == mode:
        line=" ".join(line)
        split2="".join([line,split2[1],split2[2]])
        interfaces="".join([split1[0],split1[1],split2])
        return interfaces
    #otherwise set the last element to the new mode
    line[len(line)-1]=mode
    #join the line and lines back to recreate the full string
    line=" ".join(line)
    split2="".join([line,split2[1],split2[2]])
    interfaces="".join([split1[0],split1[1],split2])

    return interfaces

This is the code I am taking over with.  I am using str.partition() to achieve this.  mode == static or dhcp, port = eth8 or eth0, etc, interfaces = copy of interfaces file.

/etc/network/interfaces looks like this:

auto lo
iface lo inet loopback

auto eth8
iface eth8 inet static
address 10.8.100.36
netmask 255.255.0.0
gateway 10.8.1.1

The splitting / partitioning makes this very complicated and hard to follow. Instead of splitting and then putting the pieces back together, it would be vastly simpler to replace with a regex, for example:

import re

def set_mode(mode, iface, interfaces):
    """
    Set the mode for the specified interface
    :param mode: 'dhcp' or 'static'
    :param iface: name of the interface, for example eth1, eth2, ...
    :param interfaces: content of /etc/network/interfaces
    :return: updated content, to be saved in /etc/network/interfaces
    """
    iface_prefix = 'iface {} inet '.format(iface)
    iface_entry = iface_prefix + mode
    if iface_prefix in interfaces:
        interfaces = re.sub(iface_prefix + r'.*', iface_entry, interfaces)
    else:
        interfaces += '\n' + iface_entry
    return interfaces

For your sample file, the following gives me the same output with the original method and this new one:

with open('./interfaces') as fh:
    content = fh.read()
    print set_mode('static', 'eth8', content)
    print '---'
    print set_mode('dhcp', 'eth8', content)
    print '---'
    print set_mode('dhcp', 'eth1', content)

Coding style

The original code has several issues that violate PEP8, the official coding style guide of Python:

Use snake_case for function and variable names (that's why I renamed setMode to set_mode
Put spaces around operators: a=something instead of a = something
A docstring is strongly recommended for the set_mode function


C++11 Singly Linked List with raw pointers Regarding C++, I have an experience in writing only short non-object-oriented programs and competitive programming challenges.

I would like to get your feedback about my C++ style, design decisions, and overall quality.

The below is the implementation of a Singly Linked List containing a multiset of values of type T. Type T is required to provide equal to operator. The class supports the following operations:

push a new value to the front of the list
push a new value to the end of the list
remove the first encountered occurrence of a given value
check if the list contains a given value
get the number of elements in the list
print the list in a pretty format
swap two lists

For the design, I decided to use both head and tail pointers to make both inserts constant time operations. For now, all pointers are raw pointers. The Node class is the internal class of LinkedList and an instance of LinkedList is responsible to delete its nodes. I decided to do that because this allows freeing the memory iteratively instead of recursively, which could cause problems if stack size is limited.

/* Singly Linked List by @pkacprzak */

#ifndef LINKED_LIST_HH__
#define LINKED_LIST_HH__

#include <ostream>

template<typename T>
class LinkedList {
public:
    LinkedList() {}

    LinkedList(const LinkedList& x) {
        auto cur = x.head;
        while (cur != nullptr) {
            pushBack(cur->val);
            cur = cur->next;
        }
    }

    LinkedList& operator=(const LinkedList& rhs) {
        LinkedList tmp(rhs);
        swap(*this, tmp);
        return *this;
    }

    ~LinkedList() {
        auto cur = head;
        while (cur != nullptr) {
            auto tmp = cur;
            cur = cur->next;
            delete tmp;
            tmp = nullptr;
        }
    }

    void pushFront(T val) {
        auto node = new Node(val);
        node->next = head;
        if (head == nullptr) {
            tail = node;
        }
        head = node;
        ++sz;
    }

    void pushBack(T val) {
        auto node = new Node(val);
        if (head == nullptr) {
            head = node;
        } else {
            tail->next = node;
        }
        tail = node;
        ++sz;
    }

    bool contains(T val) const {
        auto cur = head;
        while (cur != nullptr) {
            if (cur->val == val) {
                return true;
            }
            cur = cur->next;
        }
        return false;
    }

    void removeValue(T val) {
        if (head == nullptr) return;
        if (head->val == val) {
            auto cur = head;
            head = head->next;
            delete cur;
            if (head == nullptr) {
                tail = nullptr;
            }
            --sz;
        } else {
            auto cur = head->next;
            auto prev = head;
            while (cur != nullptr) {
                if (cur->val == val) {
                    prev->next = cur->next;
                    if (cur == tail) {
                        tail = prev;
                    }
                    delete cur;
                    --sz;
                    return;
                }
                prev = cur;
                cur = cur->next;
            }
        }
    }

    int size() const {
        return sz;
    }

    friend std::ostream& operator<<(std::ostream& os, const LinkedList& x) {
        auto cur = x.head;
        os << "[";
        while (cur != nullptr) {
            os << cur->val;
            if (cur != x.tail) {
                os << ",";
            }
            cur = cur->next;
        }
        os << "]";
        return os;
    }

    friend void swap(LinkedList& a, LinkedList& b) {
        std::swap(a.head, b.head);
        std::swap(a.tail, b.tail);
    }

private:
    struct Node {
        Node(T val_) : val(val_) {}
        T val;
        Node* next = nullptr;
    };
    Node* head = nullptr;
    Node* tail = nullptr;
    int sz = 0;
};
#endif

Item 1. swap does not handle sz member

Item 2.
No move constructor or operator= (no move semantic at all)

Item 3. Functions like contains get parameters by value - which is Ok for primitive types T and could be expensive for complex types

Item 4. No iterators - so not usable with algorithms. And also implementing something like remove_all is almost impossible - no indication of the remove success (except comparing size before and after), no possibility to continue scanning from the last location...

I suggest to take the std::forward_list http://en.cppreference.com/w/cpp/container/forward_list and try to emulate its API a lot closer then current attemp. Or justify your design choices explaining in more details why you think your change in the standard list API is superior to the stl approach.

Generally I like how the code looks and compiles. :)


Pi-calculating program I saw this question and answer about calculating pi on Stack Overflow, and I decided to write my own program for calculating pi. I used Python and only integers (I didn't want to use floating point numbers), and used the Gauss–Legendre algorithm because it was the simplest to implement (I considered using the Borwein's algorithm, but I didn't want to calculate third roots of numbers, and the Chudnovsky algorithm seemed a little complicated, although maybe I'll give it a try). I would like to know how efficient are the algorithms above, in \$\mathcal{O}(n)\$ (n is the number of digits to calculate), and how efficient is my program?

# Calculate next square root approximation of the number.
def calculate_next_square_root(number, square_root):
    next_square_root = ((number / square_root) + square_root) / 2
    return next_square_root

# Calculate the square root of the number.
def calculate_square_root(number, digits, add):
    # Start with 1, followed by (digits + add) zeros.
    square_root = 1 * (10 ** (digits + add))
    # Calculate next square root approximation of the number.
    next_square_root = calculate_next_square_root(number=number, square_root=square_root)
    while (next_square_root != square_root):
        # Replace square root with next square root.
        square_root = next_square_root
        # Calculate next square root approximation of the number.
        next_square_root = calculate_next_square_root(number=number, square_root=square_root)
    return square_root

# Calculate next pi approximation.
def calculate_next_pi(a, b, t, digits, add):
    next_pi = ((10 ** (digits + add)) * ((a + b) ** 2)) / (4 * t)
    return next_pi

# Calculate pi to 5,000 digits.
digits = 5000
add = 500
a = 10 ** (digits + add)
b = calculate_square_root(number=(10 ** ((digits + add) * 2)) / 2, digits=digits, add=add)
t = (10 ** ((digits + add) * 2)) / 4
p = 1
pi = -1 # pi must be different than next_pi
next_pi = calculate_next_pi(a=a, b=b, t=t, digits=digits, add=add)
n = 0
while (next_pi != pi):
    pi = next_pi
    next_a = (a + b) / 2
    next_b = calculate_square_root(number=a * b, digits=digits, add=add)
    next_t = t - (p * ((a - next_a) ** 2))
    next_p = 2 * p
    a = next_a
    b = next_b
    t = next_t
    p = next_p
    next_pi = calculate_next_pi(a=a, b=b, t=t, digits=digits, add=add)
    n += 1

# Remove the last 500 digits.
pi /= (10 ** add)

# Print the results.
print pi
print n

My program takes 52 lines of code (including comments), and I'm not interested in programs which take too many lines to implement, up to 100 or 200 lines are fine. I checked and it takes about one second to calculate 5,000 digits of pi (with add = 500), and a little less than two minutes to calculate 50,000 digits (with add = 5000). With 5,000 digits it took 14 iterations, and with 50,000 digits 17 iterations, but I didn't count how many iterations it took to calculate the square roots. Is my program efficiently using the Gauss–Legendre algorithm, or can it be made more efficient?

By the way, add doesn't have to be 500, even 4 or 5 is enough. But I wanted to make sure all the digits of pi are correct (and they are). With add = 0 the last 2 or 3 digits are not correct.

I took the square root program from a program I wrote in 2013, but I don't know the name of the algorithm I used and whether it's the most efficient algorithm or not.

Update: I remind you my question: I would like to know how efficient are the algorithms above, in \$\mathcal{O}(n)\$ (n is the number of digits to calculate), and how efficient is my program? I prefer not to import modules, but use pure Python (either 2 or 3) without any import. And of course if we start with a number closer to the square root (such as a or b) then it will take fewer iterations to calculate the square root.

This is a bit hard to answer, but let's try nevertheless:

From a computational point of view, using a scripting language like python for iterative algorithms doesn't make the least sense -- your PC is going to be far more occupied by translating your python code to executable structures than it will be with executing the core algorithm. For example, every number in python (assuming a CPython implementation of the runtime environment) is an object -- which makes number-number multiplication far more expensive than the actual multiplication. Now, I assume your question is "if I did this in a compiled language, with fixed length numerical types, would this algorithm perform optimally"?
approximating a square root in software is bound to be horrendously expensive compared to just calculating the floating point square root; modern general purpose CPUs even have instructions that can take the square root of several numbers at once, in just a few cycles!
I just have to stress this once more: "efficient" doesn't really make sense in this context, without actually specifying your measurement for efficiency. If your measurement is "how long does it take to have N significant digits", most probably even the most naive C implementation would beat the hell out of your Python program -- just because python is such a capable scripting language, which requires a lot of indirection. You could specify things like "minimum number of multiplications plus additions", but you'd totally neglect possibly different cost for these operations, and, much worse, memory movement complexity etc. Now, if you're actually into this kind of discussion, think about what kind of architecture you actually try to optimize for: A single threaded computer with uniform multiply, add, memory address loading and storage costs has never really existed; the norm today are hyperthreaded multicore processors with a limited number of SIMD registers and multilevel caches, which can make algorithms which "look good on paper" perform terribly because they just misuse the capabilities of the architecture, whilst stupid implementation might work exceptionally well because of possible compile- and execution-time optimization.


Reading a file and storing the contents in an array This is my code for reading a file with a delimiter.  Any suggestions to help improve my code efficiency?

I am not satisfied with using array data structure.  Can any other data structure be used instead of an array?

bool read_file(char *p_file_name,char *file_content[FILE_NO_OF_ROWS][FILE_NO_OF_COL],const char *delimiter, int& token_count )
{

   using namespace std;
   ifstream file_is_obj;
   file_is_obj.open(p_file_name,ios::in);

   string buffer,token;
   int token_pos;
   token_count=0;

   if( file_is_obj.is_open())
   {
      while( !file_is_obj.eof() )
      {
         if(token_count >= FILE_NO_OF_ROWS)
         {
            break;
         }

         buffer.clear();

         token_pos=0;

         std::getline(file_is_obj,buffer);
         for( int iter=0;  token_pos  != -1 || iter >= FILE_NO_OF_COL ; iter++)
         {
            token.clear();

            token_pos=buffer.find(delimiter,0);
            token=buffer.substr(0,token_pos);
            if( !token.empty() )
            {
               buffer.erase(0,token_pos+strlen(delimiter));
               file_content[token_count][iter] = new char[token.length()+1] ;

               memset(file_content[token_count][iter],'\0',(token.length()+1)*sizeof(char));
               token.copy( file_content[token_count][iter], token.length(), 0);
            }
         }
         token_count++;
      }
      token_count-=1; // Token incremented for reading end of file
   }
   else
   {
      printf("\nFile not found (or) Error in opening the file");
      return false;
   }

   return true;
}

Some comments:

Use std::vector instead of an inflexible array.  You're writing in C++, so you really ought to use the power of that language to write better code.
Don't use memset.  It's generally wise to avoid mixing old-style C library functions such as memset, malloc and so forth, with C++.  Strange things can happen and there's usually a better C++ way to do things without those.
Instead of using string.find, consider using other techniques such as the ones described here to split a line into its constituent pieces.
Rather than printing an error message, throw an exception.  That way your code can be reused in situtations in which std::cout is not present or not visible.
Do NOT embed using namespace std; into code like that!  It's extremely bad practice

EDIT:
Here's a quick alternative using a variation of the string split I pointed to in point #3.

#include <iostream>
#include <fstream>
#include <string>
#include <vector>

template <typename Container>
Container& split(Container& result,
  const typename Container::value_type& s,
  const typename Container::value_type& delimiters)
{
  result.clear();
  size_t current;
  size_t next = -1;
  do {
    current = next + 1;
    next = s.find_first_of( delimiters, current );
    result.push_back( s.substr( current, next - current ) );
  } while (next != Container::value_type::npos);
  return result;
}

int main(int argc, char *argv[])
{
    if (argc < 2) {
        std::cout << "Usage readarray filename\n";
        return 0;
    }
    const std::string delimiter{","};
    std::vector<std::vector<std::string> > v;
    // fetch the file into v
    std::string line;
    std::vector<std::string> fields;
    for( std::ifstream in(argv[1]); std::getline(in, line); ) {
        v.push_back(split(fields, line, delimiter));
    }
    // now print the results
    for (const auto &i : v) {
        for (const auto &j: i)
            std::cout << "[" << j << "]";
        std::cout << '\n';
    }
}

This uses C++11, but could easily be adapted for old compilers.  Based on this input file:

alpha,frank,ruby,diamond
beta,joseph,emerald,circle
gamma,may,onyx,triangle

The code produces this output:

[alpha][frank][ruby][diamond]
[beta][joseph][emerald][circle]
[gamma][may][onyx][triangle]


Java function to read a CSV file I have a java function that reads a csv and returns its content as a Vector<Vector<String>>.

The function seems to work as I need it, but my gut feeling says it could be better (never mind the fact that it is declared throws Exception).

So here it is:

private static Vector<Vector<String>> readTXTFile(String csvFileName) throws Exception {

  BufferedReader stream = new BufferedReader(
                          new InputStreamReader(
                          new FileInputStream(csvFileName)));

  Vector<Vector<String>> csvData = new Vector<Vector<String>>();

  String line;
  while ((line = stream.readLine()) != null) {

    csvData.add(new Vector<String>() );

    String[] values = line.split(",");

    for (int v=0; v<values.length; v++) {
       csvData.get(csvData.size()-1).add(values[v]);
    }
  }

  return csvData;
}

Background

Ultimately, the CSV Data will be used to fill a JTable. I could have used an String[][] for the data too, but it seemed that constructing a dynamic String[][] from a csv file would have been even more combersome (although I stand ready to be corrected on this, too).

The code can be simplified and improved in several ways, and the inner loop can be made tighter. Let me show you how:

private static List<List<String>> readTXTFile(String csvFileName) throws IOException {

    String line = null;
    BufferedReader stream = null;
    List<List<String>> csvData = new ArrayList<List<String>>();

    try {
        stream = new BufferedReader(new FileReader(csvFileName));
        while ((line = stream.readLine()) != null) {
            String[] splitted = line.split(",");
            List<String> dataLine = new ArrayList<String>(splitted.length);
            for (String data : splitted)
                dataLine.add(data);
            csvData.add(dataLine);
        }
    } finally {
        if (stream != null)
            stream.close();
    }

    return csvData;

}

A method should throw an exception as specific as possible, here it's better to use IOException instead of simply Exception
Whenever possible, return interface types instead of concrete classes. Using List is preferred to using Vector, this allows you the flexibility to change the implementation of the collection later
Talking about collections: Vector is thread-safe and synchronized in many of its public methods, that can cause a performance hit. If you don't need that, it's better to use ArrayList, it's a drop-in replacement and won't incur in the performance penalty of synchronization.
There's a simpler way to instantiate a BufferedReader, using a FileReader instead of using an InputStreamReader plus a FileInputStream. Besides, FileReader will take care of pesky details regarding character encoding
There's a simpler way to add elements to the last List added to csvData, as shown in the code
There's a simpler way to iterate through the String[] returned by split(), using an enhanced loop
Don't forget to close your streams! preferably inside a finally block

If you don't need to add more elements to the returned List<List<String>>, you can use the following alternate version. It's faster because it avoids copying the String[] of split elements, but it won't allow adding new elements, as per the contract of asList():

private static List<List<String>> readTXTFile(String csvFileName) throws IOException {

    String line = null;
    BufferedReader stream = null;
    List<List<String>> csvData = new ArrayList<List<String>>();

    try {
        stream = new BufferedReader(new FileReader(csvFileName));
        while ((line = stream.readLine()) != null)
            csvData.add(Arrays.asList(line.split(",")));
    } finally {
        if (stream != null)
            stream.close();
    }

    return csvData;

}


Checking if website exists and adding +1 I am trying to find if a subwebsite exists in sharepoint, and if it does, then add +1.

I think the loop can be improved.

// subsiteName will be like "FruitCrateOfCompanyX"

public static string FormatMyHomieURL(string subsiteName, string mainSite)
{
    if (!mainSite.EndsWith("/"))
        mainSite = mainSite + "/";

    string temp = mainSite + subsiteName;
    int i = 0;

    while (SubwebExists(temp))
    {
        i++;
        temp = mainSite + subsiteName + i;

    }

    return subsiteName + i;    // I only need to return subSiteName which will be used later on by web services to create a unique subweb
}

public static bool SubwebExists(string url)
{
    bool exists = false;
    Uri u = new Uri(url);
    using (SPSite site = new SPSite(url))
    using (SPWeb web = site.OpenWeb(u.AbsolutePath.ToString(), true))
    {
        exists = web.Exists;
    }

    return exists;
}

Eliminate potential logic error in return value

I'm assuming the purpose of this code is to let you provide the name you want for your subsite, and the code will give you an available name based on whether the provided name already exists. It does this by appending/incrementing a suffix to the provided name whenever there is a naming collision. If this is the case, your FormatMyHomieURL() function should return temp sans the mainSite prefix, not subsiteName  + i.

To walk through an example briefly, let's say you want to name a subsite AwesomeWeb. The code checks and AwesomeWeb doesn't exist, so it returns subsiteName + i which resolves to AwesomeWeb0. You then go ahead and create a subsite named AwesomeWeb0 and everyone is happy. One month later, you come along and decide to make a subsite named AwesomeWeb. The code checks and sure enough, AwesomeWeb doesn't exist (since you created AwesomeWeb0 last time), so the code returns subsiteName + i which again resolves to AwesomeWeb0. Now you're going to run into a naming collision.

Reduce the number of instantiated SPSite objects

You are only ever dealing with a single SPSite object in this code (the parent site that you are checking for subsites). Therefore, you don't really need to create and dispose of a new SPSite object in every iteration of your loop just to grab a subsite from it.

I recommend refactoring your code to instantiate the SPSite object outside of SubwebExists() and pass it in to SubwebExists() as a parameter, along with the name of the target subweb temp.


Adapter class to display songs in an Android ListView I am learning Android with Udacity nanodegree (having no support) and this is my assignment of music app. It is just an interface and no music I have to add. So far so the app is running without any error but I wold like to get expert feedback to write code in better and optimized way to achieve the same result.
My entire project is in my Bitbucket repo.
SongAdapter.java
public class SongAdapter extends ArrayAdapter<Song> {

    int     playingIndex = -1;
    boolean currentTrack = false;

    public SongAdapter(@NonNull Context context, int resource, @NonNull List<Song> objects) {
        super(context, resource, objects);
    }

    @NonNull
    @Override
    public View getView(final int position, @Nullable View convertView, @NonNull ViewGroup parent) {

        View             playlistItemView = convertView;
        final ViewHolder holder;

        if (playlistItemView == null) {
            playlistItemView = LayoutInflater.from(getContext()).inflate(R.layout.playlist_item, parent, false);

            holder = new ViewHolder();

            holder.albumCoverThumbnail = playlistItemView.findViewById(R.id.playlist_album_thumbnail);
            holder.songTitle = playlistItemView.findViewById(R.id.playlist_song_title);
            holder.songAlbumTitle = playlistItemView.findViewById(R.id.playlist_song_album_title);
            holder.songArtist = playlistItemView.findViewById(R.id.playlist_song_artist);
            holder.songPlayButton = playlistItemView.findViewById(R.id.playlist_play_button);

            playlistItemView.setTag(holder);
        } else {
            holder = (ViewHolder) playlistItemView.getTag();
        }

        final Song currentSong = getItem(position);

        // set data to the list item
        assert currentSong != null;
        holder.albumCoverThumbnail.setImageResource(currentSong.getSongAlbumCoverId());
        holder.songTitle.setText(currentSong.getSongTitle());
        holder.songAlbumTitle.setText(currentSong.getSongAlbumTitle());
        holder.songArtist.setText(currentSong.getSongSingers());

        // check the play status of the song item
        if (playingIndex == position) {
            holder.songPlayButton.setImageResource(R.drawable.ic_pause_black_24dp);
        } else {
            holder.songPlayButton.setImageResource(R.drawable.ic_play_arrow_black_24dp);
        }

        // set song button action
        holder.songPlayButton.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {

                holder.songPlayButton.setImageResource(R.drawable.ic_pause_black_24dp);

                // pause current song on other click
                if (position == playingIndex) {
                    playingIndex = -1;
                } else {
                    playingIndex = position;
                    notifyDataSetChanged();
                }

                // pause play current track
                if (currentTrack) {
                    holder.songPlayButton.setImageResource(R.drawable.ic_play_arrow_black_24dp);
                    currentTrack = !currentTrack;
                } else {
                    holder.songPlayButton.setImageResource(R.drawable.ic_pause_black_24dp);
                }
                currentTrack = !currentTrack;
            }
        });

        return playlistItemView;
    }

    static class ViewHolder {
        ImageView   albumCoverThumbnail;
        TextView    songTitle;
        TextView    songAlbumTitle;
        TextView    songArtist;
        ImageButton songPlayButton;
    }
}

ListView has been deprecated, a better practice would be to use RecyclerView and RecyclerView.Adapter, for more information: Android RecyclerView


Parsing CSV to specific format I've written this code to read CSV files written in a specific format. I would like to gather some feedback on where it could be improved. I'm trying to get into the Test-Driven-Development ideology so any way I could make the code play better with unit tests would be of particular interest to me.

An aspect I am not overly pleased with is the switch statement in the MatchHeaders function. I did try using enum representing the property names but I ran into an issue with not having constant value, and frankly it did not make the code read cleaner.

I am more interested in being pointed at what to look at and where, rather than being given code.

public class Contact
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public DateTime DateOfBirth { get; set; }
    public string FullAddress { get; set; }
    public string PostCode { get; set; }
    public string TelephoneNumber { get; set; }
    public string EmailAddress { get; set; }
}

public interface ICsvReader
{
    List<Contact> ParseCsv(string filePath);
}

public class Reader : ICsvReader
{
    public List<Contact> ParseCsv(string filePath)
    {
        List<String[]> rows = GetRows(filePath);
        string[] headers = rows[0];

        return MakeContacts(headers, rows);
    }

    private List<String[]> GetRows(string filePath)
    {
        List<Contact> users = new List<Contact>();
        string[] lines = File.ReadAllLines(filePath);
        var csv = from line in lines
                    select (line.Split(',')).ToArray();

        return csv.ToList();
    }

    private List<Contact> MakeContacts(string[] headers, List<string[]> rows)
    {
        List<Contact> contacts = new List<Contact>();
        // If we didn't know the headers we could contain values with their properties in a collection
        // List<List<Dictionary<string, object>>> csv = new List<List<Dictionary<string, object>>>();
        for (int i = 1; i < rows.Count; i++)
        {
            string[] row = rows[i];
            Contact contact = new Contact();
            for (int e = 0; e < row.Length; e++)
            {
                string header = headers[e];
                string value  = row[e];

                contact = MatchHeader(contact, header, value);
            }
            contacts.Add(contact);
        }
        return contacts;
    }

    private Contact MatchHeader(Contact user, string header, string value)
    {
        switch (header.ToLowerInvariant())
        {
            case "firstname":
                user.FirstName = value;
                break;
            case "lastname":
                user.LastName = value;
                break;
            case "dateofbirth":
                user.DateOfBirth = DateTime.Parse(value);
                break;
            case "fulladdress":
                user.FullAddress = value;
                break;
            case "postcode":
                user.PostCode = value;
                break;
            case "telephonenumber":
                user.TelephoneNumber = value;
                break;
            case "emailaddress":
                user.EmailAddress = value;
                break;
            default:
                break;
        }
        return user;
    }
}

Few comments:

Make your code more abstract, use IEnumerable instead of List

 public interface ICsvReader
 {
    IEnumerable<Contact> ParseCsv(string filePath);
 }

Provide an asynchronous version

public interface ICsvReader
{
    IEnumerable<Contact> ParseCsv(string filePath);
    Task<IEnumerable<Contact>> ParseCsvAsync(string filePath);
}

Use StreamReader instead of File.ReadAllLines, it has an async version that you can use in your ParseCsvAsync method

using(var stream = new StreamReader(filePath)){
 // read all lines
}

Try to use the var keyword for local variables, it makes refactoring easier

var rows = GetRows(filePath);

Use the beauty of deferred execution in C#

private IEnumerable<Contact> MakeContacts(string[] headers, List<string[]> rows)
{
    for (int i = 1; i < rows.Count; i++)
    {
        string[] row = rows[i];
        Contact contact = new Contact();
        for (int e = 0; e < row.Length; e++)
        {
            string header = headers[e];
            string value  = row[e];

            contact = MatchHeader(contact, header, value);
        }
        yield return contact;
     }
}

You can convert ToList immediately without calling ToArray

  private IEnumerable<List<string>> GetRows(string filePath)
  {
    // why you creating this List here???
    List<Contact> users = new List<Contact>();
    lines = File.ReadAllLines(filePath);
    var csv = (from line in lines
              select line.Split(',')).ToList();

    return csv;
  }

Consider @Snowbody suggestions about using a Dictionary instead of a massive switch.


Comparing Equals() method from MSDN I've implemented the Equals() support for my class as follows:

public override bool Equals(object obj)
{
  return Equals(obj as TwoDPoint);
}

public bool Equals(TwoDPoint p)
{
  return ((object)p != null) && (x == p.x) && (y == p.y);
}

This obviously doesn't match the reference implementation on MSDN, here, as they add explicit checking for null before and after converting to a TwoDPoint, and they implement the actual equality test in each method (i.e. no call from one to the other).

I like my implementation as it's much more succinct. But I'm wondering - what have I missed? Am I losing (significant) performance with my version? Is there actually a bug in this approach?

I like my implementation as it's much more succinct

Which isn't necessarily a good thing: we care about readable code, not about oneliners. It means we don't use 5 lines to write what can be written in 1 line but it also means we don't cram 5 lines in 1 line just for the sake of it.

That being said: your implementation does the same as the example. I would also prefer your example since it keeps the equality logic in one method Equals(T) rather than duplicating it.

Some comments however:

There's no point in doing this cast: (object)p != null
You might as well implement IEquatable<T> MSDN
There should be no performance difference unless you have an unusual amount of null arguments to Equals(object) that will now go through the as statement before being caught by the null check
Equals(T) will probably be inlined anyway


Universally unique 64 bit long generator I'm trying to make an ID generator that generates 64 bit longs. The IDs should be globally unique even between two offline machines. This is the same idea as UUID (GUID), but UUID is 128 bit long and contains redundant (constant) bits (eg. it uses the millisecs passed since the year 1980 and has 5-8 versioning bits). I could use either the least or most significant 64 bits of a UUID, but because it contains these constant bits, it may not be unique enough.

So I made my own logic. It's important that it has to run everywhere, so I can't use MAC address, because it's not always accessible (eg. on Android). What do you think of my code?

public class UID {

private static long swRndVal;
private static Random random = new Random();
private static final long mstsigMask = 0b11111111_00000000_00000000_00000000_00000000_00000000_00000000_00000000L;
private static final long middleMask = 0b00000000_11111111_11111111_11111111_11111111_11111111_11111111_11111111L;
private static final long lstsigMask = 0b00000000_00000000_00000000_00000000_00000000_00001111_11111111_11111111L;

static {
    String s1 = System.getProperty("os.arch", "");
    String s2 = System.getProperty("os.version", "");
    String s3 = System.getProperty("java.vm.version", "");
    String s4 = System.getProperty("java.class.version", "");
    Random rand1 = new Random((s1+s2+s3+s4).hashCode());
    Random rand2 = new Random();
    swRndVal = rand1.nextLong() ^ rand2.nextLong();
}

public static long getUniqueLong() {
    //8 bits of random, 36 bits of millis (~800 days), 20 bits of nanos (~1ms)
    return ((random.nextLong() & mstsigMask) | (System.currentTimeMillis() & middleMask) | (System.nanoTime() & lstsigMask)) ^ swRndVal;
}
}

I would just use SecureRandom#nextLong(). SecureRandom will seed itself and shouldn't be the same between two similar computers at the same time. Also, it uses more bits of state than Random (128 vs 48).

I don't know exactly what computers you'll be running this on, but if it's something like a cluster of servers they'll probably have the exact same s1/s2/s3/s4. If they happen to start at the same time (plausible if two processes on two cores are launched by a parents at the same time), then that only leaves the 48 bits of random state.

If you use SecureRandom, you can just call it and immediately get a 64 bit UUID that will have as few collisions as you can possibly guarantee with two offline computers.


SICP Exercise 1.3: Sum of squares of two largest numbers out of three, Rust Version The exercise 1.3 of the book Structure and Interpretation of Computer Programs asks the following:

  Exercise 1.3.  Define a procedure that takes three numbers as arguments and returns the sum of the squares of the two larger numbers.

My answer is this:

#![feature(core)]
use std::cmp;

fn sum_square_largest(x:f64, y:f64, z:f64) -> f64 {
    match partial_min_three(x, y, z) {
        Some(a) =>  x * x + y * y + z * z - a * a,
        None => 0.0_f64/0.0_f64
    }
}

fn partial_min_three<T>(v1: T, v2: T, v3: T) -> Option<T> where T: PartialOrd<T> {
    match cmp::partial_min(v2, v3) {
        Some(x) => cmp::partial_min(v1, x),
        None => None
    }
}

Rust is the language I know less, I would really appreciate your advice.

I find it always pays to be tidy and conscientious about spacing. Here, I'd add spaces after the : in the argument declaration

// fn sum_square_largest(x:f64, y:f64, z:f64) -> f64 {
fn sum_square_largest(x: f64, y: f64, z: f64) -> f64 {

Next, I'd wrap the where clause onto the next line. where clauses often get large, so I find it best to let them have breathing room, and only put one type per line:

// fn partial_min_three<T>(v1: T, v2: T, v3: T) -> Option<T> where T: PartialOrd<T> {
fn partial_min_three<T>(v1: T, v2: T, v3: T) -> Option<T>
    where T: PartialOrd<T>
{

Checking for Some or None and doing something is a common pattern. In this case, you can use Option::and_then:

// match partial_min(v2, v3) {
//     Some(x) => partial_min(v1, x),
//     None => None
// }
partial_min(v2, v3).and_then(|x| partial_min(v1, x))

Programming is often about clarity in expressing your intentions, and I doubt that the majority of programmers immediately know what 0 / 0 is for floating point numbers. You should be explicit:

// None => 0.0_f64/0.0_f64
None => std::f64::NAN,

Again, we can use existing patterns to deal with checking for Some / None. This time, we can use Option::map:

// match partial_min_three(x, y, z) {
//     Some(a) => x * x + y * y + z * z - a * a,
//     None => std::f64::NAN,
// }
partial_min_three(x, y, z)
    .map(|a| x * x + y * y + z * z - a * a)
    .unwrap_or(std::f64::NAN)

I disagree with your choice of NaN as a magic value. Magic values drive me crazy, and Rust has great choices for avoiding them - Option and Result!

fn sum_square_largest(x: f64, y: f64, z: f64) -> Option<f64> {
    partial_min_three(x, y, z)
        .map(|a| x * x + y * y + z * z - a * a)
}

I'd probably create a tiny function for squaring. It's likely to get inlined, so I'm not worried about performance, just readability:

.map(|a| square(x) + square(y) + square(z) - square(a))

All together (playpen):

fn sum_square_largest(x: f64, y: f64, z: f64) -> Option<f64> {
    partial_min_three(x, y, z)
        .map(|a| square(x) + square(y) + square(z) - square(a))
}

fn partial_min_three<T>(v1: T, v2: T, v3: T) -> Option<T>
    where T: PartialOrd<T>
{
    partial_min(v2, v3).and_then(|x| partial_min(v1, x))
}

fn square(x: f64) -> f64 { x * x }

// Copied from the standard library as it is currently unstable
// and the beta doesn't allow unstable features
use std::cmp::Ordering;
fn partial_min<T: PartialOrd>(v1: T, v2: T) -> Option<T> {
    match v1.partial_cmp(&v2) {
        Some(Ordering::Less) | Some(Ordering::Equal) => Some(v1),
        Some(Ordering::Greater) => Some(v2),
        None => None
    }
}

fn main() {
    let z = sum_square_largest(1.0, 2.0, 3.0);
    println!("{:?}", z);
}


Count number of ways to achieve a sum The Question :

  Utkarsh being a very talkative child, was scolded by his teacher
  multiple times. One day, the teacher became very angry and decided to
  give him a very rigorous punishment. He made him stand on the school
  field which is X axis.

  Utkarsh initially stood at X = 0. The teacher asked him to run to X =
  N. But, to make the process quick, Utkarsh decided that he will make
  jumps of 2 or 3 steps only, i.e., from X = S he can jump to X = S+2 or
  X = S+3.

  Utkarsh decided that he will jump 2 steps with probability P/100 and
  jump 3 steps with probability 1-P/100.

  You need to find the probability that he will reach exactly on X = N.

  Constraints: 0 < N <= 106  0 <= P <= 100

  Input Constraints: The first line contains two integer N and P.

  Output Constraints: Your answer must contain exactly 6 digits after
  the decimal point.

The Solution

I applied DP using memoization using HashMap ,on large input it gives stackoverflow error at hashmap.containsKey line , please tell why it shows that error and a better way to do memoization here . i have faced this problem many times.

import java.util.HashMap;
import java.util.Scanner;

public class Utkarsh_and_Jumps {
    private static double p;
    private static HashMap<Integer, Double> dp;

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        int n = scanner.nextInt();
        p = scanner.nextInt();
        dp = new HashMap<Integer, Double>();
        System.out.println(String.format("%.6g%n", solve(0, n)));
    }

    private static double solve(int i, int n) {
        if (dp.containsKey(i)) {
            return dp.get(i);
        }
        if (i == n) {
            return 1;
        } else if (i > n) {
            return 0;
        }
        double a = solve(i + 2, n) * (p / 100);
        double b = solve(i + 3, n) * (1 - (p / 100));
        dp.put(i, a + b);
        return a + b;
    }
}

The Input :

29194 14

The Error

Exception in thread "main" java.lang.StackOverflowError
    at java.util.HashMap.hash(HashMap.java:338)
    at java.util.HashMap.containsKey(HashMap.java:595)
    at Utkarsh_and_Jumps.solve(Utkarsh_and_Jumps.java:24)
    at Utkarsh_and_Jumps.solve(Utkarsh_and_Jumps.java:33)

You get the StackOverflowError because the solve method is called recursively too many times for large inputs such as 29194.

The first time you call solve(0, ...), nothing is already computed, so this will recursively call:
 * solve(2, ...) and then solve(4, ...), and so on until it reaches 29194
 *solve(3, ...)and thensolve(6, ...), and so on until it reaches 29194

However, there's a simple trick to avoid all the recursion: iterate backwards. Call solve(n, ...) (yes, this will call solve(n+2) and solve(n+3), but those will finish fast without recurring). Then, call solve(n-1), then solve(n-2), and so on, until solve(0). Notice how this time all recursive calls will finish in one step, as the values will already be computed and available in dp.

So, before your call to solve(0, n), add the following code:

for (int i = n; i >= 0; i--) {
    solve(i, n);
}

and you will avoid the StackOverflowError.

Here it is in action: http://ideone.com/7Ou8qq

Note that this is the smallest change to your code that I could make to get it to run, so it's more like a hack. Optimally, you would change your algorithm to run in reverse order by default.


Searching for a string in a web page I have written the below code to search for a string in a web page. This is working fine but I need suggestions on improving this code to start using this. This is written to work under IE.

$(document).ready(function()
{
var NotFounds = [];
function countString(s) {
  var re = new RegExp(s, 'gi');
  var text = document.body.createTextRange().text;
  var matches = text.replace(/\s+/g, ' ').match(re);
  return matches? matches.length : 0;
}
// This function is the Sentence search function.
function doSearch(tex) {
    if (document.body.createTextRange) {
        var textRange = document.body.createTextRange();
        textRange.collapse(true);
        if(textRange.findText(tex)) {
            countString(tex) > 1 ? textRange.execCommand("BackColor", false, "#FF6600") : textRange.execCommand("BackColor", false, "yellow");
            textRange.collapse(false);
        }
        else
        {
            NotFounds.push(tex);
        }
    }
}
function trim(value) {
    var temp = value;
    var obj = /^(\s*)([\W\w]*)(\b\s*$)/;
    if (obj.test(temp)) { temp = temp.replace(obj, '$2'); }
    var obj = / +/g;
    temp = temp.replace(obj, " ");
    if (temp == " ") { temp = ""; }
    return temp;
 }
function SearchQueue(text)
 {
    if(text !== null)
    {
        if($('#NotFounds').size() > 0)
        {
            $('#NotFounds').detach();
        }
        text = text.replace(/^\s*$[\n\r]{1,}/gm, '');
        text = text.replace(/”/g, "\"");
        text = text.replace(/“/g, "\"");
        text = text.replace(/”/g, "\"");
        text = text.replace(/’/g, "\'");
        text = text.replace(/‘/g, "\'");
        text = text.replace(/–/g, "\-");
        text = text.replace(/—/g, "\-");
        text = text.replace(/–/g, "\-");
        var txt1=text;
        text = trim(txt1);
        var SearchItems = text.split(/\r\n|\r|\n/);
        document.designMode = "on";
        for(var i=0;i<SearchItems.length;i++)
        {
            var textTrim = $.trim(SearchItems[i]);
            doSearch(textTrim);
        }
        document.designMode = "off";
        if(NotFounds.length > 0)
        {
            var st = "Mismatched texts are...";
            for(var k=0;k<NotFounds.length;k++)
            {
                st = st + "
Mismatch "+ (k+1) + ". " + NotFounds[k] + "";
            }
            st = st + "";
            $('body').prepend('

');
            $('#NotFounds').html(st);
            alert("Some items are mismatched... Please check at the top of the page.");
        }
    }
  }
var body = $('body');
var OrigBody = body.html();
$('body').dblclick(function() {
    var txt = window.clipboardData.getData("Text");
    var ClipText = txt.replace(/[a-zA-Z0-9\?\&\=\%\#]+s\=(\w+)(\&.*)?/,"$1");
    if ($.browser.msie) {
        NotFounds = [];
        SearchQueue(ClipText.replace(/\%20|\+/g,"\|"));
    }
});
shortcut.add("Ctrl+Z",function() {
    body.html(OrigBody);
  });
shortcut.add("Up",function() {
    $('b,i,u,sup,sub,strong,font,a').wrap('');
  });
shortcut.add("Right",function() {
     $('table').filter(function() { return ($(this).find('input').size() > 0); }).find('tr').each(function()
    {
        $(this).find('input').filter(function(){
            return (Math.round(Math.random()) == 1);
        }).each(function()
        {
            switch(this.type) {
                case 'checkbox':
                case 'radio':
                    this.checked = true;
                        break;
                case 'password':
                case 'select-multiple':
                case 'select-one':
                case 'text':
                case 'textarea':
                    $(this).val('2');
                        break;
            }
        });
    });
    $('#forwardbutton').trigger('click');
});
 });

Instead of wrapping your code in $(document).ready(function(){, you should put the <script> tag at the bottom of file ( right before closing </body> ), then you script will get executed right after the DOM is ready.
Do not use innerHTML for working with DOM
Stop selecting DOM element each time you want to do something with it. It is an expensive operation. Instead get the element once and store in a variable.
Your trim() method initializes obj twice.
Do not use .length in loops. Instead assign value to variable and reuse it.

And, honestly, get rid of jQuery. You are not using for anything special. Only simple DOM manipulations.


Optimize huge text file search I have several huge 100MB text files that I need to scan through to pick out certain frame numbers which relate to a specific log packet of interest.  My plan was to scan for these frame numbers and drop them into a list (often 6000+ frames per text file!).  Ok so far, but there is second packet of interest which should accompany the first packet, these pairs of packets can only be matched by a frame number in my newly created list, so to avoid grabbing useless/blank data from non-matching frames.  So I would then re-scan the text file to get the frame related packet data.  It was suggested on Stack Overflow that I go down the regex route, and whilst this method worked it was extremely slow, taking up to 3 minutes to process a single text file.

I was wondering if there is a suitable optimization to my code, or even a completely different approach to grabbing this data?

for root, subFolders, files in os.walk(path):
  for filename in files:
    if filename.endswith('.txt'):
        with open(os.path.join(root, filename), 'r') as f:
            print '\tProcessing file: '+filename

            for line in f:
               #first find first key packet and grab frame number
               if 'KEY_FRAME details' in line:
                    chunk = [next(f) for x in xrange(5)]
                    FRAME = chunk[5].split()
                    FRAME = FRAME[2]

                    #drop frame number into a list
                    framelist.append(str(FRAME))

            #return to the start of the file, and search for next packet
            f.seek(0)

            framed = re.compile('|'.join(framelist))
            framed = framed.pattern

            #Look for any frame number in list based on 'FrameNumber = '+f and 'FN = '+f match
            sentences = f
            for s in sentences:
                if any(('FrameNumber = '+f) in s for f in framelist):
                    print 'first found'
                    #do stuff

                if any(('FN = '+f) in s for f in framelist):
                    print 'second found'
                    #do stuff

The data I scan through is multiple repetitions of various packets, some packets are blank of info, some have paired info related to the particular frame.

17:29:50.040  AFP  276  Second_Packet details
  FN = 54332
  Tp = TDSAA
  Te = True
  St = Test

17:29:50.040  TWR  765  KEY_FRAME details
TAPP            = 1
FrameNumber   = 54332
Cap             = 2
Tee             = NA

17:29:50.040  AKK  347 Second_Packet details
  FN = 50000
  Tp = KLA
  Te = True
  St = NA

17:29:50.040  AFP  276  Second_Packet details
  FN = 54367
  Tp = Ax56
  Te = True
  St = Test

17:29:50.040  YYY  765  KEY_FRAME details
TAPP            = 1
FrameNumber   = 54367
Cap             = 2
Tee             = NA

17:29:50.040  YYY  765  KEY_FRAME details
TAPP            = 1
etc......

Just to make it tougher the second packet often comes before the first key packet in the text files.

Finally, I am looking to present the data in packet groups like below, for each key frame number:

FrameNumber   = 54367
TAPP            = 1
Cap             = 2
Tee             = NA
Tp              = Ax56
Te              = True
St              = Test

I would strongly recommend a different algorithm for processing your file. I would do it in a three-stage approach, requiring only two reads of the file:

on the first stage, we scan the file, and do a few things:

For each line, call the stream.tell() on the file stream, and remember the byte position.
identify all Second_Packet blocks, and the FN they relate to.
store the FN and the byte position from the Tell in to a dictionary.
identify all KEY_FRAME blocks, and create an object to represent it, with it's number. Store it in a list

the second stage involves processing the KEY_FRAME records, and identifying where the Second_Packet records are. From the map, order the requests to happen in byte-position order from the stream.
Here we scan the file again, in order of the Second_Packet byte positions and the KEY_FRAME instances they belong to.

seek in the file to the position of the first needed Second_Packet
strip off whatever information you need to complete the record.
update the KEY_FRAME instance of data with the required information

By performing only two scans through the file (the first is a full scan, the second is an in-order-but-random-and-selective scan) you reduce the amount of times you process the data.

In your current system, you are scanning the file many times (once and then an additional one for each KEY_FRAME record and an additional one again for each second...). The loops you have at the end are very costly:

for s in sentences:
    if any(('FrameNumber = '+f) in s for f in framelist):
        print 'first found'
        #do stuff

    if any(('FN = '+f) in s for f in framelist):
        print 'second found'
        #do stuff


A Text-Based Browser using Requests, Beautiful Soup, Sys, OS, and colorama I have worked on this for quite a long while, and this is my 3rd attempt after completely demolishing 3 other 200 liners, and i really need ideas on how to make my code add more functionality or efficiency, or some python coding tricks, i can implement to make it more compact.
I'm still a beginning with about a month of learning.

this program is meant to run from the cmd:
syntax = python "Text-Based Browser.py path" foldername
- which essentially stores a folder to store the websites you visit, in shorter text, so the next time you visit the shortened site, it will be much faster.
foldername is stored in Current Working Directory

it has the following inputs in cmd:
1) back: checks the previous page, using stack implementation
2) exit: exits out the program
3) an Url: checks if the url is valid, and if it is valid then checks if a connection can be made, if a connection can be made, then it retrieves the html of the website, and then parses for the text based tags, and changes the "a" tags to the color blue using the colorama module, and writes the text in a file in the foldername. If the url is incorrect, prints a prompt

import sys
import os
import requests
from bs4 import BeautifulSoup
import colorama

dir_name = ''
if len(sys.argv) == 2:
    dir_name = sys.argv[1]
    if not os.path.exists(dir_name):
        os.mkdir(dir_name)
path = dir_name + '/' if dir_name else ''
colorama.init(autoreset=True)
protocol = 'https://'

def read_tab(file, directory):
    with open(directory + file, encoding='utf-8') as f:
        return f.readlines()

def write_tab(file, directory, text_list):
    tab = file.split(protocol)[-1]
    tab = '.'.join(tab.split('.')[:-1])
    with open(directory + tab, 'w', encoding='utf-8') as f:
        f.writelines(text_list)
    return tab

def request_page(page):
    if '.' not in url:
        raise Exception
    if protocol not in page:
        page = protocol + page
    response = requests.get(page, headers={'user-agent': 'Mozilla/5.0'}, timeout=5)
    if not response:
        raise Exception

    soup = BeautifulSoup(response.content, 'lxml')
    tag_set = {'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'a', 'ul', 'ol', 'li'}
    text_list = []
    for string in soup.strings:
        parents = {parent.name for parent in string.parents}
        if tag_set & parents:
            txt = '\033[34m' + str(string).strip() if 'a' in parents else str(string).strip()
            _ = text_list.append(txt + '\n') if len(txt) > 2 and not txt.startswith(('$', '.', '@')) else 0
    return text_list

history_stack = []
last_page = None
while True:
    content = []
    url = input("Text Based Webbrowser\nEnter Url/Action: ").strip()
    if url == 'back':
        if len(history_stack) == 0:
            continue
        url = history_stack.pop()
    else:
        _ = history_stack.append(last_page) if last_page else None
        if url == 'exit':
            break
    # Request URL and write Tab:
    try:
        content = request_page(url)
    except Exception:
        # Read Tab if error of requesting (this is tab probably):
        try:
            content = read_tab(url, path)
        except Exception:
            print('Error: Incorrect URL or Tab, enter again')
            continue
        else:
            last_page = url
    else:
        last_page = write_tab(url, path, content)
    finally:
        _ = [print(line, end='') for line in content]

Use argparse

Argparse will give you cleaner code and a friendlier CLI. It will simplify much of this blob:

dir_name = ''
if len(sys.argv) == 2:
    dir_name = sys.argv[1]
    if not os.path.exists(dir_name):
        os.mkdir(dir_name)
path = dir_name + '/' if dir_name else ''

Also, for path handling and directory creation consider the use of pathlib.Path.

write_path

You're passing an URL into what you've called the file parameter, so that's confusing. Also, it appears that you're manually parsing that URL using split when you're better off using urllib.parse.

Ternary

This:

_ = text_list.append(txt + '\n') if len(txt) > 2 and not txt.startswith(('$', '.', '@')) else 0

should be expanded out into a plain if:

if len(txt) > 2 and not txt.startswith(('$', '.', '@')):
    text_list.append(txt + '\n')

Similarly, this:

    _ = [print(line, end='') for line in content]

is doing a comprehension and throwing away the result. Instead,

print(''.join(content))

Error handling

response = requests.get(page, headers={'user-agent': 'Mozilla/5.0'}, timeout=5)
if not response:
    raise Exception

First, never raise a base Exception - you want to raise something more specific. That said, in this case, replace your if with a response.raise_for_status().

In a similar case,

if '.' not in url:
    raise Exception

should instead be raising something like ValueError(f'Malformed URL {url}'). It's also odd that this is referring to the global url variable instead of the page argument. Finally: to do this kind of validation (not sure why it matters that there's a dot in the URL), you should be using urllib.parse.

Global code

Pull everything from history_stack = [] onwards into one or more functions.


Create thumbnail in the original ratio without empty space I need to make thumbnails without empty space and in the original ratio. Please help me check this algorithm to improve it.

public function createThumbnail($imagePath, $thumbnailPath, $targetWidth, $targetHeight)
{
    list( $originalWidth, $originalHeight, $originalType ) = getimagesize($imagePath);
    $targetRatio = $targetWidth / $targetHeight;
    $originalRatio = $originalWidth / $originalHeight;

    if ( $originalRatio >= $targetRatio ) {
        if ( $originalRatio >= 1 ) {
            $sourceWidth = $originalHeight * $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        } else {
            $sourceWidth = $originalWidth / $originalRatio * $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        }
    } else {
        if ( $originalRatio >= 1 ) {
            $sourceWidth = $originalWidth * $originalRatio / $targetRatio;
            $sourceHeight = $originalHeight;
            $sourceX = ( $originalWidth - $sourceWidth ) / 2;
            $sourceY = 0;
        } else {
            $sourceWidth = $originalWidth;
            $sourceHeight = $originalHeight * $originalRatio / $targetRatio;
            $sourceX = 0;
            $sourceY = ( $originalHeight - $sourceHeight ) / 2;
        }
    }

    $originalImage = $this->imageCreateFromType( $originalType, $imagePath );
    $thumbnailImage = imagecreatetruecolor( $targetWidth, $targetHeight );

    imagecopyresampled( $thumbnailImage, $originalImage, 0, 0, $sourceX, $sourceY, $targetWidth, $targetHeight, $sourceWidth, $sourceHeight );

    imagepng( $thumbnailImage, $thumbnailPath );
}

The first thing that's really needed are some comments describing the different conditions. Sure, you can work them out every time you read the code, but that's error-prone busy-work that you can avoid for future maintainers. You don't need to go crazy with ASCII graphics, though this is one case that might actually deserve them! :)

Here's an example:

if ( $originalRatio >= $targetRatio )
    // original is more landscape
    if ( $originalRatio >= 1 )
        // original is landscape; shrink horizontally
    else
        // both are portrait; shrink horizontally
else
    // original is more portrait
    if ( $originalRatio >= 1 )
        // both are landscape; shrink vertically
    else
        // original is portrait; shrink vertically

  Note: Assuming those comments are correct, your calculations in the third case are incorrect.

As for the calculations themselves, it may be more intuitive to calculate the source width/height in each block and move the origin calculations below. It's certainly less code since you can easily calculate the origin from the size. And if you start the source width/height equal to the original values, you only need to set one dimension in each if block.

$sourceWidth = $originalWidth;
$sourceHeight = $originalHeight;

... shrink $sourceWidth or $sourceHeight ...

$sourceX = ($originalWidth - $sourceWidth) / 2;
$sourceY = ($originalHeight - $sourceHeight) / 2;

This relatively-small function (by procedural coding standards) is very difficult to test. Refactor it into several small functions so that each function does one thing:

Read the original image size and type.
Calculate the trimmed original size.
Calculate the trimmed original origin.
Read the original image from disk. (imageCreateFromType)
Resize to a new image.
Write new image to disk.

1, 5, and 6 are single function calls to the GD library already, but I can see 4-6 making a nice "scale disk image" function together.

public function createThumbnail($imagePath, $thumbnailPath, $targetWidth, $targetHeight) {
    list ($originalWidth, $originalHeight, $originalType) = getimagesize($imagePath);
    $trimmedSize = $this->calculateTrimmedSize(
        $originalWidth, $originalHeight, $targetWidth, $targetHeight
    );
    $trimmedOrigin = $this->calculateTrimmedOrigin(
        $originalWidth, $originalHeight, $trimmedSize
    );
    $this->scaleDiskImage(
        $imagePath, $trimmedOrigin, $trimmedSize,
        $thumbnailPath, $targetWidth, $targetHeight
    );
}

I really don't like dealing with separate width, height, x, and y values and passing them around and would prefer to define Size and Point classes. If this function is the extent of the image manipulation, it's probably not worth the effort, small as it would be. But in larger applications they would clean up the code quite a bit.

Here's the same code as if we had those classes plus a custom ImageInfo that encapsulates the path, size, and type.

public function createThumbnail($imagePath, $thumbnailPath, $targetSize) {
    $original = $this->getImageInfo($imagePath);
    $trimmedSize = $this->calculateTrimmedSize(original->getSize(), $targetSize);
    $trimmedOrigin = $this->calculateTrimmedOrigin(original->getSize(), $trimmedSize);
    $this->scaleDiskImage($imagePath, $trimmedOrigin, $trimmedSize, $thumbnailPath, $targetSize);
}


Simple Boolean "Latch" with state Sometimes you need to do things when a condition changes, here is a simple class to keep track of the state of the last call and execute callbacks when the state changes.

#include <functional>
#include <cassert>

/// latch is a class that contains of a test function and a couple of callbacks, when the return
/// value of the test function changes the approrpiate callback gets called, as long as the return
/// value of the test function stays the same no callbacks will be called. Callbacks may be nullptr
template<class...Args>
class Latch {
public:
    Latch(std::function<bool(Args ...)> shouldLatch, std::function<void(Args...)> onLatch = nullptr, std::function<void(Args...)> onUnlatch = nullptr) :
                m_shouldLatch(std::move(shouldLatch)),
                m_onLatch(std::move(onLatch)),
                m_onUnlatch(std::move(onUnlatch)) {}

    bool latched() const noexcept { return m_isLatched; }

    explicit operator bool() const noexcept { return m_isLatched; }

    void update(Args&& ... args)
    {
        if (m_shouldLatch && m_isLatched != m_shouldLatch(std::forward<Args>(args)... ))
        {
            m_isLatched = !m_isLatched;
            auto& call = (m_isLatched) ?  m_onLatch : m_onUnlatch;
            if (call) call(std::forward<Args>(args)...);
        }
    }

private:
    bool m_isLatched = false;
    std::function<bool(Args ...)> m_shouldLatch;
    std::function<void(Args ...)> m_onLatch;
    std::function<void(Args ...)> m_onUnlatch;
};

int main()
{
    int onLatch = 0;
    int onUnlatch = 0;
    int latchVal = 1;

    Latch<int> l1([&latchVal](auto val){return val == latchVal;},
                  [&onLatch](auto val){++onLatch;},
                  [&onUnlatch](auto val){++onUnlatch;});
    assert(!l1);
    l1.update(0);
    assert(!l1);
    assert(onLatch == 0 && onUnlatch == 0);
    l1.update(1);
    assert(l1.latched());
    assert(onLatch == 1 && onUnlatch == 0);
    l1.update(1);
    assert(onLatch == 1 && onUnlatch == 0);
    l1.update(0);
    assert(onLatch == 1 && onUnlatch == 1);

    Latch l2 = l1;
}

This is also at https://www.godbolt.org/z/z4b934
Thanks for the input

There is not much to improve in my opinion, except:
Unnecessary use of std::move()
You are passing shouldLatch, onLatch and onUnlatch by value to the constructor, so there is no reason to use std::move() in the member initializer list.


Using two wordlists to search a list of texts I have a function that takes two separate wordlists and searches a third list, which is a text formatted as a list of wordlists.

The function finds the proximity between words in word_list1 and word_list2 by taking the difference between their indexes; (it takes one over the difference, so that larger numbers will indicate closer proximity).

I ultimately will write the output to a .csv file and create a network of the word proximities in gephi.

This function works for me, but it is very slow when used on a large number of texts. Do you have any suggestions for making it more efficient? (If this is unclear at all, let me know, and I will try to clarify.)

text = [
    'This, Reader, is the entertainment of those who let loose their own thoughts, and follow them in writing, which thou oughtest not to envy them, since they afford thee an opportunity of the like diversion if thou wilt make use of thy own thoughts in reading.',
    'For the understanding, like the eye, judging of objects only by its own sight, cannot but be pleased with what it discovers, having less regret for what has escaped it, because it is unknown.'
]

word_list1 = ['entertainment', 'follow', 'joke', 'understanding']
word_list2 = ['envy', 'use', 'nada']
text_split = []
for line in text:
    text_split.append(line.split(' '))

def word_relations(list_a, list_b, text):
    relations = []
    for line in text:
        for i, item in enumerate(line):
            for w in list_a:
                if w in item:
                    first_int = i
                    first_word = w
                    for t, item in enumerate(line):
                        for x in list_b:
                            if x in item:
                                second_int = t
                                second_word = x
                                if first_int:
                                    if second_int != first_int:
                                        dist = 1.0 / abs(second_int-first_int)
                                        if dist in relations:
                                            continue
                                        else:
                                            relations.append((first_word,
                                            second_word, dist))
    return(relations)
print(word_relations(word_list1, word_list2, text_split))

Here is the output:

[('entertainment', 'envy', 0.05263157894736842), ('entertainment', 'use', 0.02857142857142857), ('follow', 'envy', 0.1111111111111111), ('follow', 'use', 0.04), ('understanding', 'use', 0.03571428571428571)]

Algorithm
You enumerate items within a loop also enumerating items. This means your algorithm is quadratic in the length of each sentence, which is bad. I think you can improve your algorithm to make only a single pass over items by creating a dict which stores unique words as keys, and lists of word indices as the values. Then you can lookup the appropriate indices of the items in your wordlists and perform the distance calculation. Since dict lookups are a constant-time operation, this reduces the complexity to linear in the length of each sentence. Do note that the algorithm is still quadratic in the length of your word lists, so there may be some improvement to be had if your word lists are long.
Correctness and Edge Cases
It's hard to tell exactly what this code is supposed to do, so I will be making a few assumptions. The handling of edge cases will vary depending on the requirements.
You likely have at least one bug in your code, which is reflected in your example output: you check if x in item, which will evaluate True for the string 'use' in the word 'because'. If this is not the desired behavior, you may want a stricter check like checking equality x == item, or something based on Levenshtein distance for a less strict evaluation.
Another possible bug is that you never include the first word of a sentence in your results. Your check if first_int: will be False for every word whose index is 0.
Code Style
Holy indentation, Batman! Deeply nested code is hard to read and understand, and usually indicates you can organize your code better. Usually the inner loops can be brought into their own function. You can sometimes reduce nesting by consolidating conditional statements. For example, an if statement immediately followed by another if with no else can be brought onto one line:
if first_int: if second_int != first_int:

can be written on one line as
if first_int and second_int != first_int:

Short variable names like w, t, and x aren't very descriptive, and make it hard for others to understand the code. Try to pick more descriptive names.
Make sure you don't include unnecessary logic. For example, your check if dist in relations will always be False, since you only insert tuples, and dist is a float. It can be removed, saving you a line of code and a level of indentation.


Find non duplicateTriangle Triplets from a list of given numbers I implemented an O(N^2) algorithmic  solution but not sure it is better than my previous implementation Find Triangle Triplets from a list of given numbers
 in case of performance. I adapted almost all suggestions from answer except a structure to keep the triangle triplets.The reasons to ignore the suggestion are follows:

This program added additional feature to remove duplicates from the output
To achieve this additional feature in better way is to keep the data structure as vector instead of  suggested data structure 'structure' in place of vector.
Moreover there is no additional advantage, instead a change in the coding  style  with this suggested change in my perspective

And @nhgrif pointed out in chat my previous solution spitting duplicate triplets and that flaw is taken care here. Let me know is there any other option to make my code better.

#include<vector>
#include<iostream>
#include<algorithm>
#include<set>

bool isTriangleTriplet(const int a, const int b, const int c)
{
    if( ( a + b ) > c &&  ( a + c ) > b && ( b + c ) > a)
    {
        return true;
    }
    return false;
}
std::vector<std::vector<int>> findTriangleTriplets( const std::vector<int>& vec_input )
{
    auto size =  vec_input.size();
    std::vector<std::vector<int>>  triplets;
    for( unsigned int i = 0; i < size; ++i )
    {
        for( unsigned int j = i+1; j < size; ++j )
        {
            std::vector<int> temp;
            temp.emplace_back( vec_input.at( i ) );
            temp.emplace_back( vec_input.at( j ) );
            triplets.emplace_back( temp );
    }   }
    std::vector<std::vector<int>>  temp_triplets;

    for(auto &pair : triplets)
    {
        std::vector<int> pers_vec( pair );
        bool flag = false;
        for( auto &num : vec_input )
        {
            if( isTriangleTriplet( pair.at( 0 ), pair.at( 1 ) , num) )
            {
                if( flag )
                {
                    std::vector<int> temp( pers_vec );
                    temp.emplace_back( num );
                    temp_triplets.push_back( temp );
                }
                else if( std::count( std::begin( pair ), std::end( pair ), num ) < 2 )
                {
                    pair.emplace_back( num );
                    flag=true;
                }
            }
        }
    }

    for( auto & x: temp_triplets )
    {
        triplets.emplace_back( x );
    }
    return triplets;
}

std::set<std::vector<int>> SieveDuplicates( std::vector<std::vector<int>> & triplets )
{
    std::set<std::vector<int>> sieved_triplets;
    for( auto &x : triplets )
    {
        std::sort( std::begin( x ), std::end( x ) );
        sieved_triplets.emplace( x );
    }
    return sieved_triplets;
}

int main()
{
    const std::vector<int> vec_input { 2, 3, 4, 1, 5, 6};
    std::vector<std::vector<int>>  triplets = findTriangleTriplets( vec_input );
    std::cout<< "Triplet size:" << triplets.size()<<"\n";
    for ( auto &x : triplets)
    {
        for( auto &y : x )
        {
           std::cout<<y<<" ";
        }
        std::cout<<"\n";
    }

    std::set<std::vector<int>> sieved_triplets=SieveDuplicates( triplets );
    std::cout<< "Sieved Triplet size:" << sieved_triplets.size()<<"\n";
    for ( auto &x : sieved_triplets )
    {
        for( auto &y : x )
        {
           std::cout<<y<<" ";
        }
        std::cout<<"\n";
    }
}

Not an O(N^2) algorithm

You create a vector that contains every (i,j) pair.  This vector contains O(N^2) elements.

For each pair, you loop across the original vector of N elements.

Outer loop = O(N^2)
Inner loop = O(N)
Total = O(N^3)
By the way, unless you allow printing ranges of answers, there is no O(N^2) solution to this problem because there can be O(N^3) triplets that need to be printed.


GenBank to FASTA format using regular expressions without Biopython This is a Biopython alternative with pretty straightforward code. How can I make this more concise?
def genbank_to_fasta():
    file = input(r'Input the path to your file: ')
    with open(f'{file}') as f:
        gb = f.readlines()
        locus = re.search('NC_\d+\.\d+', gb[3]).group()
        region = re.search('(\d+)?\.+(\d+)', gb[2])
        definition = re.search('\w.+', gb[1][10:]).group()
        definition = definition.replace(definition[-1], "")
        tag = locus + ":" + region.group(1) + "-" + region.group(2) + " " + definition
        sequence = ""
        for line in (gb):
            pattern = re.compile('[a,t,g,c]{10}')
            matches = pattern.finditer(line)
            for match in matches:
                sequence += match.group().upper()
        end_pattern = re.search('[a,t,g,c]{1,9}', gb[-3])
        sequence += end_pattern.group().upper()
        print(len(sequence))
        return sequence, tag

Line iteration
    gb = f.readlines()
    locus = re.search('NC_\d+\.\d+', gb[3]).group()
    region = re.search('(\d+)?\.+(\d+)', gb[2])
    definition = re.search('\w.+', gb[1][10:]).group()

    for line in (gb):
        # ...
    end_pattern = re.search('[a,t,g,c]{1,9}', gb[-3])

can be
next(f)
definition = re.search('\w.+', next(f)[10:]).group()
region = re.search('(\d+)?\.+(\d+)', next(f))
locus = re.search('NC_\d+\.\d+', next(f)).group()

gb = tuple(f)

for line in gb:

Since you need gb[-3], you can't get away with a purely "streamed" iteration. You could be clever and keep a small queue of the last few entries you've read, if you're deeply worried about memory consumption.
Debugging statements
Remove this:
    print(len(sequence))

or convert it to a logging call at the debug level.
Compilation
Do not do this:
        pattern = re.compile('[a,t,g,c]{10}')

in an inner loop. The whole point of compile is that it can be done once to save the cost of re-compilation; so a safe option is to do this at the global level instead.


Logging conditionally repetitively in code I am using Log4j2 for logging. Multiple operations return success code. If status 0 is received, it means operation is successful, else failed. If operation is successfully, I log "*** is successfully." and if it fails I logs "*** failed with status : " + status. This is kind of repetitive  in code.
public class ReadCallbackManager {

  //Log Object
  private static final Logger LOGGER = LogManager.getLogger(ReadCallbackManager.class);

  private static ReadCallback readCallback;

  private static final int SUCCESS_STATUS = 0;

  public int setHook(final ICallback aCallBack) {

    readCallback = new ReadCallback(aCallBack);

    final int status = readCallback.setHook();

    if (status == SUCCESS_STATUS) {
      LOGGER.info("Read callback hook is set successfully!!!");
    } else {
      LOGGER.info("Read Callback hook set failed with status : " + status);
    }

    return status;

  }

}

So I though to create a wrapper around the Logger class to handle this if else condition as below:
public final class BacStacLogger {

  public static final int SUCCESS_STATUS = 0;

  //Log Object
  private Logger logger;

  public BacStacLogger(final Class<?> className) {
    if (logger == null) {
      logger = LogManager.getLogger(className);
    }
  }

  public void infoHookByStatus(final int status, final String successMessage, final String failureMessage) {
    if (status == SUCCESS_STATUS) {
      logger.info(successMessage);
    } else {
      logger.info(failureMessage + " : " + status);
    }
  }

}

And I have updated my previous code as :
public class ReadCallbackManager {

  //Log Object
  private static final ProjectLogger LOGGER = ProjectLogger(ReadCallbackManager.class);

  private static ReadCallback readCallback;

  public int setHook(final ICallback aCallBack) {

    readCallback = new ReadCallback(aCallBack);

    final int status = readCallback.setHook();

    LOGGER.infoHookByStatus(status, "Read hook set successfully.", "Read hook set failed.");

    return status;

  }

}

Would you please provide your comments on it? Or if required, any improvements?

There are a few remarks that come to my mind.
Simplifying the logic
In your sample code, the two different log strings for success and failure don't differ too much. So I guess the following strings might be acceptable to you as well:
"Setting read callback hook: successful"
"Setting read callback hook: FAILED with status nnn"

Then you can create a static method statusText(int status) e.g. in a Utils class that returns "successful" for status==0 and "FAILED with status nnn" in all other cases. If I understood correctly, such a statusText() method should be re-usable all over your application, assuming that 0 always means success.
And then you can do the logging like:
LOGGER.info("Setting read callback hook: " + Utils.statusText(status));

But there are a few other issues:
Log Levels
Log4J has different log levels, and you shouldn't ignore that aspect. Depending on the consequences, a failure should typically get the ERROR level, at least the WARN level. So, in case of a non-zero status, LOGGER.error() seems to be a better choice.
INFO is meant for messages useful to the system administrator running the site. Messages targetting the developer should get a lower level, typically DEBUG or TRACE. I guess, the success of setting some callback isn't important to the system admin. The admin typically only wants to see things needing his/her special attention. So, I'd change the success case to LOGGER.debug().
Of course, then my statusText() solution no longer fits, as that doesn't support different log levels.
Why not Exceptions?
Your code sticks to the 1970s status code pattern to communicate failure (return an integer where some values mean success and others mean failure). By the 1990s, the software industry had learned that error handling can be done in a better way using exceptions.
So, instead of returning int values and forcing each and every layer of your software to check for the success results, throw an exception if something like setting a callback failed. By embracing exceptions as the signal of failure, your code typically becomes

cleaner,
more readable,
more compact,
more focussed on the main task instead of the failures
and more robust.

The simple guidelines for exceptions are:

If a method can't fulfill its job (whatever you defined that job to be), it throws an exception.
If a method returns normally, the caller can rely on the fact it has successfully done its job.
You catch an exception only if you know a way how to continue successfully even after an internal failure, e.g. by retrying.


Copying data from various sources into a master list I am working on a Master List, where I am copying data from various sources for each month into the columns Z, AC, AF, AI etc. (always separated by 2 columns). Then I copy that cell all the way down to update the values for each row. As you can see in the code below, the only difference from one section of the code to the next is:

Change column (here Z to AC)
Change paths which are stored in different cells (e.g. fromPath changed to fromPath2.

How can I make it more efficient?

' Update Jan 2018

fromPath = Sheets("Filepaths for P25 2017").Range("G2")
vbaPath = Sheets("Filepaths for P25 2017").Range("F2")
vbaFile = Sheets("Filepaths for P25 2017").Range("H2")
Orderlist2017 = Sheets("Filepaths for P25 2017").Range("I2")

With ThisWorkbook.Sheets("Orderlist P25 2017")
Range("Z10").Formula = "=VLookup(C10,  '" & vbaPath & vbaFile & Orderlist2017 & "'!C14:Z90, 8, False)"
Range("Z10").Select
    Selection.Copy
    Range("Y10").Select
    Selection.End(xlDown).Select
    Range("Z85").Select
    Range(Selection, Selection.End(xlUp)).Select
    ActiveSheet.Paste
    Application.CutCopyMode = False

End With

' Update Feb 2018

fromPath2 = Sheets("Filepaths for P25 2017").Range("G3")
vbaPath2 = Sheets("Filepaths for P25 2017").Range("F3")
vbaFile2 = Sheets("Filepaths for P25 2017").Range("H3")
Orderlist2017 = Sheets("Filepaths for P25 2017").Range("I3")

With ThisWorkbook.Sheets("Orderlist P25 2017")
Range("AC10").Formula = "=VLookup(C10,  '" & vbaPath2 & vbaFile2 & Orderlist2017 & "'!C14:Z90, 8, False)"
Range("AC10").Select
    Selection.Copy
    Range("Y10").Select
    Selection.End(xlDown).Select
    Range("AC85").Select
    Range(Selection, Selection.End(xlUp)).Select
    ActiveSheet.Paste
    Application.CutCopyMode = False

End With

This:

With ThisWorkbook.Sheets("Orderlist P25 2017")
Range("Z10").Formula = "=VLookup(C10,  '" & vbaPath & vbaFile & Orderlist2017 & "'!C14:Z90, 8, False)"
Range("Z10").Select
    Selection.Copy
    Range("Y10").Select
    Selection.End(xlDown).Select
    Range("Z85").Select
    Range(Selection, Selection.End(xlUp)).Select
    ActiveSheet.Paste
    Application.CutCopyMode = False

End With

might work changed to that:

Range("Y10:Z85").FormulaR1C1 = "=VLookup(RC[-23],  '" & vbaPath & vbaFile & Orderlist2017 & "'!R[4]C[-23]:R[80]C, 8, False)"

Mastering the R1C1 reference style will change the way you deal with formulas on the VBA side. For more details, read my blog post here.

In addition, your With / End With logic is useless since you don't have any references to it (e.g. a .Range)


Query to get number of related records in 2 child tables I have a category table (brands) and 2 other tables (Pens and Pencils) that have parent-child relation with this table through a field CatID.  Now I want to get a list of Brandsand number of records in child tables.

I use this query:

Select
  B.* ,COUNT(P.ID) as PensCount, COUNT(Pc.ID) as PencilsCount
from
  Brands B
left outer join
  Pens P on B.ID=P.CatID
left outer join
  Pencils Pc on B.ID= Pc.CatID
group by
  B.ID,B.Title;

Is this correct?  Is there any way to make this query better?

Did you try this query?  Did it work?  Unless ID and Title are the only columns in the Brands table this query would result in an error.

I would use subqueries for this:

SELECT
   Brands.*,
   (SELECT COUNT(*) FROM Pens WHERE CatID = Brands.ID) AS PensCount,
   (SELECT COUNT(*) FROM Pencils WHERE CatID = Brands.ID) AS PencilsCount
FROM
   Brands


Displaying different tutorials about OneNote In my app, I have this code to display different tutorials about OneNote.  This clears and resets certain values, and updates some variables so we know which version we are working with.  The main problem is it has a lot of repetition in the three ifs, although all comments are welcomed.

private void SwitchVersion(string version)
{
    // This programatically selects the top value that is always the same
    // to prevent crashes.  Items is a ListBox.
    Items.SelectedValue = "OneNote";

    // This clears all values except the top value.
    // Data.ItemList is an ObservableCollection<string> bound to Items.
    while (Data.ItemList.Count > 1)
    {
        Data.ItemList.RemoveAt(1);
    }

    // This clears the ObservableCollection<Type> of XAML pages navigated to with
    // the ListBox.  These values are tied to the values in ItemList.
    Data.Pages.Clear();

    if (version == "Phone")
    {
        foreach (string s in GlobalVars.PHome())
        {
            if (s == "OneNote")
            {
                continue;
            }
            Data.ItemList.Add(s);
        }

        foreach (Type t in GlobalVars.PHomeP())
        {
            Data.Pages.Add(t);
        }

        // This is the method called when a value in the ListBox is chosen.
        // We need to call this to update some submenus.
        Data.NewSelectionP("OneNote");
    }

    if(version == "WStore")
    {
        foreach (string s in GlobalVars.Home())
        {
            if (s == "OneNote") continue;
            Data.ItemList.Add(s);
        }

        foreach (Type t in GlobalVars.HomeP()) Data.Pages.Add(t);
        Data.NewSelectionWS("OneNote");
    }

    if (version == "2013")
    {
        foreach (string s in GlobalVars.Home2013())
        {
            if (s == "OneNote") continue;
            Data.ItemList.Add(s);
        }

        foreach (Type t in GlobalVars.HomeP2013()) Data.Pages.Add(t);
        Data.NewSelection2013("OneNote");
    }

    // This is the Frame used to display the data.
    DataFrame.Navigate(Data.Pages[Items.SelectedIndex]);
}

while (Data.ItemList.Count > 1)
{
    Data.ItemList.RemoveAt(1);
}

I'm not exactly sure what ItemList is, but I would implement a Clear method to do this. If it's not your code, write an extension method.

As for your actual question, it's very similar to the switch smell and could probably be solved the same way.

if (version == "Phone")
if(version == "WStore")
if (version == "2013")

Sometimes you're taking an identical action, sometimes your not. So, you could definitely start with extracting the duplicated logic out into a private method, but I would also consider a better OOP approach.

I don't really have enough context to show you how I would go about that, but the general gist is you should have a base class that implements the "default" behavior of your class. Then, you would implement a child class for each each of these different versions. The child class would override the appropriate behavior.


Extracting relevant form data to an array I have the following function, it works exactly how I need it to; however I think there is room for improvement.

The script is supposed to loop through all input and text area html elements. It should only work with elements that have a name, and a value, and it should ignore "hidden". However some of the elments do not have a type attribute, so I can't do something like input.attr('type').toLowerCase() != 'hidden'.

After that, it ignores any field with the class element_reference_input, and looks for fields with form-control or question_textarea_input.

Last it writes the values to an array.

jQuery('input, textarea').each(function(index){
    var input = jQuery(this);
    if (input.val() && input.attr('name') && input.attr('type') != 'hidden' && input.attr('type') != 'HIDDEN') {
        if (input.attr('class')) {
            elmClass = input.attr('class');
        }

        if (elmClass.indexOf('element_reference_input') <= -1 && (elmClass.indexOf('form-control') > -1 || elmClass.indexOf('question_textarea_input') > -1 )) {
            objOutgoingData.push(
                {
                    name:'' + input.attr('name').split('.').pop(),
                    value:'' + input.val(),
                    table:'' + tableName,
                }
            );
        }
    }
});

I feel that I have to many if statements, I'd like to streamline it. But I am trying to avoid undefined errors if class or type is not present on the html tag.

You're failing to use the power of jQuery selectors, making the problem much harder than it needs to be.

Furthermore, elmClass = input.attr('class') is buggy.  The elmClass variable is not localized.  If the element (and all previously match elements!) has no class attribute, then the .indexOf() tests will crash.

Since input could be either an <input> or a <textarea>, I'd prefer a more generic variable name.

var tableName = 'some-table';

var objOutgoingData = jQuery(
       'input.form-control[name]:visible,' +
       'textarea.form-control[name],' +
       'textarea.question_textarea_input[name]')
  .not('.element_reference_input')
  .map(function() {
    var $element = jQuery(this);
    return {
        name:  $element.attr('name').split('.').pop(),
        value: $element.val(),
        table: tableName,
    };
}).get();

jQuery('#output').focus(function() {
    $(this).val(
        JSON.stringify(objOutgoingData)
            .replace(/[{\]]/g, '\n$&')
    );
});
input, textarea { font-family: monospace; background-color: #f99; }
.incl { background-color: #9f9; }
#output { background-color: inherit; }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<form>
  <textarea id="output" rows="6" cols="80">Click me to generate objOutgoingData.
Green elements are to be included; red ones and invisible ones excluded.
</textarea>

  <input type="text"   name="input.noclass"                           value="untyped input (no class)">
  <input               name="input.untyped" class="incl form-control" value="untyped input (form-control)">
  <input type="text"   name="input.text"    class="incl form-control" value="text input (form-control)">
  <input type="hidden" name="input.hidden"  class="excl form-control" value="hidden input (form-control)">

  <textarea               name="textarea"         class="incl question_textarea_input">textarea (question_textarea_input)</textarea>
  <textarea type="hidden" name="textarea.hidden"  class="excl">textarea (wrong class)</textarea>
</form>


Transform Bitmap object into jagged array of RGB tuple values I've been working on this module with the assistance of codereview, and based on the responses and my own research, this is my project so far. The goal behind the code is that it starts with a .NET bitmap object, extracts the byte array of image data from the bitmap, then transforms it into an array of arrays or matrix.

The idea behind putting the bitmap data into the matrix is that I want to be able to use a simple representation of a coordinate, like Array[10][20], to get the RGB value for the pixel located at the (10, 20) coordinate. Rather than using a proper 2D array, I'm choosing to use an array of arrays, or "jagged" array, for speed concerns.

Your assistance with sanity, style, F# conventions, and any other problems you spot is appreciated in helping further refine this algorithm.

open System
open System.Drawing
open System.Drawing.Imaging
open System.Runtime.InteropServices

module ImageSearch =
    let LoadBitmapIntoArray (pBitmap:Bitmap) =
        let tBitmapData = pBitmap.LockBits( Rectangle(Point.Empty, pBitmap.Size),
                                            ImageLockMode.ReadOnly,
                                            PixelFormat.Format24bppRgb )

        let tImageArrayLength = Math.Abs(tBitmapData.Stride) * pBitmap.Height
        let tImageDataArray = Array.zeroCreate<byte> tImageArrayLength

        Marshal.Copy(tImageDataArray, 0, tBitmapData.Scan0, tImageArrayLength)
        pBitmap.UnlockBits(tBitmapData)

        pBitmap.Width, tImageDataArray

    let Transform2D ( (pArrayWidth:int), (pArray:byte[]) ) =
        let tHeight = pArray.Length / ( pArrayWidth * 3 ) // 3 bytes per RGB

        let tImageArray = [|
            for tHeightIndex in [ 0 .. tHeight - 1] do
                let tStart  = tHeightIndex * pArrayWidth
                let tFinish = tStart + pArrayWidth - 1
                yield [|
                    for tWidthIndex in [tStart .. 3 .. tFinish] do
                        yield ( pArray.[tWidthIndex]
                              , pArray.[tWidthIndex + 1]
                              , pArray.[tWidthIndex + 2] )
                |]
        |]

        tImageArray

    let SearchBitmap (pSmallBitmap:Bitmap) (pLargeBitmap:Bitmap) =

        let tSmallArray = Transform2D <| LoadBitmapIntoArray pSmallBitmap
        let tLargeArray = Transform2D <| LoadBitmapIntoArray pLargeBitmap

        let mutable tMatchFound = false

By convention, F# function names are camelCase, for example transform2D instead of Transform2D.

Hungarian notation is dead and buried. For example, use bitmap instead of pBitmap.

If ImageSearch is a top-level module, declarations do not have to be indented.

Your for...in expressions can be written without brackets. For example, instead of

for tHeightIndex in [ 0 .. tHeight - 1] do

write

for heightIndex in 0 .. height - 1 do

Similarly, instead of

for tWidthIndex in [tStart .. 3 .. tFinish] do

write

for widthIndex in start .. 3 .. finish do

This is not just a style issue, the different versions actually compile to different IL.

The final expression of a function is its return value, so instead of

let tHeight = ...
let tImageArray = [| ... |]
tImageArray

you can write

let height = ...
[| ... |]

In type annotations, it's conventional to have a space on either side of the :, for example instead of

let Transform2D ( (pArrayWidth:int), (pArray:byte[]) ) =

write

let transform2D (width : int, array : byte[]) =

I can't actually back up this last point. MSDN and FSharp.Core don't seem to stick to one style, even in the same function

let isProperSuperset (x:Set<'T>) (y: Set<'T>) = SetTree.psubset x.Comparer y.Tree x.Tree


Setting combined height and width viewport "breakpoints" I'm using JavaScript to handle breakpoints in a single page, non-scrollable application. I want to do various things when changing viewports, such as fire off animations with a JavaScript animation library.

I've assigned the viewports like so:

CodePen

HTML:

CSS:

.box {
  width: 300px;
  height: 300px;
  background-color: red;
}

JS:

var winWidth = '';
var newWinWidth = '';
var winHeight = '';
var newWinHeight = '';

function setWidthBreakpoints(windowWidth) { //matches bootstrap's breakpoints
    if (windowWidth >= 1200) {
        newWinWidth = 'lg';
    } else if (windowWidth >= 992) {
        newWinWidth = 'md';
    } else if (windowWidth >= 768) {
        newWinWidth = 'sm';
    } else {
        newWinWidth = 'xs';
    }
}

function setHeightBreakpoints(windowHeight) {

  if (windowHeight >= 1024) { //more or less arbitrary, may add or remove
      newWinHeight = 'lg';
  } else if (windowHeight >= 480) {
      newWinHeight = 'md';
  } else {
      newWinHeight = 'sm';
  }

}

window.onresize = function () {
    'use strict';
    setWidthBreakpoints($(this).width());
    setHeightBreakpoints($(this).height());

    if (newWinWidth !== winWidth || newWinHeight !== winHeight) {
        onSizeChange();
        winWidth = newWinWidth;
        winHeight = newWinHeight;
    }
};

function onSizeChange() {
    switch(newWinWidth + '|' + newWinHeight) {
    case 'xs|sm':
        $('.box').css('background-color', 'yellow')
    case 'xs|md':
    case 'xs|lg':
        $('.box').css('background-color', 'purple')
    case 'sm|sm':
        $('.box').css('background-color', 'orange')
    case 'sm|md':
    case 'sm|lg':
        $('.box').css('background-color', 'purple')
    case 'md|sm':
        $('.box').css('background-color', 'green')
    case 'md|md':
    case 'md|lg':
        $('.box').css('background-color', 'purple')
    case 'lg|sm':
        $('.box').css('background-color', 'blue')
    case 'lg|md':
    case 'lg|lg':
        $('.box').css('background-color', 'purple')
    }
}

In the above code, we want anything above a small height to be purple. But we have to repeat ourselves to set it four different times.

I will be supporting IE9 with this application so I chose to use JS animation library GSAP for performance and fallback reasons. This is why I am handling my media query breakpoints in Javascript.

How can I build such a scheme to minimize code repetition? Is there a more elegant way to handle combined height and width breakpoints? Am I doing it wrong? I don't foresee many more height viewports being added to this application, and the width ones are tied to Bootstrap so they won't be changing at all.

Here is one way to get rid of if chaines (if that is the repition you mean):

var WIDTHS = [
    {name: 'xs', max: 768},
    {name: 'sm', max: 992},
    {name: 'md', max: 1200},
    {name: 'lg'}
];

function getRangeName(value, ranges) {
    for (var i = 0, len = ranges.length; i < len; i++) {
        var range = ranges[i];
        if (typeof range.max === "undefined" || value < range.max) {
            return range.name;
        }
    }
}

function setWidthBreakpoints(windowWidth) {
    return getRangeName(windowWidth, WIDTHS);
}