Read a line from console in Java I have to read a line of strings and split it for whitespace:

ArrayList<HashSet<String>> setOfStrings = new ArrayList<HashSet<String>>();
while ((currentLine = bufferedReader.readLine()) != null) {
        String[] tokens = currentLine.split(" ");
        for (int i = 0; i < tokens.length; i++) {
        setOfStrings.add(new HashSet<String>(Arrays.asList(tokens[i])));
        }
    }

i have to do this ,and for this reason i need for each tokens a new hashSet

i need to make operations on tokens like

[afaf] -> one hashset in first position inside the arrayList
[bafafa] -> one hashset in second position inside the arrayList
[cssss]
[dggg]
...

with your solution ,to make a unique hashset for the tokens, i can't do separate operations for each tokens.

How can I improve the split of the currentLine and the insertion into the arrayList?  As I did it, it's very slow because I first have to split and then iterate on the array of tokens.  How can I optimize this operation?

Is the use of a Set for storing the tokens deliberate? That's because you can only have unique elements in the Set, and the output of the following code snippet will be 1, not 2:

System.out.println( new HashSet<>( Arrays.asList( "a a".split( " " ) ) ).size() );

Also, what you are doing is to store one token into a new List via Arrays.asList(), this is then iterated for elements to be stored into a new HashSet, which is finally added to setOfStrings. May we know what exactly are you trying to do here? I'll attempt to illustrate the output for the following:

Input:

a b c
d e f

Output:

setOfStrings
    new HashSet [ "a" ]
    new HashSet [ "b" ]
    new HashSet [ "c" ]
    new HashSet [ "d" ]
    new HashSet [ "e" ]
    new HashSet [ "f" ]

edit:

After reading your comment, I think what you're looking for is this:

while ((currentLine = bufferedReader.readLine()) != null) {
    String[] tokens = currentLine.split(" ");
    setOfStrings.add(new HashSet<String>(Arrays.asList(tokens)));
}

You do not have to iterate through tokens... Alternative suggestion relying on the utility method from the Collections class:

while ((currentLine = bufferedReader.readLine()) != null) {
    Set<String> hashSet = new HashSet<>();
    Collections.addAll( hashSet, currentLine.split( " " ) );
    setOfStrings.add(hashSet);
}

Do pay attention to its Javadoc: "The behavior of this convenience method is identical to that of c.addAll(Arrays.asList(elements)), but this method is likely to run significantly faster under most implementations." :)


Validating a list of dictionaries of names and tags I created a Python function to check the types from a list and also check the keys inside an dictionary within that list.

I have the following data:

[
   dict(name='Frank', tags=['dog', 'cat']),
   dict(name='Manfred', tags=['cat', 'chicken'])
]

My function looks like this:

def _validate_data(self, data):
    if not isinstance(data, list):
        raise('not a list')
    else:
        for element in data:
            if not isinstance(element, dict):
                raise('not a dictionary')
            else:
                if not all(key in element for key in ('name', 'tags')):
                    raise('Keys not inside dictionary')

This works fine, but I don't like the structure and I also think there may be a smarter way to code this function. I hope someone could give me some nice and helpful hints.

There're some modules that might help you get rid of the structure you're complaining about like marshmallow or voluptous and since you didn't added the reinventing-the wheel tag I guess that's perfectly okay.

For the sake of example, I'll refer to the former one because IMO it better fits our purpose (and is also probably clearer).

From the docs:

  Marshmallow is an ORM/ODM/framework-agnostic library for converting
  complex datatypes, such as objects, to and from native Python
  datatypes.

  In short, marshmallow schemas can be used to:

  Validate input data.
  Deserialize input data to app-level objects.
  Serialize app-level objects to primitive Python types. The serialized objects can then be rendered to standard formats such as
  JSON for use in an HTTP API.

First of, you'll need to define your Schema:

class DataSchema(Schema):
    name = fields.String(required=True)
    tags = fields.List(fields.String(), required=True)

In the above, name and tags are the keys of our dictionaries. In our class I've specified each key type (str and list). They're also mandatory, so I added required=True.

Next, to validate our top-level list, we need to instantiate our list item schema with many=True argument and then just load the data we need:

data, errors = DataSchema(many=True).load([
    {'name': 'Frank', 'tags': ['dog', 'cat']},
    {'name': 'Manfred', 'tags': ['dog', 'chicken']}
])

Printing the above:

print(data)
print(errors)

Will have the following output:

[{'name': 'Frank', 'tags': ['dog', 'cat']}, {'name': 'Manfred', 'tags': ['dog', 'chicken']}]
{}

Now, if we try to pass an invalid data to our dict, the errors will warn us about this. For example, passing a str instead of a list in our tags key will result in:

[{'name': 'Frank'}, {'name': 'Manfred', 'tags': ['dog', 'chicken']}]
{0: {'tags': ['Not a valid list.']}}

Full code:

from marshmallow import Schema, fields

class DataSchema(Schema):
    name = fields.String(required=True)
    tags = fields.List(fields.String(), required=True)

data, errors = DataSchema(many=True).load([
    {'name': 'Frank', 'tags': ['dog', 'cat']},
    {'name': 'Manfred', 'tags': ['dog', 'chicken']}
])

Now, IDK if the above will be valid for all the test-cases (e.g it might allow you to pass an empty list), but it should give you a good overview of what you can achieve with this module.


Map all symbols in module to dictionary The div and add functions are located randomly in my package structure. I have a function with this signature:

def calc(x, y, func_name='add')

How do I implement this function?

My idea is to import all the modules, then use the dir function, like so:

def import_submodules(package, recursive=True):
    """ Import all submodules of a module, recursively, including subpackages

    :param package: package (name or actual module)
    :type package: str | module
    :rtype: dict[str, types.ModuleType]
    """
    if isinstance(package, str):
        package = importlib.import_module(package)
    results = {}
    for loader, name, is_pkg in pkgutil.walk_packages(package.__path__):
        full_name = package.__name__ + '.' + name
        print 'full_name =', full_name
        results[full_name] = importlib.import_module(full_name)
        if recursive and is_pkg:
            results.update(import_submodules(full_name))
    return results

def create_symbol_module_d(module_name_module_d, include=None, exclude=None):
    """ Create mapping of symbol to module

    :param module_name_module_d
    :type module_name_module_d: dict[str, types.ModuleType]
    :rtype: dict[str, types.*]
    """
    inv_res = {}
    for mod in module_name_module_d.itervalues():
        for sym in dir(mod):
            if include and sym in include:
                inv_res[sym] = mod
            elif exclude and sym not in exclude:
                inv_res[sym] = mod
            else:
                inv_res[sym] = mod
    return inv_res

Then I can just do:

sym2mod = create_symbol_module_d(import_submodules('package_name'))
def calc(x, y, func_name='add'):  return sym2mod[func_name](x, y)

Well I'm going to start by saying that your end result is confusing. sym2mod and create_symbol_module_d aren't clear names that indicate what's going on. In particular I don't know why this would be necessary, so it wouldn't occur to me that these are custom adding and dividing functions that are placed so erratically in a package that you can't even find them. Also there is already an operator module that contains these functions. If yours are more specialised or complex, then you should use different names to distinguish them and reduce confusion.

Which brings me to another problem, why are they in random places? It sounds like your problem entirely exists just because of the package structure. To me that means you ought to improve the actual structure rather than treating the symptom of the problem.

Anyway, onto your actual functions. The docstring for import_module
is a little off. It says the import is recursive, but it isn't necessarily recursive, it's optionally so. I don't think you need to mention it since it's easy to see the arguments with introspection. ie.

>>> help(import_submodules)
Help on function import_submodules in module __main__:

import_submodules(package, recursive=True)
    Import all submodules of a module, recursively, including subpackages

The fact that your importer is also printing out messages could be quite a problem for people who might have a large package, and there's no way to turn it off! I'd add an argument like logging and default it to False. Only print if its been explicitly asked for. After all there's a dictionary record of the importing that can be inspected for what was imported.

create_symbol_module_d is quite confusing to me. It's a bad name, so is module_name_module_d. They're both long and communicate little. Then there's include and exclude. Are they meant to be lists of symbols like ['+', '-', '/']? You give no examples or indication. Then when I look at what they do... they do nothing! In every result of your if statements you end up just performing the same test. In the current function there's no need for if statements at all, let alone include and exclude. This is equivalent to your function:

def create_symbol_module_d(module_name_module_d, include=None, exclude=None):
    """ Create mapping of symbol to module

    :param module_name_module_d
    :type module_name_module_d: dict[str, types.ModuleType]
    :rtype: dict[str, types.*]
    """
    inv_res = {}
    for mod in module_name_module_d.itervalues():
        for sym in dir(mod):
            inv_res[sym] = mod
    return inv_res

You need better names and documentation for me to even be able to give feedback on it. I also don't know what inv_res is or means.


Sorting numbers in LISP I am doing a CS practice exercise in which you are supposed to sort exactly three numbers using only if statements.  I wrote the following code in LISP and would appreciate any suggestions how to improve on my code.  Thank you!

(defun generate-three ()
  (loop for l from 1 to 3 collect (random 25)))

(defun result (a b c) (format t "The sorted result is (~d,~d,~d)~%" a b c))

(let (a b c (the-three (generate-three)))
    (format t "~a ~%" the-three)
    (setq a (first the-three) b (second the-three) c (third the-three))
    (if (< a b)
      (if (< b c) (result a b c)
        (if (< a c) (result a c b) (result c a b)))
      (if (< a c) (result b a c)
        (if (< b c) (result b c a) (result c b a)))))

Based on feedback from commenters, the revised code looks as follows:

(defun my-sort-3 (a b c)
    (if (< a b)
      (if (< b c) (list a b c)
        (if (< a c) (list a c b) (list c a b)))
      (if (< a c) (list b a c)
        (if (< b c) (list b c a) (list c b a)))))

(destructuring-bind (a b c) (loop repeat 3 collect (random 25))
  (format t "The sorted result of (~d,~d,~d) is ~a ~%" a b c (my-sort-3 a b c)))

Latest iteration - note the changes in the final "format" statement:

(defun my-sort-3 (a b c)
    (if (< a b)
      (if (< b c) (list a b c)
        (if (< a c) (list a c b) (list c a b)))
      (if (< a c) (list b a c)
        (if (< b c) (list b c a) (list c b a)))))

(destructuring-bind (a b c) (loop repeat 3 collect (random 25))
  (format t "The sorted result of (~{~d~^, ~}) is (~{~d~^, ~})~%" (list a b c) (my-sort-3 a b c)))

It's been a while since I've used lisp, so I can't really critique your code for style or any common lisp gotchas.

That said, the biggest thing I notice here is that the code for generating the 3 numbers, sorting them, and printing the result are all tightly coupled with each other.  I would suggest defining a separate function that takes three numbers and returns them in sorted order, which can then be combined with your existing functions for generating three numbers and printing three numbers to give the desired output.  The added benefit, of course, is that this sort function can later be used to sort any three numbers, not just the three in this bit of code.


Any performance difficulties related to List? I'm using the following structure:

class Map
{
    public List<List<Point>> points;
    public List<Base> bases;
}

Point and Base are over-normal sized classes (has more than 30 attributes).

And I'm iterating this Map class like this:

public static void calculateMatrix(Map map)
{
    LineOfSight.LineOfSight los = new LineOfSight.LineOfSight();

    foreach (var b in map.bases)
    {
        foreach (var cc in map.points)
        {
            foreach (var c in cc)
            {
                calculatePointValues(c, b, los);
            }
        }
    }
}

There is around 500*500 points List<List<T>> and around 10 bases.

This iterating is calling calculatePointValues method around 2.5 million.

And my application can be initiated around 30 seconds.

Shall I use Array instead of List<>?
Shall I use for() instead of foreach?

Or, what should I do?

As was said in the comments, I don't think List<T> makes any significant difference over an array.

But with a bit of LINQ you can reduce the nesting a bit:

foreach (var b in map.bases)
{
    foreach (var c in map.points.SelectMany(cc => cc))
    {
        CalculatePointValues(c, b, los);
    }
}

And it could be worth trying to profile it with some Parallel looping as well:

Parallel.ForEach(map.bases, b =>
    {
        foreach (var c in map.points.SelectMany(cc => cc))
        {
            CalculatePointValues(c, b, los);
        }
    });


Merge sort in C++ with iterators I implemented merge sort with C++ and would like to get some feedback.
#include <iterator>
#include <vector>

template<typename It>
std::vector<typename It::value_type> merge(const It begin, const It mid, const It end)
{
    std::vector<typename It::value_type> v;
    It it_l{ begin }, it_r{ mid };
    const It it_mid{ mid }, it_end{ end };

    while (it_l != it_mid && it_r != it_end)
    {
        v.push_back((*it_l <= *it_r) ? *it_l++ : *it_r++);
    }

    v.insert(v.end(), it_l, it_mid);
    v.insert(v.end(), it_r, it_end);

    return std::move(v);
}

template<typename It>
void merge_sort(It begin, It end)
{
    auto size = std::distance(begin, end);
    if (size < 2)
        return;

    auto mid = std::next(begin, size / 2);
    merge_sort(begin, mid);
    merge_sort(mid, end);

    auto &&v = merge(begin, mid, end);
    std::move(v.cbegin(), v.cend(), begin);
}

Usage:
std::vector<int> v{ 8, 4, 1, 9, 16, 3 };
merge_sort(v.begin(), v.end());

Is there anything I should avoid? Any mistakes in the algorithm or implementation? It should work with other containers (e.g. std::list) as well.

Very nice overall.
Don't see anything technically wrong.
I agree with @ratchet freak that your variable naming (and declaring multiple objects in one line) is a not great. I would prefer better names and one variable per line.
Things I would change for efficiency:
// You know how big this vector is going to get.
std::vector<typename It::value_type> v;

// So reserve the appropriate space:
v.reserve(std::distance(begin, end));

Also this is not what you want:
return std::move(v);

// jsut return the object
return v;

Otherwise you will screw up RVO done by the compiler and it will generate a copy rather than a move.
At the call site the value returned by a function is already an rvalue reference. So adding the move here does not change anything (it will still be moved).
But this is not going to move the values:
buffer.push_back((*left <= *right) ? *left++ : *right++);

Here you are only getting lvalues passed to push_back so you are copying the underlying elements. Why not try and get a move out of it?
buffer.push_back(std::move((*left <= *right) ? *left++ : *right++));


Refactor Ruby method for getting domain? How do I make this more Ruby like? I want to return the host, for example
if the URL is "http://www.facebook.com" then I want to get 'facebook.com'.

Any other sub-domains without 'www' should give the subdomin as well.
If  the URL is "http://gist.github.com" then I want 'gist.github.com':

def get_domain
  a = @url.split('.')
  a[-1] =  a[-1].gsub(/\//,'')
  if a[0][-3..-1] == "www"
    a.delete_at(0)
  else
    a[0] = a[0].gsub(/https?:\/\//,'')
  end
  domain = a.join('.')
  domain
rescue => e
  puts e.message
end #end of get_domain

Use URI::parse:

require 'uri'
URI.parse("http://gist.github.com/a/b/c").host.sub(/^www\./, '')
#=> "gist.github.com"


Find password to a word document I am using Java and COM4j to communicate with Word. The approach I used here is brute force. I have a password protected .doc, and I forgot its password.

Complexity is (26)(maxPassLength). [a-z characters only]

Before I run this code and wait for 7-10 days here are my concerns.

My main concern is checking whether a generated string matches the password. Is there any alternative to this? Would you recommend threading to reduce the search space?

import com4j.Variant;
import java.util.HashMap;
import javax.swing.JOptionPane;
import word.Documents;
import word.WdOpenFormat;
import word._Application;
import word._Document;

public class DocPasswordFinder {

    public final static String filePath2 = "\\file\\abc.doc";
    private static HashMap<Integer, Character> mapC;
    private static final int passMaxLength;
    public static  _Application app;
    public static Documents doc;
    public static char[] arr;

    static {
    passMaxLength=9;
    arr = new char[passMaxLength];
    app = word.ClassFactory.createApplication();
    doc = app.documents();
    mapC=new HashMap<>();
        for (int i = 0; i < 26; ++i) {
            mapC.put(i, (char) ('a' + i));
        }
    }

    public static void genPass(int start, int indx) {
        if (indx == passMaxLength) {
            String PASSWORD = String.valueOf(arr);
            try {
        doc.open2000(filePath, false, true, false, PASSWORD, "", false,
    PASSWORD, "", WdOpenFormat.wdOpenFormatAuto, false, false);
                JOptionPane.showMessageDialog(null, "FOUND: " + PASSWORD);
                System.exit(0);
            } catch (com4j.ComException ex) {
                //continue;
            }
        } else {
            if (indx < arr.length) {
                for (int i = start; i < 26; ++i) {
                    arr[indx] = mapC.get(i);
                    genPass(start, indx + 1);
                }
            }
        }
    }

    public static void main(String[] args) {
        genPass(0, 0);
    }
}

EDIT
Bottleneck appears to be the doc.open2000() part.

Complexity is (26)(maxPassLength).

You know that the password is only lower-case alpha characters?

  Would you recommend threading to reduce the search space?

It's possible that threads wouldn't help. The doc.open2000 will be more expensive than anything else, and your communication with Word may (I don't know) be effectively single-threaded (i.e. talking to a single Word process/task).

You might get better performance from a Word emulator which is designed to run on a server, e.g. from http://www.aspose.com/


Printing the number of ways a message can be decoded My below solution to the following problem is exceeding the maximum recursion depth. I am looking for any improvements which i can make.

Problem

  Alice and Bob need to send secret messages to each other and are
  discussing ways to encode their messages:

  Alice: “Let’s just use a very simple code: We’ll assign ‘A’ the code
  word 1, ‘B’ will be 2, and so on down to ‘Z’ being assigned 26.”

  Bob: “That’s a stupid code, Alice. Suppose I send you the word ‘BEAN’
  encoded as 25114. You could decode that in many different ways!”
  Alice: “Sure you could, but what words would you get? Other than
  ‘BEAN’, you’d get ‘BEAAD’, ‘YAAD’, ‘YAN’, ‘YKD’ and ‘BEKD’. I think
  you would be able to figure out the correct decoding. And why would
  you send me the word ‘BEAN’ anyway?” Bob: “OK, maybe that’s a bad
  example, but I bet you that if you got a string of length 5000 there
  would be tons of different decodings and with that many you would find
  at least two different ones that would make sense.” Alice: “How many
  different decodings?” Bob: “Jillions!”

  For some reason, Alice is still unconvinced by Bob’s argument, so she
  requires a program that will determine how many decodings there can be
  for a given string using her code.

Input

Input will consist of multiple input sets. Each set will consist of a single line of at most 5000 digits representing a valid encryption (for example, no line will begin with a 0). There will be no spaces between the digits. An input line of ‘0’ will terminate the input and should not be processed.

Output

For each input set, output the number of possible decodings for the input string. All answers will be within the range of a 64 bit signed integer.

Example

Input:

25114
1111111111
3333333333
0

Output:

6
89
1

The following is my solution -

def helper_dp(input_val, k, memo):

    if k == 0:
        return 1

    s = len(input_val) - k

    if input_val[s] == '0':
        return 0

    if memo[k] != -1:
        return memo[k]

    result = helper_dp(input_val, k - 1, memo)

    if k >= 2 and int(input_val[s:s+2]) <= 26:
        result += helper_dp(input_val, k - 2, memo)

    memo[k] = result
    return memo[k]

def num_ways_dp(input_num, input_len):
    memo = [-1] * (len(input_num) + 1)
    return helper_dp(input_num, input_len, memo)

if __name__ == '__main__':
    number = input()
    while number != '0':
        print(num_ways_dp(number, len(number)))
        number = input()

Problem Link - https://www.spoj.com/problems/ACODE/

Your code performs one recursion per character and recursion depth is limited. For long inputs, your code will raise RecursionError: maximum recursion depth exceeded. Replacing recursion with an explicit loop solves the issue.

Also, if you start the calculation at k = 1 with memo[-1] and memo[0] properly initialized and then work upwards to k = len(s), the code will become significantly simpler. When calculating memo[k], you only ever access memo[k-2] and memo[k-1], so you can store only those. I renamed them old and new. Here is how it could look like:

def num_ways_dp(s):
    old = new = 1
    for i in range(len(s)-1):
        if int(s[i:i+2]) > 26: old = 0 # no two-digit solutions
        if int(s[i+1]) == '0': new = 0 # no one-digit solutions
        (old, new) = (new, old + new)
    return new

if __name__ == '__main__':
    number = input()
    while number != '0':
        print(num_ways_dp(number))
        number = input()


Formatting HTML for use in a locally hosted iframe This formats HTML for use in a locally hosted iframe so that you can manipulate the content in the iframe freely, without running into cross domain issues. It uses Goutte to retrieve the HTML. I'd like to improve the code, whether it's to fit a certain design pattern or just more efficient. I don't feel like it's as clean as it could be.

<?php
class HTMLFixerClass {
    public $client;
    public $crawler;
    public $url;
    public $originalHTML;
    public $validHTML;

    public function __construct($url) {
        $this->url = $url;
        $this->client = new GoutteClient();
        $this->crawler = $this->client->request('GET', $url);
        if ($this->crawler->filter('html')->count()) {
            $this->originalHTML = $this->crawler->filter('html')->html();
            $this->validHTML = $this->correct_directories($this->originalHTML);
        }
    }

    public function get_html() {
        if ($this->validHTML !== '') {
            return "<!DOCTYPE HTML>\n<html>\n" . $this->validHTML . "\n</html>";
        } else {
            return 'Invalid URL';
        }
    }

    public function correct_directories($html) {
        $temp_url = $this->url;
        preg_match_all('/<.*?\.{2}\/.*?>/', $html, $matches);
        $initial_matches = $matches;
        if ($matches) {

            //foreach match in the entire doc
            for ($i = 0; $i < count($matches[0]); $i++) {

                //foreach match in each match ex ../../
                preg_match_all('/[\.]{2}[\/]/', $matches[0][$i], $sub_matches);
                $replacement_string = '';
                for ($j = 0; $j < count($sub_matches[0]); $j++) {
                    $replacement_string .= '../';

                    //reduces base url by one directory foreach ../ found in
                    if ($j == 0) {
                        preg_match_all('/.*(?=\/.*\/)/', $temp_url, $replacement_url);
                    } elseif ($j > 0) {
                        preg_match_all('/.*(?=.*\/)/', $replacement_url[0][0], $replacement_url);
                    }

                }

                //in the end $replacement_url[0] should be the desired url for appending to css or script in order to become absolute link
                $matches[0][$i] = str_replace($replacement_string, $replacement_url[0][0] . '/', $matches[0][$i]);
            }
            for ($k = 0; $k < count($matches[0]); $k++) {
                $html = str_replace($initial_matches[0][$k], $matches[0][$k], $html);
            }
            return $html;
        } else {
            var_dump($matches);
        }
    }
}

SOLID

It crawls HTML code from sites and then it manipulates the returned code, not adhering to the Single Responsibility Principle.

Bad Naming

Don't name your class HTMLFixerClass, obviously it is a class. Also with OOP, in PHP, the convention is camelCase for variables and PascalCase for class names. E.g. yours would become HtmlFixerClass.

What does correct_directories() do? Programmers should be reading this method signature and immediately get an idea of what it is that this method is doing. Currently I have no clue, and it requires me to dig inside the body of the method to figure it out. Especially when it's doing a bunch of regex string manipulations which looks like gibberish at first sight (WTF moment).

Encapsulation

Make your class as private as possible.

If you have no means of having certain components extended or directly called from the public, you can make those private.

A Constructor Constructs

Your constructor is doing way too much.

A constructor is exactly as the name implies it to be, it constructs the object. You are creating a new instance of an object, crawling from a URL, doing a whole bunch of stuff that shouldn't be in there.

Dependency Injection

Your class is tightly coupled to GoutteClient.

What if you wanted to use another instance of GoutteClient? You'd now have to modify your class, which is a bad thing. You can utilize Dependency Injection to make it loosely coupled.

Comments

I do appreciate the comments, but they don't really help all that much. Maybe you should include an example in it, to give the programmer a quicker idea of what's being done.

Also //foreach match in the entire doc, pretty redundant. It doesn't add any additional information. So I would just remove it.

I would also start commenting your entire class. See PHPDoc for examples.

Coding Standards

Try to adhere to the coding standards defined by the PHP Framework Interop Group. It gives better readability to your code, especially for other programmers who do it as well. Of course, you do not have to be perfect with it.

Conclusion

Trying to fathom the use case of this class from a higher level, I came to realize that a class like this is pretty much unnecessary.

Basically it contains just one relevant utility method that does something helpful, correct_directories(), although I can't be bothered putting some more effort into actually understanding what it does.

I would probably just make this a standalone function, or contain it in a static utility class HtmlFixer which would contain every method that fixes anything HTML code related.

As far as crawling the HTML from URLs, this should be done somewhere in the application level, where different objects and functions work together to achieve  something.


Check where the integers are Tprime or not The objective of the code is to check the given inputs are tprime or not.

Example:

3 //no of integers(1 ≤ n ≤ 10^5)
4 5 6 //space-separated integers xi (1 ≤ xi ≤ 10^12)

Output:

YES
NO
NO

The code works fine but always exceeds time limit 2000ms. How to reduce it?

import java.util.Scanner;
import java.util.ArrayList;
import java.io.BufferedReader;
import java.io.InputStreamReader;

public class TprimePractise {

public static void main(String[] args) throws Exception {
BufferedReader br =
new BufferedReader(new InputStreamReader(System.in));
String line = br.readLine();
int input = Integer.valueOf(line);
line = br.readLine();
long sqrt, x;
for (String num : line.split("\\s")) {
    x = Long.valueOf(num);

    if (x == 1) {
        System.out.println("NO");
        continue;
    }
    sqrt = (long) Math.sqrt(x);
    if (sqrt * sqrt == x && isPrime(sqrt)) {
        System.out.println("YES");
    } else {
        System.out.println("NO");
    }

  }
}

public static boolean isPrime(long num) {

for (int j = (int) num / 2; j >= 2; j--) {
    if (num % j == 0) {
        return false;
    }
}
return true;
 }

}

Change your algorithm

Currently, for each of \$10^5\$ numbers, you do a trial division loop that could check up to \$N/2\$ numbers, where \$N\$ can be as big as \$10^{12}\$.  This means that your program may require as many as \$10^{17}\$ operations.

You will certainly need to do better than that to finish within 2 seconds.

Proposal #1

The simplest way to speed up your program is to change your trial division loop.  I would make the following changes:

Check for divide by 2 as a special case before the loop.  In the loop, only check for division by odd numbers.
Make the loop go from small to large, as it is more likely that a random number will be divisible by a smaller number rather than a larger one.  In other words, start your loop at 3 and go up by 2 each loop.
Make the loop terminate at sqrt(n) instead of n/2.

Of the points above, #3 is the most important point.  This makes the trial division an \$O(\sqrt N)\$ operation instead of an \$O(N)\$ operation.  It reduces the max number of iterations from \$10^{12}\$ to \$10^6\$, making your total number of iterations in the worst case \$10^5 * 10^6 = 10^{11}\$.  So with this algorithm, the worst case probably wouldn't finish in 2 seconds, but the actual input might succeed.

Proposal #2

To make your program even faster, I would suggest the following:

Use the sieve of Eratosthenes to find all prime numbers from 2 to 10^6.  You can leave the output of the sieve as an array of booleans, telling you which numbers are prime.
Read in the input numbers.  For each input number, determine if it is a perfect square.  If it is, look up the square root of the input number in the array of booleans.  If the square root is prime, then the input number is a "tprime".

Part 1, the sieving part, takes around \$O(N * \log \log N)\$ time, where \$N\$ is 10^6, which means around \$4.3 * 10^6\$ steps.

Part 2 takes \$O(N)\$ time, where \$N\$ is 10^5, so around \$10^5\$ steps.

So the total of both parts comes out to roughly \$4.4 * 10^6\$ steps, which is much faster than proposal #1, and would definitely finish in under 2 seconds no matter what the input contained.

Thanks to maaartinus for suggesting the \$O(1)\$ lookup.


Stored procedure where column to ORDER BY depends on input parameters I'm getting three inputs and based on those three inputs, I'm returning. Input items may vary, so I used CASE WHEN statements here.

Is this code correct? If so, how can I make it more optimized?

ALTER Procedure [dbo].[WP_GetAllItems]
    @IsActive     bit,
    @OrderMode    bit,
    @OrderBy      varchar(75)
as
Begin
    Select ItemPartNumber, ItemDescription, CreatedDate, InitialPrice from Items where IsActive = @IsActive order by
    CASE WHEN @OrderBy='ItemDescription' AND @OrderMode = 0 THEN ItemDescription END ASC,
    CASE WHEN @OrderBy='ItemDescription' AND @OrderMode = 1 THEN ItemDescription END DESC,
    CASE WHEN @OrderBy='ItemPartNumber' AND @OrderMode = 0 THEN ItemPartNumber END ASC,
    CASE WHEN @OrderBy='ItemPartNumber' AND @OrderMode = 1 THEN ItemPartNumber END DESC,
    CASE WHEN @OrderBy='CreatedDate' AND @OrderMode = 0 THEN CreatedDate END ASC,
    CASE WHEN @OrderBy='CreatedDate' AND @OrderMode = 1 THEN CreatedDate END DESC,
    CASE WHEN @OrderBy='InitialPrice' AND @OrderMode = 0 THEN InitialPrice END ASC,
    CASE WHEN @OrderBy='InitialPrice' AND @OrderMode = 1 THEN InitialPrice END DESC
End

It looks like your code produces the intended results (I assume you've tested it), but not for the right reasons.

You misunderstood the way CASE ..  WHEN .. THEN .. END is intended to be used. Check out the official documentation. It has examples. There are supposed to be multiple WHEN .. THEN .. between the CASE and the END.

However, since there is just a single WHEN clause in each CASE clauses, if the WHEN clause is not satisfied, then the whole CASE .. END clause evaluates to NULL. ORDER BY NULL has no effect, so it turns out that only the clauses you want to use are being used. Lucky you.

However, it would improve the code to use CASE in the way it was intended. I can't do it all at once but I can refactor the code slightly.

ALTER Procedure [dbo].[WP_GetAllItems]
    @IsActive        bit,
    @OrderMode       bit,
    @OrderBy         varchar(75)
as
Begin
    Select ItemPartNumber, ItemDescription, CreatedDate, InitialPrice from Items where IsActive = @IsActive order by
    IIF( @OrderMode=0,
        CASE
            WHEN @OrderBy='ItemDescription' THEN ItemDescription
            WHEN @OrderBy='ItemPartNumber' THEN ItemPartNumber
            WHEN @OrderBy='CreatedDate' THEN CreatedDate
            WHEN @OrderBy='InitialPrice' THEN InitialPrice
        END, NULL ) ASC,
    IIF( @OrderMode=1,
        CASE
            WHEN @OrderBy='ItemDescription' THEN ItemDescription
            WHEN @OrderBy='ItemPartNumber' THEN ItemPartNumber
            WHEN @OrderBy='CreatedDate' THEN CreatedDate
            WHEN @OrderBy='InitialPrice' THEN InitialPrice
        END, NULL ) DESC
End


Guard from the missing and invalid function params Is there a nicer way to guard all the inputs against missing and invalid param(s)? A long list of if statement at the top seems so hard to read.

 deliveryForCategory(code, category, dateStart, dateEnd) {
    if (!code) {
      throw Error(ERROR_MISSING_CODE);
    }

    if (!category) {
      throw Error(ERROR_MISSING_CATEGORY);
    }

    if (!DELIVERY_CATEGORIES.has(category)) {
      throw Error(ERROR_INVALID_CATEGORY);
    }

    if (!dateStart || !dateEnd) {
      throw Error(ERROR_MISSING_DATETIME);
    }

    return get(
      `${URL_APP_BASE}/accounts/${code}/delivery/${category}/${dateStart}/${dateEnd}`
    );
  }

What you have done is generally the approach I take; however, if these guards are used a lot throughout the application, perhaps you could refactor them into separate functions:

deliveryForCategory(code, category, dateStart, dateEnd) {
    VerifyCode(code);
    VerifyCatergory(category);
    VerifyDate(dateStart);
}

Potentially name it something like ThrowErrorIfCodeInvalid(code) so it's super clear what it does.


MySQL data fetching without page refresh I've recently finished a prototype for a little Raspberry Pi website. The main page of the site displays current users found in the room (through bluetooth). I wanted this list updated regularly from data in a MySQL table, so no refresh is needed. When someone walks into or out of the room, the webpage shows almost instantly.

This is the solution I created:

index.html

Present:

Absent:

<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css">

var timer = setInterval(listLoad, 1000);

function listLoad(){
    $(function(){
       $("#list1").load("herelist.php");
       $("#list2").load("notherelist.php");
    });
}
</script>

herelist.php

<!DOCTYPE html>

<?php
$servername = "localhost";
$username = "xxxx";
$password = "xxxx";
$dbname = "xxxx";

// Create connection
$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
     die("Connection failed: " . $conn->connect_error);
}

$sql = "SELECT firstname, lastname FROM room_Data WHERE attendance = 1";
$result = $conn->query($sql);

if ($result->num_rows > 0) {
     // output data of each row
     echo("
")
     while($row = $result->fetch_assoc()) {
         echo ("

". $row["firstname"]. " " . $row["lastname"] . "

");
 }
     echo("
");
} else {
     echo ("");
}

$conn->close();
?>

notherelist.php

Same as herelist.php, just the where clause is = 0 instead of = 1

This project isn't for some major production scale, which is why I don't really mind hitting my MySQL server every second requesting a read. I actually like this solution a lot since it was my first time ever injecting PHP through jQuery and I thought it was a neat idea.

Is this a good solution for a personal project/school project? Or, is there something I should look into to improve this?

Personally, I hate having a setInterval in JS code, especially to
 fetch live updates. I hate it even more when it is used for hitting
 the server for a database read/write operation, irrespective the size
 of project.

The code you have is quite good, considering that it was your first time. There are quite a few suggestions though. Read on:

Instead of having 2 separate files to fetch data of attendance = 0 and attendance = 1, use a single file with a parameter passed via a GET or POST request.
Instead of dumping the entire data as HTML, I'd suggest outputting the results as JSON so that it might be of use to other applications, without having to resort to HTML parsers. This helps if you think/plan on providing an API for other users to develop on.
Since the data for room_data gets updated with an underlying python application, you can modify it to write the output to a static JSON file and hit this JSON content instead of executing a MySQL query every second. This will help as the browser will get a 304 response status from the server if the JSON was not updated since last fetch. Caching FTW ^_^
Put the external script/stylesheets in head.
Since all you need for the MySQL to return is concatenated name string, do so in MySQL itself:

SELECT CONCAT(firstname, ' ', lastname) AS 'name'
FROM room_Data
WHERE attendance = :something

Do not use h4 tags for list items.

If you follow (1) above, you won't need the (3). I strongly recommend using (3) though.


Navigating menus in automation tests I am trying to write a method that can find the child item in a menu. The below code works but is not exactly elegant or scaleable. Is there another way to find the path to child menu item that could be used?

public class Item
{
    public string Name { get; set; }
    public string Parent { get; set; }
}

public void OpenMenuItem(string name)
{
    var items = new List<Item>
    {
        new Item  { Name = "Test", Parent = null },
        new Item  { Name = "TestChild1", Parent = "Test" },
        new Item  { Name = "TestChild2", Parent = "Test" },
        new Item  { Name = "TestGranChild1", Parent = "TestChild1" },
        new Item  { Name = "TestGranChild2", Parent = "TestChild1" },
        new Item  { Name = "TestGranChild3", Parent = "TestChild2" },
        new Item  { Name = "TestGranChild4", Parent = "TestChild2" }
    };
    var menuItem = items.Where(x => x.Name == name);
    if (menuItem.FirstOrDefault().Parent != null)
    {
        var parent1 = items.Where(x => x.Name == menuItem.FirstOrDefault().Parent);
        if (parent1.FirstOrDefault().Parent != null)
        {
            var parent2 = items.Where(x => x.Name == parent1.FirstOrDefault().Parent);
            app.Tap(x => x.Marked(parent2.FirstOrDefault().Name));
        }
        app.Tap(x => x.Marked(parent1.FirstOrDefault().Name));
    }
    app.Tap(x => x.Marked(menuItem.FirstOrDefault().Name));
}

Make parent a list of ancestors as a path to the item:

new Item  { Name = "TestGranChild4", Path = {"Test", "TestChild2"} },

var menuItem = items.Where(x => x.Name == name);
menuItem.FirstOrDefault().Path.ForEach(a => app.tap(x=>x.Marked(a)));
app.Tap(x => x.Marked(menuItem.FirstOrDefault().Name));


Reading event logs of several computers This is my first PowerShell script. It reads the event logs of several computers in the local network and lists error/warning messages written to them in the last 24 hours, if any.

I'll appreciate any suggestion to improve my code.

$computers = 'COMP1','COMP2','COMP3','COMP4'
$logs = 'Log1','Log2','Log3','Log4'
$yesterday = (Get-Date).AddDays(-1)

foreach ($comp in $computers) {
    try {
        $events = Get-WinEvent @{logname=$logs; starttime=$yesterday; level=2,3} -ComputerName $comp -ErrorAction Ignore;

        if ($events.count -gt 0) {
            $count = $events.count;
            "$comp $count event(s):";
            $events | select logname, providername, timecreated, leveldisplayname, message `
                    | Format-Table -AutoSize;
        } else {
            "$comp no events";
        }
    }
    catch {
        "$comp error!!! $_";
    }
}

Basics

Aliases

PowerShell is meant to be both shell and programming language. As such, it has a concept of Aliases. An Alias is another name for a command. An alias can resolve to a cmdlet, a function, or even a native application. You can create your own aliases, but there are already many built in.

Aliases are great for quickly typing out commands in the shell or prototyping, but I and many others strongly believe you should avoid using them in scripts.

Try to use the full name of each command, as this can reduce ambiguity and increase code clarity.

In your script you only use one: select

This is an alias to Select-Object.

Parameters

Many (most) PowerShell commands accept parameters both as named parameters (MyCommand -ParamName ParamValue) and positional parameters (MyCommand ParamValue).

Similarly to the preference for using full command names, it is recommended to always use named parameters. In your script for example, so you are doing select logname, providername, timecreated (truncating the rest).

What you may not realize is that you are passing an array to a single parameter, and that parameter does in fact have a name: Property (the same is true for your first parameter to Get-WinEvent, whose first parameter in your case is called FilterHashTable).

Expanded out:

$events | Select-Object -Property logname,providername,timecreated

Although not in use in your script, you should be aware of the following:

Parameter names can also have aliases. So for example Get-WinEvent -ComputerName $comp could also be written Get-WinEvent -cn $comp. Aliases have to be defined; they aren't automatic.
Parameter names accept any unambiguous stub of the name. So, you can use a partial name, like Get-WinEvent -FilterH @{}. In this example, you can't use Get-WinEvent -Filter because there are also -FilterXml and -FilterXPath parameters, but you can do down to a single character as long as it's unambiguous for that command.

Both of these are good to know, but again should be avoided in scripts.

Format- commands

PowerShell makes heavy use of pipelines, but unlike most shells, PowerShell sends complete objects throughout its pipeline.

This is a great thing, and it's what allows you to do complex filtering and manipulation.

But when you see the output on the screen and it shows you a table or list of information, it's easy to think just think of it as formatted text.

PowerShell has Format- cmdlets that let you display information explicitly the way you want (since normally the host chooses a format based on the data types or amount of information).

Here's the really important thing to remember: Format- cmdlets are for end-result, final display only.

Once you send an object through a Format- cmdlet, it is no longer the original object; you've lost the original.

In almost all cases, it's best to send the original object, then let the caller or use decide what they want to do with it (whether it's format it, convert it to another data type, save is as a CSV, or whatever).

It's hard to tell what you want to do with the data in your script, but since you use Format-Table you've already excluded all of your other options.

Backtick ` as line continuation character

The backtick ` is the escape character in PowerShell. The standard backslash \ used in most languages is not used here because backslash is also the path separator on Windows, and since PowerShell is supposed to work as a shell and was born on Windows, this would be inconvenient. The backtick was chosen instead.

When you use backtick at the end of a line, you are in essence "escaping" the newline for the purposes of the parser.

The unfortunate thing is that the backtick is very small, and very difficult to see, so it can make things confusing.

I highly recommend that you avoid the backtick for line continuation at all costs.

Luckily, you do have some other options, as there are other points in commands and statements where you can start a newline without the need for any special character.

Pipe |

Since pipes are so common in PowerShell and you may be doing a pipeline that includes several statements, it's very useful to know that you can use a newline after a pipe (but not before). So the following works without any line continuation:

$events |
    Select-Object -Property logname,providername |
    Sort-Object -Property logname

(indentation after the first pipe is a stylistic choice, not at all required)

Operators

You can use a newline directly after an operator:

if ($var -gt 5 -and
    $var -lt 100) {
    # code
}

For long conditionals, this is essential.

ScriptBlocks {}

Any scriptblock is essentially a whole script, so you can use newlines liberally inside it, even if you were passing the scriptblock as a parameter. As soon as the scriptblock is opened, go for it!

Invoke-Command -ScriptBlock {
    # A whole new script here
    Get-Service |
        Get-Random |
        Stop-Service -Verbose -WhatIf
}

In grouping expressions () and sub-expressions $()

Using Grouping and Sub expressions even where not necessary can help if you feel the code needs better formatting. You can use newlines inside them as needed:

Get-Service -Name (
    'Win*'
)

Get-Service -Name (
    "Win$(
        '*'
    )"
)

Those examples are convoluted, but you get the idea.

In array/hashtable definitions @()/@{}

I use these a lot. Format as needed within these. Note that in a hashtable definition, if you are used to using semicolons ; to separate items on a single line (like @{ Key1 = 'val1' ; Key2 = 'val2' } the newlines take the place of the semicolon.

# Hashtable
@{
    Key1 = 'val1'
    Key2 = 'val2'
}

# Array
@(
    1,2,3
)

@(
    1,
    2,
    3
)

@(
    1 ,
    2
    ,3
) # why oh why

# my preferred (long) array style:
@(
     1
    ,2
    ,3
)

Combined:

@{
    Set1 = @{
        Arr1 = @(
             'A'
            ,'B'
            ,'C'
        )

        Arr2 = @(
             1
            ,2
            ,3
        )
    }

    Set2 = @{
        Scalar = 10
    }
}

In a command:

Get-WinEvent -FilterHashTable @{
    logname = $logs
    starttime = $yesterday
    level = @(
         2
        ,3
    ) # I wouldn't expand the array like this for 2 elements, just an examples
} -ComputerName $comp

Errors and Exceptions

PowerShell has a concept of terminating vs. non-terminating errors. Different commands and different code determine whether an error stops execution or whether it just reports an error and keeps going. Most commands allow you to override whatever their default is with the -ErrorAction common parameter.

In your code, you are using try/catch, but you've added -ErrorAction Ignore to Get-WinEvent.

Errors that are non-terminating, which includes an error you've told PowerShell to ignore or ContinueSilently, will not trigger a try/catch, so I believe your catch block will never run even if there is an error.

Instead, you probably want -ErrorAction Stop, which is supposed to force all errors to become terminating errors, so that they do trigger the exception handling.

Truthiness and Falsiness

PowerShell tries pretty hard to convert or coerce values into the type that is required. In the case of a boolean value, like that used in an if statement, almost anything can be interpreted as true or false.

The following all evaluate to $false with no additional action or conversion needed:

$false
0
$null
Empty string
Empty array
Uninitialized variable (not the same as one containing $null)

Just about everything else, including, confusingly, an empty hashtable, is interpreted as $true.

So what does this mean for you?

if ($events.count -gt 0)

Can be reduced to:

if ($events.count)

Which could even further be reduced to:

if ($events)

This is useful to check for lots of things; return values of course, but also whether a parameter contains a value or is otherwise available.

Pipeline Output

When you do something like this:

"Some thing"

You're used to it showing up on the screen, so it's easy to think that this is the same as a sort of print statement (which in PowerShell would be Write-Host).

But that isn't what's happening; it's actually implicitly calling Write-Output, which sends that to the output stream.

If the output stream makes its way back to the host, then the host decides what to do with it; in the case of powershell.exe (or ISE) it decides to display it, but in the case of a function for example, this becomes part of the return value. In the case of a status message that's not really what you want.

Here's an example:

function Test-Output1 {
    "I'm returning 5"
    5
}

function Test-Output2 {
    Write-Host -Object "I'm returning 5"
    5
}

Now, if you run these directly:

Test-Output1
Test-Output2

You will see the exact same output. They are indistinguishable that way.

But try to assign the result to a variable instead:

$v1 = Test-Output1
$v2 = Test-Output2

Now, you will see the difference. Test-Output1 shows nothing on the screen. If you check the value of $v1, it will be an array, with 2 elements. The first element is a string (your message), the second element is the number.

Test-Output2 on the other hand showed a message anyway, and the value of $v2 is only the number.

That's the critical difference.

So even if you're in a situation where you can't see the difference, you should always be thinking about it. "Am I trying to show a message to a person on the screen, or am I trying to return a value to somewhere else in the program?"

Details

Ok, so now some stuff that actually has to do with the task you're trying to achieve!

Parameters

This is something of a general language element thing too, but instead of hardcoding variables at the top of your script, consider adding parameters to the script so that as a caller you can specify the values you want.

Let's look at replacing the first 3 lines of your script:

$computers = 'COMP1','COMP2','COMP3','COMP4'
$logs = 'Log1','Log2','Log3','Log4'
$yesterday = (Get-Date).AddDays(-1)

With this:

[CmdletBinding()]
param(
    $Computers,
    $Logs,
    $StartTime
)

Now, I renamed $yesterday to $StartTime so that you can specify the date you want when calling, so that variable would have to change, but the entire rest of your script would work the same. If your script is named Get-MyLogs.ps1 then you can call it like this:

.\Get-MyLogs.ps1 -Computers 'COMP1','COMP3' -Logs 'Log1','Log9999' -StartTime (Get-Date).AddDays(-1)

But maybe you almost always want to use "yesterday" and it's too annoying to enter that value every time. You can provide your parameter with a default value instead:

[CmdletBinding()]
param(
    $Computers,
    $Logs,
    $StartTime = $((Get-Date).AddDays(-1))
)

Now you can leave that parameter off, and it will use the default, but you can still explicitly set a value to override it. This gives you a great amount of flexibility, without having to edit the script, and it required almost no effort.

I didn't even go into adding data types to the parameters, validation, etc.

I will only add that there are some naming conventions you should follow for parameters.

Typically plural is not used, so even if you want to accept multiple logs, you would probably name that parameter $Log and not $Logs.

Accepting a computer name is common and the parameter is pretty much always called $ComputerName (again, even if multiple names are accepted). It's good to follow these conventions. You can add aliases to your parameters if you want to support non-standard additional names, but real name should generally follow convention when possible.

Parallelization

Right now, you go through the list of computers sequentially. For more than a few computers, this is going to be slow. If any of the computers are not available, and have to time out, it's going to be really slow.

Before going further, I'll just say this: if it's ok that it's slow, then stop here. Let it be slow. Sequential, non-parallelized code is easy to write, easy to understand. If this just runs as a background task or whatever, don't complicate it.

But if you want to check multiple computers at once, you can.

PowerShell sucks at this, but there are options.

The built-in ways on Windows are Workflows and Jobs.

Workflows are weird. They're a bit complicated, difficult to understand all the implications, and hard to find good help on, because nobody uses them. They are also not going to be included in PowerShell core since they only work on Windows so I don't recommend this. For very simple things, it might be a good way to go.

Jobs are supported through the *-Job* cmdlets, and the -AsJob parameter that's available on some commands.

Jobs actually run as separate PowerShell processes so they are a bit expensive in terms of start time and resource usage (especially with large numbers of them). Also because they are not in-process, any objects passed between them are serialized/deserialzed so some functionality may be lost.

If you start a scriptblock in a job, you then have a job object which you can wait on, check the progress on, and then when ready receive its output.

There are more advanced ways to do parallel workflows like runspaces, which you can do yourself, or use a library like Boe Prox's excellent PoshRsJob module.

I'm not going to get any deeper on this since I don't even know your intention and there's so much to write. If you try to parallelize this, try some stuff, try different methods, and use Stack Overflow as needed.

Conclusion

Despite my novel, your code is pretty straightforward. Without knowing how you intend to use the data it's hard to be opinionated on any deeper design changes, so clearly most of my suggestions are general language stuff.


Get file absolute path from file URL in PHP Reading several questions regarding the topic, I made my function to return file's absolute path from the file URL. As I'm using this in an WordPress environment, hence I availed untrailingslashit() in it.

/**
 * Get file absolute path from URL.
 *
 * @author  carlosveucv
 * @link    https://stackoverflow.com/a/17744219/1743124
 *
 * @param   string $file_url File URL.
 * @return  string           File path.
 * --------------------------------------------------------------------------
 */
function wp20171021_get_file_path_from_url( $file_url ) {
    $file_path = parse_url( $file_url, PHP_URL_PATH );
    return wp_normalize_path( $_SERVER['DOCUMENT_ROOT'] . $file_path );
}

How bulletproof is this?
What kind of caveats this might have in real world?
I tested this in Windows, so is it 'cross platform-friendly?

This should be the most cross platform way and uses less memory (also doesn't depend on wp)

function get_file_path_from_url( $file_url ){
   return realpath($_SERVER['DOCUMENT_ROOT'] . parse_url( $file_url, PHP_URL_PATH ));
}

It uses the native PHP function realpath().


Rule Of Three for a Coordinate class I just recently got back into C++ and did a quick and simple exercise in the Rule of Three. The code for the copy constructor, overloaded assignment operator, and destructor follows below for my class (code is also below), and I was wondering if someone could take a quick look at it and point out anything that I overlooked.

class Coordinate
{
    private:
        int x;
        int y;
        string *name;

    public:
        Coordinate(int X, int Y, string Name)
        {
            this->x=X;
            this->y=Y;
            *(this->name)=Name;
        }

        //COPY CONSTRUCTOR
        Coordinate(const Coordinate& source)
        {
            this->x=source.x;
            this->y=source.y;
            this->name=new string(*(source->name));
        }

        //OVERLOADED ASSIGNMENT OPERATOR
        Coordinate& operator= (const Coordinate& source)
        {
            //check for self-assignment aka Coordinate1=Coordinate1
            if(this==&source)
            {
                return *this;
            }

            this->x=source.x;
            this->y=source.y;
            this->name=*(source->name);

            return *this;
        }

        //DESTRUCTOR
        ~Coordinate()
        {
            delete name;
        }

        void setX(int X)
        {
            this->x=X;
        }

        void setY(int Y)
        {
            this->y=Y;
        }

        void setName(string Name)
        {
            *(this->name)=Name;
        }

        const int getXValue()
        {
            return this->x;
        }

        const int getYValue()
        {
            return this->y;
        }

        const string getName()
        {
            return *(this->name);
        }
};

This post corrects some errors that led to my previous question to be rejected.  I realize that I should have included all the code for my class from the beginning.

This unfortunately shows a rather erroneous understanding of the rule of three. Yes, all three methods are present, but they do not serve their expected purposes.

Your copy constructor doesn't free the old name before assigning the new one. That's a memory leak.
Your copy constructor dereferences an unitialized pointer. That's undefined behavior (almost certainly a segfault in this case).
Your assignment operator makes a shallow copy. Not only is it unintuitive to consumers since objects may affect each other, the destructors would attempt to double free the same object likely resulting in a segfault.

Provided the other two were fixed, your destructor is actually correct.

This code may technically compile, and it may even execute, but I would suggest that you run it through something like valgrind before continuing much farther. In addition, it might help to read more about the Rule of 3, learn about the copy and swap idiom, and learn about the Rule of 0, or more generally RAII and idiomatic resource management.

From a design/style perspective:

This isn't a Coordinate. It's a coordinate with a name, perhaps a NamedCoordinate. I would expect the x,y to be encapsulated in some kind of struct in real code and then a NamedCoordinate to contain an instance of that.
Arguments that are not modified should be taken by const reference rather than value. For example, void setName(string Name) makes an unnecessary copy of Name. This gets a bit more complicated in C++11, I'm afraid as your taking parameters by value would be correct, but you would missing std::move calls. Anyway, for C++ < 11, you just need to know that non-modified parameters should be taken by const reference.
using namespace std; is a bad habit
It's customary to leave off this unless it must be included in function calls or member accesses.
snake_case and camelCase are the standards for argument/parameter/member/etc names with either snake_case or PascalCase for classes. Your class name is good, but some of your uppercased parameter names are rather unusual.


Activate submit button on form change and detach event listener We have pages that contain 1-3 forms. Each form does have a submit button which is styled as inactive* as long as the user didn't change the form.

Now I need to:

remove the class from the button as soon as the form changes
remove the event listener from the form

I came up with this:

for (const form of Array.from(document.getElementsByClassName('form'))) {
    const formHandlerChange = event => {
        const submit = form.getElementsByClassName('disabled')[0];
        submit.classList.remove('disabled');
        form.removeEventListener('change', formHandlerChange);
    };

    form.addEventListener('change', formHandlerChange);
}

Is this a good solution? Is it a bad thing to create formHandlerChange for each form? Would it be better to get the form each time formHandlerChange is entered manually and have this function defined only once outside the loop?

* It's also disabled but I removed it for simplicity of this review.

If you use event.currentTarget instead of form, you can declare the event handler function once and reuse it:

function formHandlerChange(event) {
    const form = event.currentTarget;
    const submit = form.querySelector('.disabled');
    submit.classList.remove('disabled');
    form.removeEventListener('change', formHandlerChange);
}

for (const form of document.querySelectorAll('form')) {
    form.addEventListener('change', formHandlerChange);
}

(Note: I also used querySelector() and querySelectorAll() as they are more flexible than the various getElementBy methods)


Using a MutationObserver to reformat numbers in the user's locale I mostly just want to make sure I've understood the specs correctly with this code, but here goes:

var nums = document.getElementsByTagName('x-number');
var observer = new MutationObserver(replaceNumerics);
var config = { childList: true, subtree: true };
observer.observe(document.body, config);
replaceNumerics(); // initial load

function replaceNumerics() {
    var l = nums.length, i, span;
    for( i=l-1; i>=0; i--) { // node list is live so iterate backwards
        span = document.createElement('span');
        span.classList.add("x-number");
        span.textContent = parseFloat(nums[i].textContent).toLocaleString();
        nums[i].parentNode.replaceChild(span,nums[i]);
    }
}

The idea is to have a custom <x-number> tag that contains... a number, obviously. JavaScript will detect these tags, parse out the number, then render it according to the user's locale. I'm planning to do a similar thing for dates, which will be especially useful since <input type="date" /> already uses the user's locale.

My biggest concern is I think this will result in an infinite loop, as the replaceChild call will fire the MutationObserver again... but if I understand the spec correctly, this second firing will only happen if and when all <x-number> elements have been replaced (as opposed to the earlier DOMSubtreeModified event which would fire immediately), at which point there would be no more <x-number> elements to replace and the loop would end.

Honestly this is all very unfamiliar territory to me, but I believe this should work fine... But I figured I'd subject it to public opinion before going any further with it.

One issue I'm seeing is that you're listening to the entire tree. While not an issue in small apps, it might be an issue for huge apps, with deep trees. Consider limiting it to the places where you expect your custom tag will exist.

If you are in control of the process of adding these elements to the DOM, consider doing this transformation there instead of listening to the entire DOM for you to change it. It will be easier to debug since the transformation is involved in the process of generating that element, instead of an indirect side-effect of some other operation.

You know that feeling when someone else's JavaScript re-arranges the HTML you rendered on the backend and you had to hunt it down? Same story.


Asynchronous JavaScript to fetch remote JSON data with promises and generators I am trying to figure out how to do async calls using promises and generators (without 3rd party libraries). Most of the examples I found on the Internet are either incomplete or confusing (at least to me), so I decided to try and write the code myself.

I would really appreciate it if someone could review the code and provide comments and suggestions for improving it.

function getJSON(url) {
    return new Promise((resolve, reject) => {
        const https = require("https");
        try {
            https.get(url, (response) => {
                let data = "";
                response.on("data", (part) => {
                    data += part;
                });
                response.on("end", () => {
                    try {
                        resolve(JSON.parse(data));
                    }
                    catch(e) {
                        reject("Error parsing data");
                    }
                });
            }).on("error", () => {
                reject("Error downloading data");
            });
        }
        catch(e) {
            reject("Error: " + e.message);
        }
    });
}

function request(generator) {
    var iterator = generator();
    function process(result) {
        if (result.done) {
            return;
        }
        const value = result.value;
        if (value instanceof Promise) {
            value.then(
                (data) => {
                    console.log(data);
                    process(iterator.next());
                }
            )
            .catch(
                err => {
                    iterator.throw(err);
                }
            );
        }
    }
    try {
        process(iterator.next());
    }
    catch(e) {
        iterator.throw(e);
    }
}

function *DataGenerator() {
    try {
        yield getJSON("https://jsonplaceholder.typicode.com/posts/1");
        yield getJSON("https://jsonplaceholder.typicode.com/posts/2");
        // yield getJSON("https://jsonplaceholder.typicode.com/"); // Parsing error
        // yield getJSON(""); // Domain name error
    }
    catch (e) {
        console.log(e);
    }
}

// Run

console.log("Before");
request(DataGenerator);
console.log("After");

This is also available as a repo.

Why do not use async / await functions? Its just syntactic sugar for promises / generators. Check this vanilla JS snippet, is live:

var url1 = 'https://jsonplaceholder.typicode.com/posts/2'
var url2 = 'https://jsonplaceholder.typicode.com/posts/3'

async function mimicSyncInside () {
  console.log('Before')
  console.log(await fetch(url1).then(resp => resp.json()))
  console.log(await fetch(url2).then(resp => resp.json()))
  console.log('After')
}

mimicSyncInside()


Faults using mod_rewrite I never really bothered using mod_rewrite to achieve SEO friendly names but I've always admired from a distance because I've never been good at regex.

The below .htaccess file does work, but I'm sketchy about all of it and maybe I've structured it incorrectly, furthermore it looks terrible and I know that I'm doing it wrong.

It's the very last line that has my most concerns, it sends anything other than what is matched in the first lookup to pageLoader.php and then that obviously loads the view.

Any tips, even small things would assist as this is something I want to get my head around.

RewriteEngine On

ReWriteRule ^css/custom.css$ PlugCMS/public/genCSS.php
ReWriteRule ^js/custom.js$ PlugCMS/public/genJS.php

ReWriteRule ^plugcms$ PlugCMS/index.php [NC]

ReWriteRule ^send-mail$ PlugCMS/public/mailhandler.php [NC]
ReWriteRule ^index\.php$ PlugCMS/public/pageLoader.php?slug=/ [NC]

ReWriteRule ^(?=^(?:(?!PlugCMS|css|js|img|robots.txt|sitemap.xml|font|fonts|sandbox|private|output).)*$)([A-Za-z0-9-]+).*$ PlugCMS/public/pageLoader.php?slug=$1 [NC,L]

The thing that leaps out at me is that the normal way to avoid redirecting valid files to the pageLoader.php handler is to use RewriteCond like so:

RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . PlugCMS/public/pageLoader.php [L]

Doing it this way means that you can easily add a new file to the site without having to add to the RewriteRule to avoid it matching.

You usually shouldn't have to use the slug hack to get the original URL.  See here for more discussion.

You may want to set a RewriteBase for this.  Perhaps

RewriteBase /PlugCMS/

Which would make your rewrite rules a little shorter.  See here for more discussion.


C++ string_cast template function In C++, to simplify string conversion between std::string and std::wstring, I created the following utility template functions:

#pragma once

#include <vector>
#include <string>
#include <cstring>
#include <cwchar>
#include <cassert>

template<typename Td>
Td string_cast( const char* pSource, unsigned int codePage = CP_ACP );

template<typename Td>
Td string_cast( const wchar_t* pSource, unsigned int codePage = 1200 );

template<typename Td>
Td string_cast( const std::string& source, unsigned int codePage = CP_ACP );

template<typename Td>
Td string_cast( const std::wstring& source, unsigned int codePage = 1200 );

template<>
std::string string_cast( const char* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    return std::string( pSource );
}

template<>
std::wstring string_cast( const char* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    std::size_t sourceLength = std::strlen( pSource );
    if( sourceLength == 0 )
    {
        return std::wstring();
    }

    int length = ::MultiByteToWideChar( codePage, 0, pSource, sourceLength, NULL, 0 );
    if( length == 0 )
    {
        return std::wstring();
    }

    std::vector<wchar_t> buffer( length );
    ::MultiByteToWideChar( codePage, 0, pSource, sourceLength, &buffer[ 0 ], length );

    return std::wstring( buffer.begin(), buffer.end() );
}

template<>
std::string string_cast( const wchar_t* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    size_t sourceLength = std::wcslen( pSource );
    if( sourceLength == 0 )
    {
        return std::string();
    }

    int length = ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, NULL, 0, NULL, NULL );
    if( length == 0 )
    {
        return std::string();
    }

    std::vector<char> buffer( length );
    ::WideCharToMultiByte( codePage, 0, pSource, sourceLength, &buffer[ 0 ], length, NULL, NULL );

    return std::string( buffer.begin(), buffer.end() );
}

template<>
std::wstring string_cast( const wchar_t* pSource, unsigned int codePage )
{
    assert( pSource != 0 );
    return std::wstring( pSource );
}

template<>
std::string string_cast( const std::string& source, unsigned int codePage )
{
    return source;
}

template<>
std::wstring string_cast( const std::string& source, unsigned int codePage )
{
    if( source.empty() )
    {
        return std::wstring();
    }

    int length = ::MultiByteToWideChar( codePage, 0, source.data(), source.length(), NULL, 0 );
    if( length == 0 )
    {
        return std::wstring();
    }

    std::vector<wchar_t> buffer( length );
    ::MultiByteToWideChar( codePage, 0, source.data(), source.length(), &buffer[ 0 ], length );

    return std::wstring( buffer.begin(), buffer.end() );
}

template<>
std::string string_cast( const std::wstring& source, unsigned int codePage )
{
    if( source.empty() )
    {
        return std::string();
    }

    int length = ::WideCharToMultiByte( codePage, 0, source.data(), source.length(), NULL, 0, NULL, NULL );
    if( length == 0 )
    {
        return std::string();
    }

    std::vector<char> buffer( length );
    ::WideCharToMultiByte( codePage, 0, source.data(), source.length(), &buffer[ 0 ], length, NULL, NULL );

    return std::string( buffer.begin(), buffer.end() );
}

template<>
std::wstring string_cast( const std::wstring& source, unsigned int codePage )
{
    return source;
}

Are there any bugs or further optimizations?

While your approach is simple and straightforward to implement there are some important drawbacks to realize.

First, it doesn't take advantage of the power templates offer. Since you're specializing for every possible usage of string_cast there's no opportunity for the compiler to generate code for you. A consequence of this is that you have a lot of 'clipboard heritance', copy and pasting the same function and changing parts of it to do what you want. This is a form of code duplication.

The approach does not lend itself to extendibility. What happens if you want to add support for another string type later on? The combination of functions you have to write would explode!

So there're clearly opportunities for some major improvement. Let see how we can refactor this so that it better adheds to DRY. If you take a step back and think about how string_cast is used you'll find there are really just 3 scenarios it has to support:

cast to the same string type.
cast to a different string type.
cast from a raw pointer representation into a string type.

Each of these cases can be handle by writing a template for them. Starting with the string_cast function that acts as an interface:

template <typename Td, typename Ts>

string_cast now takes 2 template parameters. Keeping your naming convention, I use Ts to indicate the source type. (TO and FROM are probably better names.)

Td string_cast(const Ts &source)
{

We use type deduction to identify what we're casting from.

    return string_cast_imp<Td, Ts>::cast(source);

Once we know what type Td and Ts is we delegate to string_cast_imp and the appropriate template will be instantiated.

}

Let's handle the easy case first:

template <typename Td>
struct string_cast_imp<Td, Td>
{
    static const Td& cast(const Td &source)
    {
        return source;

For casting to the same string type, we don't need to do anything. Just return back what was given. Since this is nothing more than a pass-through returning by reference is ok. string_cast will make a copy before going out of scope since it's return by value.

    }
};

Now for the important case, the reason for writing string_cast in the first place! The basic process is the same, only certain aspects are different:

conversion function used. eg. WideCharToMultiByte vs MultiByteToWideChar
buffer type used. eg. vector<char> for string vs vector<wchar_t> for wstring
string type returned. That's captured by our template parameter Td so we don't have to worry about this as much.

You can extract those differences into a trait-like policy class.

template <typename Td, typename Ts>
struct string_cast_imp
{
    static Td cast(const Ts &source)
    {
        int length = string_traits<Ts>::byte_convert( CP_ACP, source.data(), source.length(),
                                                      NULL, 0 );
        if( length == 0 )
        {
            return Td();
        }

Here I removed the string.empty() check since it's not really needed. If the string is empty length will be 0 anyway so this is properly handled.

        vector< typename string_traits<Td>::char_trait > buffer( length );

Here we use our policy class to tell us the proper character-type to use for our buffer. If Td = string then string_traits<Td>::char_trait will be a char. If it's a wstring then string_traits<Td>::char_trait will evaluate to a wchar_t.

        string_traits<Ts>::byte_convert( CP_ACP, source.data(), source.length(),
                                                 &buffer[ 0 ] , length );

Similiarly, byte_convert acts as a wrapper to the correct byte function to call. This attrib is captured by our policy class as well.

        return Td( buffer.begin(), buffer.end() );
    }
};

We define our string_traits policies like this:

template <typename T>
struct string_traits;

Declare the general base-form but don't define it. This way if code tries to cast from an illegit string-type it will give a compile error.

template <>
struct string_traits<string>
{
    typedef char char_trait;
    static int byte_convert(const int codepage, LPCSTR data  , int data_length,
                                                LPWSTR buffer, int buffer_size)
    {

You might want to play around with the parameters it accepts but this should give you the general idea.

        return ::MultiByteToWideChar( codepage, 0, data, data_length, buffer, buffer_size );
    }
};

And now for the last case. For raw pointer types we can just wrap it into an appropriate string type and call one of our above string functions. We have to overload string_cast here because our base form accepts a reference type. Since reference types to arrays do not decay into a pointer type, this second template form will specifically handle that case for us.

template <typename Td, typename Ts>
Td string_cast(Ts *source)
{
    return string_cast_imp<Td, typename string_type_of<const Ts *>::wrap >::cast(source);

Notice I'm using const Ts * as template parameter for string_type_of. Regardless of whether Ts is const or not we always use template form <const Ts *> to get the information we need.

}

string_type_of is another policy we define as follows:

template <typename T>
struct string_type_of;

This policy class tells us what string-type to use for a given raw pointer type.

template <>
struct string_type_of<const char *>
{
    typedef string wrap;
};

With this refactor, you've reduced the number of written functions from 8 down to just 4 and eliminated code duplication. Perhaps more importantly adding support for another string-type is significantly easier, just specialized another policy for that string-type and you're done.


Optimize slow SQL query There is a application with a very slow query (listed below), and I have to optimize to run faster. The truth is that I don't know where to start. Any help? Thanks!

SELECT DISTINCT v.order_ANC,
                v.order_ANC2,
                f.codi_arxiu,
                f.nom_arxiu,
                f.codi_grup,
                f.codi_fons_refe,
                f.codi_fons,
                f.nom_fons,
                f.any_ini,
                f.any_fi,
                f.cronologia,
                SUM (num_uni_imatges) AS num_uni_imatges,
                SUM (unitats_text) AS unitats_text,
                SUM (unitats_notext) AS unitats_notext
  FROM (  SELECT f.codi_arxiu,
                 f.nom_arxiu,
                 f.codi_grup,
                 f.codi_fons_refe,
                 f.codi_fons,
                 f.nom_fons,
                 f.any_ini,
                 f.any_fi,
                 f.cronologia,
                 SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
                 COUNT (u.codi_unitat) AS unitats_text,
                 0 AS unitats_notext
            FROM ianc_fons f
            JOIN ianc_unicat u ON ( f.codi_arxiu = u.codi_arxiu AND f.codi_fons = u.codi_fons )
           WHERE     1 = 1
                 AND u.anc_unicat_id IN
                        (SELECT DISTINCT u.anc_unicat_id
                           FROM ianc_unicat u
                          WHERE     1 = 1
                                AND u.codi_arxiu = '1'
                                AND u.codi_grup = 'ANC'
                                AND u.codi_fons = 1)
        GROUP BY f.codi_arxiu,
                 f.nom_arxiu,
                 f.codi_grup,
                 f.codi_fons_refe,
                 f.codi_fons,
                 f.nom_fons,
                 f.any_ini,
                 f.any_fi,
                 f.cronologia
        UNION
          SELECT f.codi_arxiu,
                 f.nom_arxiu,
                 f.codi_grup,
                 f.codi_fons_refe,
                 f.codi_fons,
                 f.nom_fons,
                 f.any_ini,
                 f.any_fi,
                 f.cronologia,
                 SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
                 0 AS unitats_text,
                 COUNT (u.nt1_unitat) AS unitats_notext
            FROM ianc_fons f
            JOIN ianci_unicats u ON (     f.codi_arxiu = u.nt1_codi_arxiu
                                      AND f.codi_fons = u.nt1_codi_fons )
           WHERE     1 = 1
                 AND u.nt1_id IN
                        (SELECT DISTINCT u.nt1_id
                           FROM ianci_unicats u
                          WHERE     1 = 1
                                AND u.nt1_codi_arxiu = '1'
                                AND u.codi_grup = 'ANC'
                                AND u.nt1_codi_fons = 1)
        GROUP BY f.codi_arxiu,
                 f.nom_arxiu,
                 f.codi_grup,
                 f.codi_fons_refe,
                 f.codi_fons,
                 f.nom_fons,
                 f.any_ini,
                 f.any_fi,
                 f.cronologia
       ) AS f
LEFT JOIN v_ianc_arxius_order v ON ( f.codi_arxiu=v.codi_arxiu )
GROUP BY v.order_ANC,v.order_ANC2,f.codi_arxiu, f.nom_arxiu, f.codi_grup, f.codi_fons_refe,
         f.codi_fons, f.nom_fons, f.any_ini,  f.any_fi, f.cronologia
ORDER BY v.order_ANC,v.order_ANC2,f.nom_arxiu, f.nom_fons

I suspect one bottleneck is the grouping in the subqueries. For each row in the ianc_fons table the rows are being duplicated - one for each join the chile table ianc_unicat. And then they are being grouped again.

I would try a different approach and move the summing into a joined subquery :

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       child.num_uni_imatges,
       child.unitats_text
 FROM ianc_fons f
 LEFT JOIN
      ( SELECT codi_arxiu, codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.codi_unitat) AS unitats_text
      FROM ianc_unicat u
      WHERE u.anc_unicat_id IN
                    (SELECT u.anc_unicat_id
                       FROM ianc_unicat u
                      WHERE     1 = 1
                            AND u.codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.codi_fons = 1)
      GROUP BY codi_arxiu, codi_fons
      ) AS child ON f.codi_arxiu = child.codi_arxiu AND f.codi_fons = child.codi_fons

Note I have removed the DISTINCT from the Select from ianc_unicat. As this is just used in the filtering, the DISTINCT is not necessary. Using DISTINCT is always an overhead as the query engine needs to sort the records and then compare them all one by one to remove duplicates. Avoid as much as possible.

Each side of the UNION is selecting from the same table, and then summing different child records.

As the summing is now in a separate joined subquery, you can get rid of the union, and just join in the other table in a similar fashion. The Select will now need to sum up the totals from both joined subqueries.

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       COALESCE(child.num_uni_imatges, 0) + COALESCE(child2.num_uni_imatges, 0) AS num_uni_imatges,
       child.unitats_text,
       child2.unitats_notext

Note the use of COALESCE. As you are left joining these totals now, there is a chance that the values may be Null. Null is not the same as 0. Null + 4 would not equal 4, the result would be Null. So the COALESCE is necessary to make Null values 0.

Now you no longer need the outer select as this was being used to just sum the rows within the union. You can get rid of this all together. The row joined in to the outer query can just be joined into our main query. We no longer the the extra grouping here either. This gives us a query similar to the following :

SELECT f.codi_arxiu,
       f.nom_arxiu,
       f.codi_grup,
       f.codi_fons_refe,
       f.codi_fons,
       f.nom_fons,
       f.any_ini,
       f.any_fi,
       f.cronologia,
       COALESCE(child.num_uni_imatges, 0) + COALESCE(child2.num_uni_imatges, 0) AS num_uni_imatges,
       child.unitats_text,
       child2.unitats_notext
 FROM ianc_fons f
 LEFT JOIN
      ( SELECT codi_arxiu, codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.codi_unitat) AS unitats_text
      FROM ianc_unicat u
      WHERE u.anc_unicat_id IN
                    (SELECT u.anc_unicat_id
                       FROM ianc_unicat u
                      WHERE     1 = 1
                            AND u.codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.codi_fons = 1)
      GROUP BY codi_arxiu, codi_fons
      ) AS child ON f.codi_arxiu = child.codi_arxiu AND f.codi_fons = child.codi_fons

 LEFT JOIN
      ( SELECT u.nt1_codi_arxiu, u.codi_fons,
               SUM (CASE WHEN u.num_imatges > 0 THEN 1 ELSE 0 END) AS num_uni_imatges,
               COUNT (u.nt1_unitat) AS unitats_notext
        FROM ianci_unicats u
       WHERE     1 = 1
             AND u.nt1_id IN
                    (SELECT u.nt1_id
                       FROM ianci_unicats u
                        WHERE     1 = 1
                            AND u.nt1_codi_arxiu = '1'
                            AND u.codi_grup = 'ANC'
                            AND u.nt1_codi_fons = 1)
      ) AS child2 ON f.codi_arxiu = child2.nt1_codi_arxiu AND f.codi_fons = child2.nt1_codi_fons

  LEFT JOIN v_ianc_arxius_order v ON ( f.codi_arxiu=v.codi_arxiu )
  ORDER BY v.order_ANC,v.order_ANC2,f.nom_arxiu, f.nom_fons


Writing computer generated music to a .wav file in C See the next iteration.

I wrote this simple C program for writing computer generated music to a WAV file over three years ago. I refactored it a little, but it does not look good to me. Any suggestions?

#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>

/******************************
*  Magic file format strings. *
******************************/
const char fChunkID[]     = {'R', 'I', 'F', 'F'};
const char fFormat[]      = {'W', 'A', 'V', 'E'};
const char fSubchunk1ID[] = {'f', 'm', 't', ' '};
const char fSubchunk2ID[] = {'d', 'a', 't', 'a'};

/********************************
* WriteWavePCM() configuration: *
* - 2 channels,                 *
* - frequency 44100 Hz.         *
********************************/
const unsigned short N_CHANNELS = 2;
const unsigned int SAMPLE_RATE = 44100;
const unsigned short BITS_PER_BYTE = 8;

bool WriteWavePCM(short* sound, size_t pairAmount, char* fileName){
    const static unsigned int fSubchunk1Size = 16;
    const static unsigned short fAudioFormat = 1;
    const static unsigned short fBitsPerSample = 16;

    unsigned int fByteRate = SAMPLE_RATE * N_CHANNELS *
                             fBitsPerSample / BITS_PER_BYTE;

    unsigned short fBlockAlign = N_CHANNELS * fBitsPerSample / BITS_PER_BYTE;
    unsigned int fSubchunk2Size;
    unsigned int fChunkSize;

    FILE* fout;
    size_t ws;

    if (!sound || !fileName || !(fout = fopen( fileName, "w" ))) return false;

    fSubchunk2Size = pairAmount * N_CHANNELS * fBitsPerSample / BITS_PER_BYTE;
    fChunkSize = 36 + fSubchunk2Size;

    // Writing the RIFF header:
    fwrite(&fChunkID, 1, sizeof(fChunkID),      fout);
    fwrite(&fChunkSize,  sizeof(fChunkSize), 1, fout);
    fwrite(&fFormat, 1,  sizeof(fFormat),       fout);

    // "fmt" chunk:
    fwrite(&fSubchunk1ID, 1, sizeof(fSubchunk1ID),      fout);
    fwrite(&fSubchunk1Size,  sizeof(fSubchunk1Size), 1, fout);
    fwrite(&fAudioFormat,    sizeof(fAudioFormat),   1, fout);
    fwrite(&N_CHANNELS,      sizeof(N_CHANNELS),     1, fout);
    fwrite(&SAMPLE_RATE,     sizeof(SAMPLE_RATE),    1, fout);
    fwrite(&fByteRate,       sizeof(fByteRate),      1, fout);
    fwrite(&fBlockAlign,     sizeof(fBlockAlign),    1, fout);
    fwrite(&fBitsPerSample,  sizeof(fBitsPerSample), 1, fout);

    /* "data" chunk: */
    fwrite(&fSubchunk2ID, 1, sizeof(fSubchunk2ID),      fout);
    fwrite(&fSubchunk2Size,  sizeof(fSubchunk2Size), 1, fout);

    /* sound data: */
    ws = fwrite(sound, sizeof(short), pairAmount * N_CHANNELS, fout);
    fclose(fout);
    return true;
}

/************************************
* Around 23 seconds of pure techno! *
************************************/
const unsigned int N_SAMPLE_PAIRS = 1048576;

int main(int argc, char* argv[]){
    short* sound;
    int i;
    int j;
    bool status;
    char* file_name;

    sound = (short*) malloc( sizeof(short) * N_SAMPLE_PAIRS * N_CHANNELS );

    if (!sound)
    {
        puts("Could not allocate space for the sound data.");
        return (EXIT_FAILURE);
    }

    for (i = 0, j = 0; i < N_SAMPLE_PAIRS * N_CHANNELS; i += 2, j++ )
    {
        short datum1 = 450 * ((j >> 9 | j >> 7 | j >> 2) % 128);
        short datum2 = 450 * ((j >> 11 | j >> 8 | j >> 3) % 128);

        sound[i]     = datum1; // One channel.
        sound[i + 1] = datum2; // Another channel.
    }

    file_name = argc > 1 ? argv[1] : "Default.wav";
    status = WriteWavePCM(sound, N_SAMPLE_PAIRS, file_name);
    free(sound);

    if (status)
    {
        printf("Discotheque is ready in \"%s\"\n", file_name);
    }
    else
    {
        puts( "Something seems to have gone wrong." );
        return (EXIT_FAILURE);
    }

    return 0;
}

(What comes to playing the generated file, VLC and QuickTime Player are capable to play it; not sure about other media players.)

Endianness
Your program works on little endian machines only.  If you care for it to work on any machine, you would need to fix up all of the fwrite() calls to output specifically in little endian format.
Use struct for header
I would suggest creating a struct for the file header.  That way, you could fill in all the fields (in an endian correct manner) and then do one single fwrite().  From the code it looks like you can combine all the fields into a single header, but if it makes more sense you could have multiple headers (RIFF header, format header, data header, etc).


Separating presentation/BLL/DAL I'm new to ASP.Net and up until now my code has worked but has been a mish mash all over the place with no real structure. I'm trying to teach myself how to keep the presentation/BLL/DAL separate so I have built a simple page to try and work with.

Page has 3 text boxes: clock number, first name and last name. On inputting a clock number in the clock number text box, I want to go off to my database and fetch the first name and last name of the person that clock number belongs to.

My DAL consists of 3 classes as below:

Public Class EmployeeName
Private _FirstName As String
Private _Lastname As String

Public Property FirstName() As String
    Get
        Return _FirstName
    End Get
    Set(value As String)
        _FirstName = value
    End Set
End Property

Public Property LastName() As String
    Get
        Return _Lastname
    End Get
    Set(value As String)
        _Lastname = value
    End Set
End Property
End Class

Public Class EmployeeNameDBAccess

Public Function GetEmployeeName(clockNo As Integer) As EmployeeName

    Dim employeeName As EmployeeName = Nothing
    Dim params As SqlParameter() = New SqlParameter() {New SqlParameter("@clockNo", clockNo)}

    Using tbl As DataTable = SMARTDBHelper.ExecuteParameterisedSelectCommand("getEmployeeName", CommandType.StoredProcedure, params)
        If tbl.Rows.Count = 1 Then
            Dim row As DataRow = tbl.Rows(0)
            employeeName = New EmployeeName
            employeeName.FirstName = row("INITIALS").ToString()
            employeeName.LastName = row("SURNAME").ToString()
        End If
    End Using
    Return employeeName
End Function
End Class

Public Class SMARTDBHelper
Const constr As String = "connection string goes here"

Friend Shared Function ExecuteParameterisedSelectCommand(cmdName As String, cmdType As CommandType, param As SqlParameter()) As DataTable

    Dim tbl As New DataTable()

    Using con As New SqlConnection(constr)
        Using cmd As SqlCommand = con.CreateCommand()
            cmd.CommandType = cmdType
            cmd.CommandText = cmdName
            cmd.Parameters.AddRange(param)

            Try
                If con.State <> ConnectionState.Open Then
                    con.Open()
                End If

                Using da As New SqlDataAdapter(cmd)
                    da.Fill(tbl)
                End Using
            Catch ex As Exception
                Throw
            End Try
        End Using
    End Using

    Return tbl

End Function

End Class

My BLL consists of the following class

Public Class EmployeeNameHandler

Private employee As EmployeeNameDBAccess

Public Sub New()
    employee = New EmployeeNameDBAccess()
End Sub

Public Function GetFirstname(clockNo As Integer) As String

    Dim emp = employee.GetEmployeeName(clockNo)
    Return emp.FirstName

End Function

Public Function GetLastname(clockNo As Integer) As String

    Dim emp = employee.GetEmployeeName(clockNo)
    Return emp.LastName

End Function
End Class

Then on my page I have the following code to populate the text boxes

Protected Sub txtCLock_TextChanged(sender As Object, e As EventArgs) Handles txtCLock.TextChanged

    Dim emp As New EmployeeNameHandler
    Dim cn As Integer = CInt(txtCLock.Text)
    Dim firstname = emp.GetFirstname(cn)
    Dim lastname = emp.GetLastname(cn)

    txtFirstName.Text = firstname
    txtLastName.Text = lastname

End Sub

I appreciate this is basic however I would be interested to gather opinion of if I am going down the correct route here? Could this be done in a better way? Also, there is next to no error handling here, this is a topic I'm very weak on so would appreciate any advice etc. Thanks

Just a few suggestions:

Probably advisable to use Int.TryParse instead of CInt(txtCLock.Text) for parsing user input so you don't get an invalid type exception.

On general error handling you might at least put a try/catch around the logic in txtCLock_TextChanged to catch any errors at a lower layer and either log them or present to the user in a nice way if applicable.

If you're not actually referencing the first/last name backing fields you can use automatic properties so you don't have to write a get/set for every property.


R - Outlier Detection Algorithm I am trying to implement an algorithm for detecting outliers in R and I am pretty new to the language. The outlier algorithm is described in this paper in detail on page 10-11, but to summarize it works like this:

  Algorithm – Outlier Detection using K-Nearest Neighbor Data Distributions

  Find the set S(K) of K nearest neighbors to the test data point O.
  Calculate the K distances between O and the members of S(K). These distances define fK(d,O).
  Calculate the K(K-1)/2 distances among the points within S(K). These distances define fK(d,K).
  Compute the cumulative distribution functions CK(d,O) and CK(d,K), respectively, for fK(d,O)
  and fK(d,K).
  Perform the K-S Test on CK(d,O) and CK(d,K). Estimate the p-value of the test.
  Calculate the Outlier Index = 1-p.
  If Outlier Index > 0.95, then mark O as an “Outlier”. The Null Hypothesis is rejected.
  If 0.90 < Outlier Index < 0.95, then mark O as a “Potential Outlier”.
  If p > 0.10, then the Null Hypothesis is accepted: the two distance distributions are drawn from
  the same population. Data point O is not marked as an outlier.

I implemented this in the following code but I am unsure if this code is appropriate for R in terms of structure and style (compared with Python). Also, I don't know if there is a more efficient way than the simple step by step way I have done. I added code comments to delineate each step of the algorithm.

outlier_test <- function(data , neighbors){
  #Find the set S(K) of K nearest neighbors to the test data point O.
  sk <- nn2(data, k=neighbors)$nn.idx #matrix of indices of neighbors

  #Calculate the K distances between O and the members of S(K). These distances define fK(d,O).
  outlier_distances <- nn2(data, k=neighbors)$nn.dist #distances between value and its neighbors

  #Calculate the K(K-1)/2 distances among the points within S(K). These distances define fK(d,K).
  plyr::adply(sk, .margins = 1, function(row) { data[row, ] })

  df1<- plyr::alply(sk, .margins=1, function(row){data[row,]})
  df1 = plyr::ldply(df1, rbind)
  neighbor_distances <- plyr::alply(df1, 1, function(row) c(dist(unlist(row))))

  neighbor_distances <- df1 %>%
    dplyr::group_by(X1) %>%
    do({
      row_data <- .
      my_dist <- dist(row_data[ ,c(colnames(data)[1],colnames(data)[2])])
      as.data.frame(t(as.vector(my_dist)))
      }) %>%
    dplyr::ungroup() %>%
    dplyr::select(-X1) %>%
    as.matrix()

  #daply(test, .(X1), function(x) as.vector(dist(x))) This  is an alternative way to do the above piping

  #Compute the cumulative distribution functions CK(d,O) and CK(d,K), respectively, for fK(d,O)
  #and fK(d,K).
  # Since ks.test function in R can take vectors of values, we don't need to explicitly find the ECDF of our samples

  #Perform the K-S Test on CK(d,O) and CK(d,K). Estimate the p-value of the test.
  all_pvalues = sapply(1:nrow(outlier_distances), function(i) ks.test(as.vector(outlier_distances[i,]), as.vector(neighbor_distances[i,]))$p.value)

  #Calculate the Outlier Index = 1-p.
  outlier_index = 1-all_pvalues
  #If Outlier Index > 0.95, then mark O as an Outlier. The Null Hypothesis is rejected.
  #If 0.90 < Outlier Index < 0.95, then mark O as a Potential Outlier.
  #If p > 0.10, then the Null Hypothesis is accepted: the two distance distributions are drawn from
  #the same population. Data point O is not marked as an outlier
  return(outlier_index)
}

In my opinion there are only few aspects:

Consistent use of assignment operator (either = or <-, mind that in special cases they aren't equivalent)
I like that you use package-references (::), you forgot about RANN:: though
You could use the purrr-package to replace sapply-type commands, which gives you more control over the output
When you refer to columns in a fixed position way such as colnames(data)[1] you should make sure that the data structure you give to the function is as expected