Swift FloatingPoint rounded to places I'm playing a bit with extensions for default protocols in Swift 3.0.

I would like to implement rounded(toPlaces places: Int) -> Self for Double, Float.

Both of these structs refer to FloatingType protocol.
I wrote small extension for FloatingType protocol:

extension FloatingPoint {

    typealias Exponent = Int // is this okay? how can I write where `Exponent: Int` ?

    public func rounded(toPlaces places: Int) -> Self {
        guard places >= 0 else { return self }
        let divisor = Self(Int(pow(10.0, Double(places))))
        //let divisor = Self(sign: .plus, exponent: places, significand: Self(Int(pow(5, Double(places)))))
        return (self * divisor).rounded() / divisor
    }
}

However I can't find the most easiest and fastest method. Self(Int(pow(5, Double(places)))) so much conversions... Ugh...

The problem is that there is no pow for floating point type, so I need to do lots of conversions.

At 1m calls it loses about 0.04s for the Double extension, which doesn't do Self(Int(...)). Is it okay for such method or there is another approach?

First of all one should be aware that most decimal fractions cannot
be represented exactly by a binary floating point number. For example,

let y = Double.pi.rounded(toPlaces: 3)

does not assign the number 3.142 to y but the closest number
which is representable in the IEEE 754 format for double precision
floating point numbers, which happens to be

3.141999999999999904076730672386474907398223876953125

If the purpose of rounding the number is to present a result to the
user up to a certain precision, then it is better to use a number
formatter instead, for example

print(String(format: "%.3f", Double.pi)) // 3.142

or NumberFormatter which is very versatile and can do the conversion
according to the user's locale settings.

In other cases (like working with monetary values) it may be more
appropriate to work with integers only (or DecimalNumber) to avoid all rounding issues.

Having said that, here are some remarks and possible improvements
of your method.

I have named different implementations rounded1, rounded2, ... so that all can be tested in the same
program.

I am not sure what the effect of

typealias Exponent = Int

in the protocol extension is, but the proper way to restrict an extension
is a where clause:

extension FloatingPoint where Exponent == Int {

    public func rounded1(toPlaces places: Int) -> Self {
        guard places >= 0 else { return self }
        let divisor = Self(sign: .plus, exponent: places, significand: Self(Int(pow(5, Double(places)))))
        return (self * divisor).rounded() / divisor
    }
}

Of course you don't need a restriction for your other variant:

extension FloatingPoint {

    public func rounded2(toPlaces places: Int) -> Self {
        guard places >= 0 else { return self }
        let divisor = Self(Int(pow(10.0, Double(places))))
        return (self * divisor).rounded() / divisor
    }
}

The first one looks a bit obfuscated, and relies on the representation
of the numbers as a binary mantissa with an exponent.
I would use it only if it gives a clear performance advantage.

So let's benchmark it! I used this simple
test code which rounds 10,000,000 numbers in the range 0.0 .. 10.0
to 1 .. 8 decimal digits:

let start = Date()
var sum = 0.0
for i in 1 ... 10_000_000 {
    let x = Double(i)/1_000_000.0
    for p in 1 ... 8 {
        let y = x.rounded(toPlaces: p)
        sum += y
    }
}
let end = Date()
let time = end.timeIntervalSince(start)
print(sum, time)

The results are added to prevent the compiler from removing function
calls whose result is unused, and to verify that all methods give
exactly the same results.

On a 3,5 GHz iMac, with the code compiled in Release mode, I got
the following timings:

rounded1:   2.66 sec
rounded2:   2.52 sec

so the "obfuscated" method is actually a tiny bit slower.

But we can improve the performance. As you said, there is a lot of type conversions in

let divisor = Self(Int(pow(10.0, Double(places))))

This can be improved a bit if we restrict the extension method to
the BinaryFloatingPoint protocol instead. This is not a severe
restriction, because all current floating point types (Float, Double,
CGFloat) conform to BinaryFloatingPoint. The advantage is that
a value can be initialized from a Double:

extension BinaryFloatingPoint {

    public func rounded3(toPlaces places: Int) -> Self {
        guard places >= 0 else { return self }
        let divisor = Self(pow(10.0, Double(places)))
        return (self * divisor).rounded() / divisor
    }
}

That makes it a bit faster:

rounded1:   2.34 sec

The bottleneck seems to be the pow function. Since places
will usually be a small integer, it might be worth to use
iterated multiplication instead:

extension BinaryFloatingPoint {
    public func rounded4(toPlaces places: Int) -> Self {
        guard places >= 0 else { return self }
        let divisor = Self((0..<places).reduce(1.0) { (accum, _) in 10.0 * accum })
        return (self * divisor).rounded() / divisor
    }
}

Here Double values are multiplied and the result converted to
the type of Self. This turned out to be faster than multiplying
integers or instances of Self.

This is more than 4x faster compared to rounded3 and needs only
a single type conversion:

rounded4:   0.49 sec


Modify array of arrays to create custom array of objects I have an array of arrays and I want to create an array of objects from it. There are custom properties to which I want to associate the sub-array values. I have the following array:

var testArray = [[1,5,10], [10,20,7], [11,10,25]];

I am modifying it to create the following

[{
    coordinates: [1, 5],
    reach: 10,
    power: 0
},
{
    coordinates: [10, 20],
    reach: 7,
    power: 0
},
{
    coordinates: [11, 10],
    reach: 25,
    power: 0
}]

For this I have written the following code.

function modifyArray(array) {
    return array.map(array => {
        return Object.assign({}, { coordinates: array.slice(0, 2), reach: array[2], power: 0 });
    });
}

I get the desired result. My question is if this is a good way to modify this type of data performance wise, since the function has two return statements. How can I improve on this code?

Overview

You're overwriting the name array in defining the mapping param.
You don't need to assign to a new object. This is done for you.

Rewrite

function modifyArray(arrays) {
    return arrays.map(array => {
        return { coordinates: array.slice(0, 2), reach: array[2], power: 0 };
    });
}


Render elements from an array of objects The goal is to list out some data, but the methods vary on options, and whether the user is on a touch device.

This is my current function declaration. Very non-DRY. Values of isTouch and headlinesOnly don't change during loop, so I don't want to make if tests inside the loop.

Edit: This is primarily meant for a memory-sensitive environment which, when not handled properly, reboots to safe mode, not a regular browser. (But I'm making it for both.)

var makeElement;

if (isTouch) {
    if (headlinesOnly) {
        makeElement = function(item, top, left) {
            var a = document.createElement("a");
            a.setAttribute('ontouchstart', 'itemTouchStart()');
            a.setAttribute('ontouchmove', 'itemTouchMove()');
            a.setAttribute('ontouchend', 'itemTouchEnd(\'' + item.link + '\')');
            a.style.top = top + 'px';
            a.style.left = left + 'px';
            a.innerHTML = '
' + item.title + '
';
            return a;
        };
    }
    else {
        makeElement = function(item, top, left) {
            var a = document.createElement("a");
            a.setAttribute('ontouchstart', 'itemTouchStart()');
            a.setAttribute('ontouchmove', 'itemTouchMove()');
            a.setAttribute('ontouchend', 'itemTouchEnd(\'' + item.link + '\')');
            a.style.top = top + 'px';
            a.style.left = left + 'px';
            a.innerHTML = '
' + item.title + '
'
                        + '
' + item.content + '
';
            return a;
        };
    }

    var dragging;

    window['itemTouchStart'] = function() {
        dragging = false;
    };

    window['itemTouchMove'] = function() {
        dragging = true;
    };

    window['itemTouchEnd'] = function(url) {
        if (dragging) return;

        if (cycript) {
            tryOpenUrl(url);
        }
        else {
            window.open(url, "_blank");
        }
    };
}
else {
    if (headlinesOnly) {
        makeElement = function(item, top, left) {
            var a = document.createElement("a");
            a.setAttribute('onmouseup', 'itemMouseUp(\'' + item.link + '\')');
            a.style.top = top + 'px';
            a.style.left = left + 'px';
            a.innerHTML = '
' + item.title + '
';
            return a;
        };
    }
    else {
        makeElement = function(item, top, left) {
            var a = document.createElement("a");
            a.setAttribute('onmouseup', 'itemMouseUp(\'' + item.link + '\')');
            a.style.top = top + 'px';
            a.style.left = left + 'px';
            a.innerHTML = '
' + item.title + '
'
                        + '
' + item.content + '
';
            return a;
        };
    }

    window['itemMouseUp'] = function(url) {
        window.open(url, "_blank");
    };
}

var item = myCollection.shift();

container.appendChild(makeElement(item));

var columnWidth = container.childNodes[0].offsetWidth;
var containerWidth = container.offsetWidth;
var numberOfColumns = containerWidth / columnWidth;
var columns = [];

for (var i = 0; i < numberOfColumns; i++) {
    this.columns.push(0);
}

while (item = myCollection.shift()) {
    var sh = getShortestColumn();
    var itemElement = makeElement(item, sh.height, sh.index * columnWidth);
    container.appendChild(itemElement);
    columns[sh.index] += itemElement.offsetHeight;
}

Two things right off the bat:

You're assuming that item.title and item.content are "safe" to insert. I.e. that they don't contain HTML, something that could be mistaken for HTML, or that if they contain HTML, it's meant to be there.

For instance, if title is "How to use the  tag", then the "" will be interpreted as HTML by the browser, and you won't get the right result.
Putting block elements (h1 and div) inside an inline element (a) is invalid HTML. (Edit: I was wrong - it's allowed, if you're using HTML5. Still invalid in earlier version, though. Tip 'o the hat to @dev-null for the correction). Anyway, your a elements hardly need to be a elements; you're handling all the interaction in JavaScript. They could just as well be div elements, or article elements or something else entirely.

Moving on. Janos already touched on how to improve makeElement, but I'd take it a step further.

I don't like the idea of defining a global function in an if/else to begin with, when said if/else could just as well be inside the function.

Using Janos' extracted createAnchor, you could simply define makeElement as:

function makeElement(item, top, left, includeContent) {
    var innerHTML = '
' + item.title + '
';
    if(includeContent) {
        innerHTML += '
' + item.content + '
';
    }
    return createAnchor(item, top, left, innerHTML);
}

The includeContent argument can probably be left out, since headlinesOnly seems to be a global (which is not a good thing, by the way). Here, I've made the argument "positive", as in "yes, do include content", since that's the part you're deciding on. Headlines are always present, so headlinesOnly is a bit of a double negation in the sense that true actually means "Yes, don't include content".

Overall, however, makeElement is poor name. Everything on the page is an element, so "make element" doesn't explain anything about the function's purpose. I don't know what your page is displaying, but I'm sure there's some more descriptive name for those elements; forget that they're HTML elements and consider instead what they're representing.

And while I used Janos' createAnchor, I'm not a huge fan of it. Using onevent-style attributes is not good practice. It's been outmoded for many years now.

Instead you should use addEventListener to attach event handling functions to your elements.

This would also mitigate the other big no-no you're committing: window['something'] = function ....

First of all, why not just say window.something = ... instead of the bracket/subscript? Using brackets is only useful if you want to access a property using a dynamic string. Here, the strings are hardcoded so it makes no difference.

But much more importantly, why are you declaring so many global functions? It's plain bad practice to pollute the global namespace like that.

Again, using addEventListener would avoid all that.

I'm also wondering why the behavior is different for touch vs mouse interaction. If I drag on a touch screen, the touchend event is ignored. But if I drag with a mouse, the mouseup event is not ignored. So why not use the click event instead?

Of course, click does have a delay on many touch screen clients, since the device is waiting to see if there's a 2nd tap, i.e. if it's a double-tap that's meant to zoom or some similar "meta-interaction". Whereas on with a mouse and keyboard there's no inherent meaning to a double-click, hence there's no delay.

So maybe keep the touchstart/move/end handlers for touch screens, but just use click for mouse events.

Lastly, as idle speculation, do you need to absolutely position your elements? You could make "column" elements instead, arranged side-by-side with CSS, and simply add your item elements to those. The browser would figure out how to do the layout. But I don't know the particulars of your page.

Here's one way to factor the code. Not saying it's perfect (far from it), but there's no duplication:

function createItemElement(item, includeContent) { // still not a great name
  var element = document.createElement('div'), // not an
      heading = document.createElement('h1'),
      content;

  heading.innerHTML = item.title; // NOTE: I'm *assuming* the title is "safe" to insert as HTML
  element.appendChild(heading);

  if(includeContent) {
    content = document.createElement('div');
    content.innerHTML = item.content; // NOTE: See the other note
    element.appendChild(content);
  }

  addItemClickListener(item, element);

  return element;
}

function addItemClickListener(item, element) {
  var touchmove;

  // define what should happen on click/tap
  function openItemUrl(event) {
    isTouch && cycript ? tryOpenUrl(item.url) : window.open(url, '_blank');
  }

  if(isTouch) {
    touchmove = function (event) {
      // remove move/end listeners in case of touchmove
      element.removeEventListener('touchend', openItemUrl);
      element.removeEventListener('touchmove', touchmove);
    };

    element.addEventListener('touchstart', function () {
      // only add the move/end listeners when a touchstart occurs
      element.addEventListener('touchmove', touchmove);
      element.addEventListener('touchend', openItemUrl);
    });
  } else {
    element.addEventListener('click', openItemUrl);
  }
}

// ....

// basic usage
var itemElement = createItemElement(item, !headlinesOnly);
itemElement.style.top = ...;
itemElement.style.left = ...;
container.appendChild(itemElement);

Here, createItemElement doesn't care about positioning; it just creates an element, and then it's up to you to do something with it. Likewise, it knows nothing about touch vs mouse; that's all contained in addItemClickListener. And the event handling doesn't use an explicit drag boolean; it just adds and removes the event listeners as needed.

Either function knows as little as possible, while the things that always change (positioning) is simply handled directly without being passed via arguments.

Optimally, the isTouch and cycript (whatever that is) variables wouldn't need to be global either, as they're what's creating the most coupling right now; addItemClickListener depends on both being present, while createItemElement is much better off just relying only on its arguments (and, well, on document, but that's a pretty safe thing to rely on in a browser).

I've also been a bit pedantic in building the elements, explicitly creating them with document.createElement. I'm just not a fan of defining chunks of HTML as strings. Besides, with this, you can more easily control how the title and content is added to the elements.


Is there a proper way to query ExpandoObject? I am receiving the JSON from WebService and populating ExpandoObject from this one:

 var converter = new ExpandoObjectConverter();
 var jsonObject = JsonConvert.DeserializeObject<ExpandoObject>(json, converter);

The JSON object is quite complex and in order to find the value I am doing this spaghetti code:

KeyValuePair<string, object> CountPair = new KeyValuePair<string, object>();

var currentCountPair = jsonObject.FirstOrDefault(x => x.Key == "data").Value;

if (currentCountPair != null)
{
    currentCountPair = ((ExpandoObject)currentCountPair).FirstOrDefault(x => x.Key == "children");

    if (currentCountPair != null)
    {
        currentCountPair = ((IList<object>)((KeyValuePair<string, object>)currentCountPair).Value).FirstOrDefault();

        if (currentCountPair != null)
        {
            currentCountPair = ((ExpandoObject)currentCountPair).FirstOrDefault(x => x.Key == "data").Value;

            if (currentCountPair != null)
            {
                CountPair = ((ExpandoObject)currentCountPair).FirstOrDefault(x => x.Key == "score");
                result.Score = CountPair.Value == null ? null : CountPair.Value.ToString();
            }
        }
    }

}

What do you think?  Is there any better way of getting that value from ExpandoObject?
Or at least to reduce the number of if statements and code complexity...

First, I don't see any reason for using ExpandoObject in your code. The same result could be achieved with cleaner code using JObjects.

Second, what would help if you could express your query using “LINQ to null”. Basically, treat null as an empty collection and any other value as a collection containing just that value. (The technical term for this is the Maybe monad.)

With both of these changes, your code could look something like this:

var jsonObject = JsonConvert.DeserializeObject<JObject>(json);

string result =
    from o in jsonObject.ToNullWrapper()
    from data in o["data"]
    from children in data["children"]
    from child in children.FirstOrDefault()
    from data2 in child["data"]
    from score in data2["score"]
    select score.ToString();

Note that (ab)using LINQ like this is not idiomatic and probably very confusing. But I think it results in very clean code.

To make this work, you need the implementation of LINQ to null:

public struct NullWrapper<T>
    where T : class
{
    public T Value { get; private set; }

    public NullWrapper(T value)
        : this()
    {
        Value = value;
    }

    public static implicit operator T(NullWrapper<T> wrapper)
    {
        return wrapper.Value;
    }
}

public static class NullExtensions
{
    public static NullWrapper<T> ToNullWrapper<T>(this T value)
        where T : class
    {
        return new NullWrapper<T>(value);
    }

    public static NullWrapper<TResult>
        SelectMany<TSource, TIntermediary, TResult>(
        this NullWrapper<TSource> source,
        Func<TSource, TIntermediary> intermediarySelector,
        Func<TSource, TIntermediary, TResult> resultSelector)
        where TSource : class
        where TIntermediary : class
        where TResult : class
    {
        if (source.Value == null)
            return new NullWrapper<TResult>();

        var intermediary = intermediarySelector(source.Value);

        if (intermediary == null)
            return new NullWrapper<TResult>();

        return resultSelector(source.Value, intermediary).ToNullWrapper();
    }
}


Scraping Clinical Studies database (via ClinicalTrials.Gov) I have written the following script to scrape data from the U.S. National Library of Medicine website ClinicalTrials.Gov based on an NCTID.

def clinicalTrialsGov (nctid):
    data = BeautifulSoup(requests.get("https://clinicaltrials.gov/ct2/show/" + nctid + "?displayxml=true").text, "xml")
    subset = ['study_type', 'allocation', 'intervention_model', 'primary_purpose', 'masking', 'enrollment', 'official_title', 'condition', 'minimum_age', 'maximum_age', 'gender', 'healthy_volunteers', 'phase', 'primary_outcome', 'secondary_outcome', 'number_of_arms']
    tag_matches = data.find_all(subset)
    tag_dict = dict((str('ct' + tag_matches[i].name.capitalize()), tag_matches[i].text) for i in range(0, len(tag_matches)))
    tag_dict = multipleFields(data, ['intervention_name'], tag_dict)
    tag_dict = multipleFields(data, ['intervention_type'], tag_dict)
    tag_dict = multipleFields(data, ['arm_group_type'], tag_dict)
    tag_dict['ctID'] = nctid
    #for key in tag_dict:
        #print(key + ': ' + tag_dict[key])
    return removeEmptyKeys(tag_dict)

def multipleFields (data, subset, tagDict):
    fields = data.find_all(subset)
    field = []
    try:
        for each in fields:
            field.append(each.text)
        tagDict[str('ct' + subset[0].capitalize())] = ", ".join(field)
        return tagDict
    except:
        return tagDict

def removeEmptyKeys (dict1):
    newDict = {}
    for key in dict1:
        if str(dict1[key]) is not '':
            newDict[key] = dict1[key]
    return newDict

What can I do to make this process more efficent?

I hope I'm not too late.

There are a few things you could do :

Solution 1

import requests
from bs4 import BeautifulSoup
import pprint

def clinicalTrialsGov (nctid):
    data = BeautifulSoup(requests.get("https://clinicaltrials.gov/ct2/show/" + nctid + "?displayxml=true").text, "xml")
    subset = ['study_type', 'allocation', 'intervention_model', 'primary_purpose', 'masking', 'enrollment', 'official_title', 'condition', 'minimum_age', 'maximum_age', 'gender', 'healthy_volunteers', 'phase', 'primary_outcome', 'secondary_outcome', 'number_of_arms']
    tag_matches = data.find_all(subset)
    tag_dict = {'ct' + current_tag.name.capitalize(): current_tag.text for current_tag in tag_matches}
    tag_dict = multipleFields(data, 'intervention_name', tag_dict)
    tag_dict = multipleFields(data, 'intervention_type', tag_dict)
    tag_dict = multipleFields(data, 'arm_group_type', tag_dict)
    tag_dict['ctID'] = nctid
    return removeEmptyKeys(tag_dict)

def multipleFields (data, subset, tagDict):
    fields = data.find_all(subset)
    field = [each.text for each in fields]
    tagDict['ct' + subset.capitalize()] = ", ".join(field)
    return tagDict

def removeEmptyKeys (dict1):
    newDict = {k:v for (k, v) in dict1.items() if v}
    return newDict

pprint.pprint(clinicalTrialsGov("NCT01220960"))

I have used a dictionary comprehension to define tag_dict and newDict. This is similar to a list comprehension or a generator expression but specialized for dictionaries
I have removed the try … except from multipleFields because I don't see in which case an exception will be raised (especially since you didn't specify which one you were trying to catch)
I have presumed that subset in multipleFields() is a string and not a list of strings since you were looking only for one tag
I have used a list comprehension to define field in multipleFields()
I have used the pprint module to see better the answer.


Find the nearest (small) number x where x can be represented by sum of cubes I recently came across a problem where a number \$x\$ is given and I have to find \$y\$:

\$y < x > 1\$
\$y\$ can be expressed as \$a^3 + b^3\$ (more than one combination)

Example:

If 4105 is \$x\$ then 4104 is the answer, where 4104 can be expressed as \$16^3 + 2^3\$ and also \$15^3 + 9^3\$.

I am able to write a program in Objective-C, but I am sure that my method is not optimized and is not best.

I am posting this to find out

the best algorithms to solve this problem
Flaws in my way of solving

#import <Foundation/Foundation.h>

BOOL isPerfectCube(int cubeValue)
{
    float CalculatedCubeRoot = powf(cubeValue, 1.0/3.0);
    CalculatedCubeRoot = floorf(CalculatedCubeRoot * 100) / 100;
    return ((CalculatedCubeRoot - (int) CalculatedCubeRoot) == 0);
}

int cubeRoot(int value)
{
    return powf((float)value, 1.0/3.0);
}

int nearbyNumber(int numberUnderInvestigation)
{
    if(numberUnderInvestigation < 2)
    {
        return -1;
    }
    else
    {
        int firstCube = 1;
        int combination = 0;

        for(int i=2;firstCube<numberUnderInvestigation;i++)
        {
            firstCube = i*i*i;
            int nextCubeNumber = numberUnderInvestigation - firstCube;
            if(isPerfectCube(nextCubeNumber))
            {
                combination++;
            }
        }

        if(combination > 3)
        {
            return numberUnderInvestigation;
        }
        else
        {
            return nearbyNumber(numberUnderInvestigation - 1);
        }
    }
}

int main(int argc, const char * argv[])
{
    @autoreleasepool
    {
        int inputNumber = 4150;

        NSLog(@"%d",nearbyNumber(inputNumber));
    }

}

personally I would calculate that value like e.g. this:

/*
 * check the updated algorithm, please
 *
- (Float64)neareastSmallerSumOfCubes:(NSInteger)originalNumber {
    if (originalNumber > 1) {
        Float64 _firstAttempt = pow((Float64)originalNumber, 1.0/3.0);
        if (_firstAttempt == floor(_firstAttempt)) originalNumber--;
        NSInteger _a = pow((Float64)originalNumber, 1.0/3.0);
        NSInteger _b = pow((Float64)originalNumber - pow(_a, 3.0), 1.0/3.0);
        return pow(_a, 3.0) + pow(_b, 3.0);
    } else {
        return MAXFLOAT;
    }
}
*/

update

thank for @Szu, he has highlighted an issue with some large numbers;
thank for @Rob, he has highlighted an issue when input is already a pure sum of cubed;

I've updated the algorithm a bit according to the feedbacks, here is the final solution:

- (Float64) neareastSmallerSumOfCubes:(NSInteger)originalNumber {
    if (originalNumber > 1) {
        Float64 _firstAttempt = pow((Float64)originalNumber, 1.0/3.0);
        if (_firstAttempt == floor(_firstAttempt)) originalNumber--;
        NSInteger _a = pow((Float64)originalNumber, 1.0/3.0);
        NSInteger _b = pow((Float64)originalNumber - pow(_a, 3.0), 1.0/3.0);
        if (_b > 1) _b--;
        NSInteger _s = round(pow((Float64)originalNumber / 2.0, 1.0/3.0));
        Float64 _neareast = 0.0;
        do {
            Float64 _possibleNearest = pow(_a, 3.0) + pow(_b, 3.0);
            if (_possibleNearest > originalNumber) _a--;
            else if (_possibleNearest > _neareast && _possibleNearest != originalNumber) _neareast = _possibleNearest, _b++;
            else _b++;
        } while (_b <= _s);
        return _neareast;
    } else {
        return MAXFLOAT;
    }
}

random test values:

99998 returns 99937 (= 333 + 403) (runtime O(36))
4105 returns 4104 (= 163 + 23) (runtime O(17))
1023 returns 1008 (= 103 + 23) (runtime O(11))
999 returns 945 (= 93 + 63) (runtime O(6))
216 returns 189 (= 53 + 43) (runtime O(4))
3 returns 2 (= 13 + 13) (runtime O(1))

explanation

let us get the two essential conditions from the OP

y < x > 1;
y can be expressed with a3 + b3;
where x, y ∈ ℕ, a, b ∈ ℕ+ and a > 0, b > 0 (that last condition obviously has to be satisfied);

#1

we know 13 + 13 < x < (x+1)1/3 + (x+1)1/3, therefore the actual smaller value also must be in this interval.

if we can build a simple (x+1)1/3 by (x+1)1/3 matrix which holds every possible combination of sums of cubes, that matrix must contain the value we are chasing.

  COMMENT: after certain number of iterations (maximum number of iterations: ((x+1)1/3)2) the requested nearest smaller value can be extracted from that massive interval via e.g. brutal-force algorithms, but we are looking for a much efficient solution than that.

#2

we know the already the addition is commutative operand, so a+b=b+a, so we can drop the part under the diagonal of the matrix, so we can stop the iteration later when we reach the column s = (x/2)1/3. the s is rounded down to the nearest natural, s ∈ ℕ.

#3

the possible values are at certain locations in the half matrix, the possible elements are on an exponential curve, that helps to establish the actual logic of the iterations.

we need to find where this curve enters the matrix, and we walk through the values of the matrix what the curve intersects.

the enter row index is the whole part of a = x1/3 and the column index is the whole part of the b = (x - a3)1/3.

we know:

a > s > b is always true, therefore:
x > a3 + b3; that value also can be the possible nearest smaller, which we called y.
x < (a+1)3 + (b+1)3;
we don't know the relation between x and a3 + (b+1)3; that is what we will find out:

let b = b + 1 and check what we have got:

if the x > a3 + b3 > y, that is a another possible nearest smaller; if not, we ignore it;

if the x = a3 + b3, that is the same as input, we ignore it, according to the original y < x condition;

if the x < a3 + b3, that value is greater than the original number, so we ignore it; because the a > s > b and x ≧ 2s3 statements are always true, therefore x ≧ (a-1)3 + b3 will be true as well, so we can let a = a - 1 and start the next iteration with the new a, b values.

the iteration stops when the b reaches the intersection of the matrix diagonal and the exponential curve, that value was calculated above as s; so if we reach the intersection point, we can be sure that we have already checked all possible nearest smaller values which the matrix holds; and during making the iterations we always kept the largest one as y, thus when we stop the algorithm, the y will hold the only nearest smaller value.


htmlspecialchars() equivalent in C I'm trying to write an equivalent of PHP's htmlspecialchars() in C. What is my implementation missing or doing wrong?

char www_specialchars_html(char *dest, char *dest_last, char *src) {
while (*src != 0) {
    switch(*src) {
        //If a special character is found, replace with its HTML entity.
        case '&':
            if ((dest + 5) > dest_last) {
                return 1;
            }
            *dest = '&';
            dest++;
            *dest = 'a';
            dest++;
            *dest = 'm';
            dest++;
            *dest = 'p';
            dest++;
            *dest = ';';
            dest++;
            src++;
            break;
        case '"':
            if ((dest + 6) > dest_last) {
                return 1;
            }
            *dest = '&';
            dest++;
            *dest = 'q';
            dest++;
            *dest = 'u';
            dest++;
            *dest = 'o';
            dest++;
            *dest = 't';
            dest++;
            *dest = ';';
            dest++;
            src++;
            break;
        case '\'':
            if ((dest + 6) > dest_last) {
                return 1;
            }
            *dest = '&';
            dest++;
            *dest = '#';
            dest++;
            *dest = '0';
            dest++;
            *dest = '3';
            dest++;
            *dest = '9';
            dest++;
            *dest = ';';
            dest++;
            src++;
            break;
        case '<':
            if ((dest + 4) > dest_last) {
                return 1;
            }
            *dest = '&';
            dest++;
            *dest = 'l';
            dest++;
            *dest = 't';
            dest++;
            *dest = ';';
            dest++;
            src++;
            break;
        case '>':
            if ((dest + 4) > dest_last) {
                return 1;
            }
            *dest = '&';
            dest++;
            *dest = 'g';
            dest++;
            *dest = 't';
            dest++;
            *dest = ';';
            dest++;
            src++;
            break;
        case '\\': //Not required in HTML.
            if ((dest) > dest_last) {
                return 1;
            }
            src++;
            break;
        default:
            //Default for characters that don't need to be replaced.
            if ((dest + 1) > dest_last) {
                return 1;
            }
            *dest = *src;
            src++;
            dest++;
            break;
    }
}
*dest = 0;
return 0;
}

The function takes pointers to the destination, the last element of the destination and the source. It returns 1 if it can't write to the destination, and returns 0 when it has successfully copied to the destination. I'm still a relatively new C programmer.

You shouldn't pass a char* as buffer end but a size_t size corresponding to the size of the allocated buffer. So your available length will be size - 2 ("zero-indexed" and "trailing '\0'").
You return either 1 or 0, why don't using bool as return type? (Or maybe a size_t corresponding to the final string length, but, that's a design decision).
Variables where value doesn't change in your function (as locales or parameters) should be marked const. That's not faster but it show your intends.
You can maybe use strcpy, strncpy or  memcpy to ... copy the replacements string into destination (and replace multiple incrementation by a single add-assign).
For non-special chars sequences, you can maybe get them in chunk by chunk (between two special chars) with strcspn or strpbrk. (and after, copying them with same functions than former)

Edit

This is a raw try using theses functions.

I think it is safe, with no overflow, but maybe not the optimal implement.
Don't be scared about the size, there is a lot of comments :)

I outed two functions from source (strlenand replace_chrstr), 'cause I think they are reusable somewhere.

#include <string.h>

// Get the position of the c char in the str string.
// -> str: a null-terminated string where to lookup
// -> needle: char to find
// Return: Position of the char `c` or 1 past the end (strlen + 1)
size_t strpos(const char* const str, const char c)
{
    char* p = strchr(str, c);
    return p ? (p - str) : strlen(str) + 1;
}

// Scan `in_str` and replace all char occurrences from `search`
// by corresponding string from `replace`, then place the result in `out_str`.
// -> search: A null-terminated string with all char to fiend
// -> replace: An array of null-terminated strings with index based replacement from `search`
// -> out_str: a buffer to put the result, at least as big than max_size
// -> in_str: a null-terminated string where to process replacements
// Return: length of resulting string, or length+1 in case of overflow
size_t replace_chrstr(const char* const search, const char* const* const replace,
                      char *const out_str, const char* const in_str, const size_t max_size)
{
    size_t in_offset = 0;
    size_t out_offset = 0;
    size_t index;
    size_t length;
    const size_t index_max = strlen(search);
    const size_t length_max = strlen(in_str);

    while (max_size > out_offset) {
        length = strcspn(in_str + in_offset, search);
        // buffer overflow detected, here we truncate the input string
        // we add `\0` and return max_size (equivalent to strlen + 1)
        // but maybe you want another handling
        if (max_size <= out_offset + length) {
            strncpy(out_str + out_offset, in_str + in_offset, max_size-out_offset-1);
            out_str[max_size-1] = '\0';
            return max_size;
        }
        strncpy(out_str + out_offset, in_str + in_offset, length);
        in_offset += length + 1;
        out_offset += length;
        // normal end of input string
        // add a `\0`, then return out_offset (equivalent to strlen)
        if (in_str[in_offset-1] == '\0') {
            out_str[out_offset] = '\0';
            return out_offset;
        }
        index = strpos(search, in_str[in_offset-1]);
        length = strlen(replace[index]);
        // buffer overflow detected, here we truncate the replacement string
        // we add `\0` and return max_size (equivalent to strlen + 1)
        // but maybe you want another handling
        if (max_size <= out_offset + length) {
            strncpy(out_str + out_offset, replace[index], max_size-out_offset-1);
            out_str[max_size-1] = '\0';
            return max_size;
        }
        strcpy(out_str + out_offset, replace[index]);
        out_offset += length;
    }
    // we never reach here, normally
    return 0;
}

size_t www_specialchars_html(char* const dest, const char* const source, size_t max_size)
{
    const char search[] = "&\"'<>";
    const char * replace[] = {"&amp;","&quot;","&#039;","&lt;","&gt;"};
    return replace_chrstr(search, replace, dest, source, max_size);
}


Hamburger menu with Javascript - clearing state I wanted a menu that is always a hamburger, regardless of screen size. Plus it has to be activated by click, not hover.
Any comments / review will be appreciated.
Pls let me know if this is not worded properly / off topic etc.
Please see Codepen
<!DOCTYPE html>

    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>Sample website</title>
    <link rel="stylesheet" type="text/css" href="css/test.css">

 &#9776;

Menu 1

Menu 1-1

Menu 1-2

Menu 1-3

Menu 1-4

Menu 2

Menu 2-1

Menu 2-2

Menu 2-3

Menu 2-4

Menu 3

Menu 3-1

Menu 3-2

Menu 3-3

Menu 3-4

#menuBar > ul  {
    display: none;
}

ul {
    list-style-type: none;
    margin: 0;
    padding: 0;
    width: 10em;

}

li {
    list-style-type:none;
    margin: 1px;
    padding: 1px;
}

li:first-child {
    background: grey;
    color: #fff;
}

li:not(:first-child) {
    display: none;
}

.menuHamburger {
    font-size: 2em;
}

function hideAllButFirstChild(parentId) {

    var children = document.getElementById(parentId).children;
    for (var i=0; i < children.length; i++) {

       if (children[i].style.display=='' || children[i].style.display=='none') {

       children[i].style.display="block";

       // but set its children to none?

       } else {

            // block all except first
            if (i != 0) { children[i].style.display="none"; }

        }

     }

}

HTML

The formating could be a bit cleaner ( on its own line, superfluous spaces around =, content of
 not indented).
on... attributes shouldn't be used. Event handlers should be assigned in the script.
I don't believe &#9776; ("Trigram For Heaven") is a suitable character to use for a  menu icon.
The submenus aren't structured properly in my opinion:

The clickable header entries shouldn't be list items themselves, but separate elements before the ul.
Also they should be an interactive elements such as <button> or .
If you would also wrap all the submenus in a separate item with the hamburger button before it, then you could simplify the JavaScript so that it simply toggles that single element, which spares you the loop and the exception of the first element.
Alternatively you could even consider using the  element, which has the toggling built in, requiring no JavaScript at all.

JavaScript

Instead of passing the ID of the parent element into the function you could get the clicked element (and thus its parent) from the event object. Example:

// Assign event handler in JavaScript instead of using the HTML attribute
document.querySelector(".menuHamburger").addEventListener("click", hideAllButFirstChild);

function hideAllButFirstChild(event) {
   // TODO Add null checks.
   const children = event.target.parent.children;

   // ...
}

And as seen above use const or let instead of var for variables.
Start the loop at i = 1 to skip the first element instead of checking inside the loop.
Instead of manipulating style in JavaScript set/remove a class (or, better, in this case the hidden attribute)


Determining if a file or URL is a git repo or archive I'm getting these three offenses:

libraries/helpers.rb:4:1: C: Method has too many lines. [11/10]
def get_files(url, file, destination)
^^^
libraries/helpers.rb:18:1: C: Assignment Branch Condition size for unpack_archive is too high. [16.52/15]
def unpack_archive(url, file, destination)
^^^
libraries/helpers.rb:18:1: C: Method has too many lines. [16/10]
def unpack_archive(url, file, destination)

These are all length based offenses. Is there any way to refactor to meet the style guidelines?

# Takes a file and or a url decides if it is a git repo or an archive.
# Downloads or clones it. to the destination.
# Do not enter url with trailing slash.
def get_files(url, file, destination)
  case File.extname(file)
  when '.tar.gz', '.zip' then unpack_archive(url, file, destination)
  when '.git'
    git 'destination' do
      repository "#{url}/#{file}"
      reference 'master'
      user node['apache']['user']
      action :sync
    end
  else fail ArgumentError, "dunno how to handle #{file}"
  end
end

def unpack_archive(url, file, destination)
  remote_file "#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}" do
    owner 'root'
    group 'root'
    mode '0644'
    source "#{url}/#{file}"
    not_if File.readable?(file)
  end
  extract = case File.extname(file)
            when '.tar.gz' then "tar-xC #{destination} -f #{file};"
            when 'zip' then "unzip -d #{destination} -qo #{file};"
            end
  bash "Extract #{file}" do
    cwd ::File.dirname("#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}")
    code   extract
    not_if { ::File.readable(file) }
  end
end

Well the easiest option is to extract parts into separate methods, no?

In unpack_archive the handling of File.extname seems buggy for zip
files?  At least it's different to the same case statement in
get_files.

File.readable? is used, but the other case in bash is missing the
question mark.

I don't know enough about Ruby and Chef, but I also saw that the
not_if arguments are specified in two different ways; maybe it's
possible to use the same way for both directives?

With two additional functions it looks a bit less redundant and more to
the point, however I guess there's still space for further improvements:

# Takes a file and or a url decides if it is a git repo or an archive.
# Downloads or clones it. to the destination.
# Do not enter url with trailing slash.
def get_files(url, file, destination)
  case File.extname(file)
  when '.tar.gz', '.zip' then unpack_archive(url, file, destination)
  when '.git' then git_clone(url, file)
  else fail ArgumentError, "dunno how to handle #{file}"
  end
end

def git_clone(url, file)
  git 'destination' do
    repository "#{url}/#{file}"
    reference 'master'
    user node['apache']['user']
    action :sync
  end
end

def get_extract_command(file)
  case File.extname(file)
  when '.tar.gz' then "tar -xC #{destination} -f #{file};"
  when '.zip' then "unzip -d #{destination} -qo #{file};"
  else fail ArgumentError, "dunno how to extract #{file}"
  end
end

def unpack_archive(url, file, destination)
  directory = "#{Chef::Config['file_cache_path'] || '/tmp'}/#{file}"
  remote_file directory do
    owner 'root'
    group 'root'
    mode '0644'
    source "#{url}/#{file}"
    not_if File.readable?(file)
  end
  extract = get_extract_command(file)
  bash "Extract #{file}" do
    cwd ::File.dirname(directory)
    code extract
    not_if { ::File.readable(file) }
  end
end


Bash - script to encrypt pdf files using pdfencrypt I wrote a small script to get around the issue when using pdfencrypt on the command line the password is always echoed as well as being saved to the 'history'.

#!/bin/bash

 export LC_ALL=de_DE.latin1

 while true; do
     echo "please select Pdf input file:"
     read -e -r infile
     if file --mime-type "$infile" | grep -q pdf$; then
         echo
         break
     else
         printf "\nis not a PDF file :-(\n\n"
     fi
 done

 echo "please type in Pdf output filename"
 echo "without extension (.pdf):"
 read -e -r outfile

 if [ -f "${outfile}.pdf" ]; then
     echo
     echo "File allready exists!";
     echo "please enter a new filename, or the same to override: "
     read -e -r outfile
 fi

 echo "please type in Password"
 echo "(only ASCII chracters recommended)"
 read -r -s

 pdfencrypt "$infile" -p "$READ" -o "${outfile}.pdf" \
  && echo "success, ${outfile}.pdf is encrypted :-)" \
  || echo "failed, did you use non ASCII characters for Password?"

exit

I had the problem with following non ASCII-Characters in the password:
§ ä ö ü etc. When I used any of those for encryption I got the following error:

  Encoding::CompatibilityError: incompatible character encodings: UTF-8 and ASCII-8BIT

I solved this by including export LC_ALL=de_DE.latin1 in the script. Though there still are some compatibility problems when using any of those characters, therefore I added a recommendation not to use non ASCII in the password.

(I know that PDF encryption itself is pretty low-level security but I sometimes like to add this level when sending files via e-mail etc...)

Usability

Typing filenames on standard input is extremely annoying.
It would be better if the script took the input filename as command line argument.
That way users can benefit from path completion in the shell.

Simplify

Instead of this:

 while true; do
     echo "please select Pdf input file:"
     read -e -r infile
     if file --mime-type "$infile" | grep -q pdf$; then
         echo
         break
     else
         printf "\nis not a PDF file :-(\n\n"
     fi
 done

I would eliminate the else statement, and also simplify the echo-ing:

 while true; do
     echo "please select Pdf input file:"
     read -e -r infile
     echo
     file --mime-type "$infile" | grep -q pdf$ && break
     echo "is not a PDF file :-("
     echo
 done

Redundant exit

The exit at the end of the script is redundant, I suggest to remove it.


Balanced Random Assignment in Python It has been a very long time since I've used Python.

What I'm looking for: I would like to create a 6-by-6 random matrix where each component is either 1 or 2, so that there are 18 ones and 18 twos.

Attempt:

import numpy
import random

array = numpy.zeros((6, 6))
while (array == 1).sum() != 18 or (array == 2).sum() != 18:
    for i in range(0, 6):
        for j in range(0, 6):
            array[i][j] = random.randint(1, 2)
print array

Sure, this works. But this seems to me to be extremely inefficient, as the code is literally having to insert new matrix values until the number of ones/twos is 18. How can I make this more efficient?

Just create a list of 18 ones and 18 twos, shuffle it, then reshape to 6x6:

from random import shuffle
from numpy import reshape

nums = [1]*18 + [2]*18
shuffle(nums)
arr = reshape(nums, (6, 6))

Produces (for example):

array([[1, 2, 2, 1, 2, 1],
       [2, 2, 2, 2, 2, 1],
       [2, 2, 2, 1, 1, 2],
       [1, 1, 1, 1, 2, 2],
       [2, 1, 1, 2, 1, 1],
       [1, 2, 1, 1, 1, 2]])


Choosing evenly distributed points from a million points on a sphere I have \$10^5\$ to \$10^6\$ points on a sphere, and want to choose some points from them which are as close as uniformly distributed as possible. For that reason, I do the following: at each step I add the point in the data which is farthest away from any point which I have already chosen. In order to do this, I find the closest distance from each point in the data to any of the points that are already chosen. Then I pick the point from the data which has the largest minimum distance to any of the points that are already chosen. The algorithm is the following:

Choose a random point from all the \$10^6\$ points, add it to the chosen set of points
For each of the \$10^6\$ points:

2.1 Find the distances to all the points which are chosen already

2.2 Find the min distance to a point which is chosen already

2.3 Find the point which is furthest away from any point which is chosen already

2.4 Add this point to the chosen points
Repeat 2 until I have enough points (say 1000 or 10000 or 100000).

data is \$10^6\$ by 3 array of points on a sphere, e.g.

[[ 0.26750522 -0.92342735  0.27517791]
 [-0.26753053  0.9228284  -0.27715548]
 [ 0.34058837  0.35873472  0.86908513]
 [-0.33563178 -0.35794416 -0.8713365 ]
 [-0.8945523  -0.15682272  0.41854847]
 [ 0.906739    0.15103321 -0.39371734]
 [ 0.49138659 -0.64830133 -0.581588  ]
 [-0.87922161  0.24492971 -0.40863039]
 [ 0.18062012  0.39852148 -0.89919798]
 [-0.49103509  0.65872966  0.57005243]
 [-0.55615839 -0.82248645 -0.11918007]
 [ 0.85802915 -0.25207255  0.44748789]
 [-0.16990087 -0.40390349  0.89888579]]

I have implemented this in Python:

import random
import numpy

N = 1000

starting_point_ind = random.SystemRandom().randint (1 , len(data))

points_from_data = numpy.array(numpy.zeros((N, 3)), dtype=float, order='C')
points_from_data[0] = data[starting_point_ind]

for i in range(1, N):
    distancesToClosestPoints = numpy.array(numpy.zeros(len(data)), dtype=float)

    for j, point in enumerate(data):
        distancesToEstablishedPoints = numpy.linalg.norm (point - points_from_data[0:i] , axis=1)

        distancesToClosestPoints[j] = distancesToEstablishedPoints.min()

    k = distancesToClosestPoints.argmax()

    points_from_data[i] = data[k]

This doesn't run very efficiently for large N, so I also tried to optimise it by doing

import random
from sklearn.metrics import pairwise_distances
import time

t0 = time.time()

N = 1000

starting_point_ind = random.SystemRandom().randint (1 , len(data))

points_from_data = numpy.array([data[starting_point_ind]])

for i in range(1, N):
    pairwise_distances_to_data = pairwise_distances (data, Y=points_from_data, metric='euclidean', n_jobs=-1)
    pairwise_distances_to_data = numpy.array(pairwise_distances_to_data)

    min_distances_to_data = numpy.amin(pairwise_distances_to_data, axis=1)

    k = min_distances_to_data.argmax()

    points_from_data = numpy.append(points_from_data, [data[k]], axis=0)

print(points_from_data)

But this still has the issue that the pairwise distances are computed each time, and has obvious memory issues for large N. I would like to hear suggestions how I can improve the performance of the algorithm.

Since we can assume all your points are on a sphere, there is a better way to compute distance than using the L2 Norm (Euclidean distance, which you're using).

If we transform your data points to Spherical coordinates, we could use a simple Manhattan distance on the theta and rho coordinates, which is less expensive to compute. Since you compute this distance about \$Nm\$ times (6 million points * 1000 sample points), I'm sure it would make a good performance difference.

Small note on the Spherical Coordinates :

Instead of having x,y,z coordinates, we use r, theta, rho, which correspond to :

r : the length of the vector
theta : angle on the XY plane
phi : angle of inclination starting from theta

So, to change data to spherical coordinates, we'd do the following (I assume data is arranged in a x,y,z manner). The conversion is very fast, but it could be further optimized if needed :

def convert_to_spherical(data):
    spherical_data = np.zeros_like(data)
    for i,d in enumerate(data):
        r = math.sqrt(d[0]**2 + d[1]**2 + d[2]**2)
        theta = math.acos(d[2] / r)
        phi = math.atan(d[1] / d[0])

        spherical_data[i] = np.array([r, theta, phi])

    return spherical_data

Then, you could use this instead of data and change the metric used by pairwise_distances to manhattan. You'll need to dig a little bit to understand why this works, but consider that since the parameter r is always the same, you can simply compute the difference between the angles.

Since my laptop is pretty bad, I couldn't generate data to sample 1000 points out of 1000000. These times were generated sampling 300 points from 100000 points, I started timing after generating the points but I included the coordinates transformation in the times :

Time using your optimized solution : 213.1844208240509s
Time using the spherical coordinates : 140.83173203468323s

Considering this pretty big time difference, I'm pretty sure performance would be much better on your data sample.

If afterwards, you need to get your coordinates back to the cartesian plane, the formula is pretty easy and you can find it online easily.

I've generated the points using thanks to this answer :

def sample_spherical(npoints, ndim=3):
    vec = numpy.random.randn(ndim, npoints)
    vec /= numpy.linalg.norm(vec, axis=0)
    return numpy.transpose(vec)


Extracting an IP address from a hosts file by hostname I am currently working with Vagrant for the first time, and therefore also working with Ruby for the first time. I have a specific hostname and IP address I would like set for my VM, both of which are stored in my hosts file. I would like to define the hostname in my Vagrantfile, and then have it automatically pull the matching IP address out of my hosts file for me. To do so, I have written the following code in my Vagrantfile:

Vagrant.configure("2") do |config|
  hostname = "vagrantvm"
  hostfile = "/etc/hosts"

  ip_address = File.foreach(hostfile).grep(/\b#{hostname}\b/) { |m| /^([0-9\.]*)\b.*/.match(m)[1] }.first

  config.vm.network :private_network, :ip => ip_address
  config.vm.hostname = hostname
  config.vm.provision :shell, :path => "bootstrap.sh"
end

Line 5 where I am assigning ip_address works, however I am not sure if it is the right way to do it. It seems a little ugly and long. I was thinking that maybe I should not put the code block passed to grep all on the same line. Or is it even necessary to have 2 separate regexes in there? Can it be done in 1 step?

It also seems strange to me that I need to add .first on the end in order to get the IP address string out of the array returned by grep. I feel like there is a better way to do this that I am missing.

You can probably get by with this:

ip = File.read(hostfile).match(/^\s*([\d\.]+)\s+\b#{hostname}\s*$/)[1]

A more "complete" way might be to do:

hosts = File.foreach(hostfile)
  .grep(/^\s*([\d\.:]+)/)
  .map { |line| line.strip.split(/\s+/).reverse }
  .to_h

Which gives you a { hostname => IP } hash for everything in the file (IPv6 included for good measure). I.e. you'd just do

ip_address = hosts[hostname]

to get the IP address.

For your current code:

  It seems a little ugly and long

It is a long line, but you can always break it up. You can chain off of the end keyword just as well as off of a }, so something like this would help readability:

File.foreach(hostfile).grep(/\b#{hostname}\b/) do |m|
  /^([0-9\.]*)\b.*/.match(m)[1]
end.first

In the 2nd approach above, I could've used grep to both match and map the lines, but I thought it nicer to separate the two.

  It also seems strange to me that I need to add .first on the end in order to get the IP address string out of the array returned by grep

grep is being called for each line, meaning it'll produce an array of matching lines. Hence why you need first. You're basically filtering and mapping every line in the file. After filtering there's (presumably!) only one line left, but it'll still be in an array.

You could use detect if you're only interested in the first matching line:

line = File.foreach(hostfile).detect { |line| line =~ %r[\b#{hostname}\b] }

That'll give you only the line containing the hostname you're looking for (or nil), which you can then parse further.


Convert sum from monthly to quarterly values I'm attempting to aggregate multiple columns of monthly data into quarterly chunks. Currently, I'm applying a rolling sum to the columns and then selecting only every third row to 'delete' or hide the rows.

I get the feeling that not only is there a far simpler approach, but there is a more Pythonic/Pandorable way of doing this.

import pandas as pd

data = pd.DataFrame({'parent_id': [1, 1, 1, 1, 1, 1, -99999, -99999, -99999],
                     'id': [123, 123, 123, 123, 123, 123, 123, 123, 123],
                     'data_1': [10, 20, 30, 40, 50, 60, 0, 0, 0],
                     'data_2': [10, 20, 30, 40, 50, 60, 0, 0, 0],
                     'period': [0, 1, 2, 3, 4, 5, 6, 7, 8],
                     'date': ['2017-06-30', '2017-07-31', '2017-08-31',
                              '2017-09-30', '2017-10-31', '2017-11-30',
                              '2017-12-31', '2018-01-31', '2018-02-28'],
                     'quarter': [0, 0, 0, 1, 1, 1, 2, 2, 2]})

def convert_to_quarterly(df, date):
    """Aggregates 3 months of data to a quarterly value."""
    columns = ['data_1', 'data_2']
    dates = pd.to_datetime(df['date'])
    quarter_end_dates = map(lambda offset: (date + pd.DateOffset(months=offset)).to_period('M').to_timestamp('M'), np.arange(0, 40, 3))
    df_grouped = df.groupby('id')
    #df[columns] = df_grouped[columns].apply(pd.rolling_sum, window=3, min_periods=1)
    return df.loc[df[dates.isin(quarter_end_dates)].index]

convert_to_quarterly(data, date=pd.to_datetime('2017-06-30'))

Before:

After:

You could use panda's resample to group your data into quarterly blocks. I think the key thing to note is that your dates start at the end of the month, so you need to set it to resample from the start of the month. The main problem is that the rest of your columns don't really aggregate well, but you can just update them from the original data by index.

def convert_to_quarterly(df):
    """Aggregates 3 months of data to a quarterly value."""
    df.date = pd.to_datetime(df['date'])
    df = df.set_index("date")
    agg_columns = ['data_1', 'data_2']
    extra_cols = [x for x in df.columns if x not in agg_columns]
    df_out = df.resample("QS-JUN")[agg_columns].sum()
    df_out.index = df_out.index + MonthEnd(1)
    df_out[extra_cols] = df[extra_cols]
    return df_out

data = convert_to_quarterly(data)

Since we resampled by month start, if you want the dates to be from the end of the month, you can use pandas.tseries.offsets MonthEnd to fix your dates.

Alternatively, you could keep your rollingsum method and just generate your quarter end dates by a daterange:

pd.date_range(data.date.min(),data.date.max(),freq="Q")

Also, converting the date column and setting it as index is not really the responsibility of a monthly_to_quarterly function, so you might want to consider separating the concerns and doing that outside of the function from a design perspective.


Python selenium object repository I was trying to create the selenium web element object repository in python. I have come up with three ways to do so. However, I am new to python so I am not sure which one is better than other.

Note: I am using Page object pattern for writing my tests, however just for understanding and simplicity I have put all the function in one file (class).

First: Using Decorator

def FindBy(by, value):
    def decorator(func):
        def wrapper(self):
            return self.driver.find_element(by, value)
        return wrapper
    return decorator

class TestGoogle(object):

    @FindBy(By.NAME, "q")
    def search_input(self): pass

    def test_search(self):
        self.driver = webdriver.Firefox()
        self.driver.get("https://google.com")
        self.search_input().send_keys("selenium python")

Second & Third: using Lambda and Method

class TestGoogle(object):

    def find_element(self, by, value):
        return self.driver.find_element(by, value)

    def search_input(self): return self.find_element(By.NAME, "q")

    search_input_1 = lambda self: self.find_element(By.NAME, "q")

    def test_search(self):
        self.driver = webdriver.Firefox()
        self.driver.get("https://google.com")
        self.search_input().send_keys("selenium python")
        self.search_input_1().clear()

If I were to use "find by" PageFactory-like notation in Python, I would go with the one proposed here - similar to your versions, but more concise and supports more features:

from pageobject_support import callable_find_by as find_by

class GoogleSearchPage(object):
    search_input = find_by(how=By.NAME, using='q')

Old Answer (in case there is anything useful there)

I would go further and follow the Page Object pattern, creating abstractions for the pages and elements you are working with, as demonstrated in the selenium documentation. This way you are isolating your locators from the logic of the test providing an "API" to work with a particular page.

Here is one way to do it - pay attention to how simple and concise the actual test has become:

import unittest

from selenium import webdriver
from selenium.webdriver.common.by import By
from selenium.webdriver.common.keys import Keys

class MainPageLocators(object):
    """A class for main page locators. All main page locators should come here"""
    SEARCH_INPUT = (By.NAME, 'q')

class BasePage(object):
    """Base class to initialize the base page that will be called from all pages"""

    def __init__(self, driver):
        self.driver = driver

class MainPage(BasePage):
    """Home page action methods come here."""

    def search(self, search_query):
        """Triggers the search"""
        search_input = self.driver.find_element(*MainPageLocators.SEARCH_INPUT)
        search_input.send_keys(search_query)
        search_input.send_keys(Keys.ENTER)

class TestGoogle(unittest.TestCase):
    def setUp(self):
        self.driver = webdriver.Firefox()
        self.driver.get("https://google.com")

    def tearDown(self):
        self.driver.close()

    def test_search(self):
        main_page = MainPage(self.driver)
        main_page.search("selenium python")

Note that you may also want a separate class for an element, but this should be good enough to get you started.

Also note that I'm using a unittest.TestCase as a base class for your test which, aside from bringing other benefits, allows to nicely start and close a browser via setUp and tearDown.


Add To Query Based Off Date Picker Or Drop Down List I have a page where the user can either input a start date and an end date into two date fields, OR the user can select 10 days, 20 days, 30 days from a drop down list on the page.  My php/html knowledge is very basic and I have the syntax set-up like such which works, but there is probably a more efficient way to code this syntax.  Speed is my #1 priority here (and this current syntax will have the page load in roughly 3 seconds, sweet!) - but is there optimization/better logic flow that can be used for htis?

Start Date:<input type="date" name="bgin" value="<?= $_POST['bgin'] ?>">
End Date:<input type="date" name="end" value="<?= $_POST['end'] ?>">

    Show last 10 - 20 - 30 Days
    <select name="daterange">
        <option value="">Select...</option>
        <option value="10">10 Days</option>
        <option value="20">20 Days</option>
        <option value="30">30 Days</option>
    </select>

<input type="submit" name="submit" value="Show Me">
<?php
    $bgin = $_POST['bgin'];
    $end   = $_POST['end'];
    if (isset($_POST['daterange'])) {
        $datecriteria = $_POST['daterange'];
    }
    //The user input a begin and end date so process based off those values
    if (isset($bgin) && isset($end)) {
        //Connect to mssql here
        $db                 = JDatabase::getInstance($option);
        $query              = $db->getquery(true);
        $query              = "Select statement blah blah where datefield >= '$bgin' and datefield <= '$end'";
    }
    if (isset($datecriteria)) {
        //Connect to mssql here
        $db                 = JDatabase::getInstance($option);
        $query              = $db->getquery(true);
        $query              = "Select statement blah blah
        if ($datecriteria = '10') {
            $query              = " where datefield >= DateAdd(day, -10, getdate()) And datefield <= getdate())";
        }
        if ($datecriteria = '20') {
            $query              = " where datefield >= DateAdd(day, -20, getdate()) And datefield <= getdate())";
        }
        if ($datecriteria = '30') {
            $query              = " where datefield >= DateAdd(day, -30, getdate()) And datefield <= getdate())";
        }
    }
    else {
        echo "Please check the selection criteria and process again."
    }
?>

I'm not familiar with Joomla's database library, so forgive me if some details below are suboptimal. But this is how it looks like you could start cleaning the script up.

// Query the dates.
$records = QueryDates();

// If we didn't get any dates...
if($records == null)
{
    echo "Please check the selection criteria and process again.";
}
else
{
    // Do something.
}

// Put your logic in a function that can be called wherever you need.
// You'll eventually want your PHP logic to be in a different file from
// your presentation markup.
function QueryDates()
{
    $bgin = $_POST['bgin'];
    $end  = $_POST['end'];

    if (isset($_POST['daterange']))
    {
        $datecriteria = $_POST['daterange'];
    }

    // Wrap our database interactions in a try-catch for safety.
    //
    // You might want to comment out the try-catch when first debugging.
    try
    {
        // Connect to the database.
        //
        // Is $option defined somewhere?
        $db = JDatabase::getInstance($option);

        // Get a new query object.
        $query = $db->getquery(true);

        // SELECT * FROM table_name ...
        $query->select('*');
        $query->from($db->quoteName('table_name'));

        // Get a quoted version of our datefield column.
        $datefield_name = $db->quoteName('datefield');

        if (isset($bgin) && isset($end))
        {
            $query->where("$datefield_name >= " . $db->quote($bgin), 'AND');
            $query->where("$datefield_name <= " . $db->quote($end));
        }
        elseif (isset($datecriteria))
        {
            // Make sure the data is well-formed.
            if ($datecriteria != 10
                && $datecriteria != 20
                && $datecriteria != 30)
            {
                return null;
            }

            // Calculate our range.
            $min_date = DateAdd(day, $datecriteria * -1, getdate());

            $query->where("$datefield_name >= " . $db->quote($min_date), 'AND');
            $query->where("$datefield_name <= " . $db->quote(getdate()));
        }
        else
        {
            return null;
        }

        // Execute the query.
        $db->setQuery($query);
        $results = $db->loadObjectList();

        return $results;
    }
    catch (Exception $e)
    {
        // Could return something more useful, based on what
        // is containe din $e.
        return null;
    }
}

You'll notice a few things here. Let's break them down:

I put your logic into a function. It's a good habit to get into, as
you start to learn more about PHP and web development. You'll start
to out-grow the mode of putting all your PHP code in the same file
as your HTML markup and will start to write functions you want to
call from multiple pages.
I changed your query from being a string to a series of function
calls. Joomla appears to utilize an abstraction library where you
build queries out of function calls against a query object. It also
appears to use quoteName for input sanitation.

That last part sounds iffy, to me. If Joomla has query parameters,
I'd recommend you use those, instead. But, as in the intro, I'm not
familiar with the Joomla db library so this is what their
documentation seems to suggest doing.
https://docs.joomla.org/Selecting_data_using_JDatabase

In either case, don't simply pass a POST parameter into the query. You'll be
opening up your entire database to anyone who knows basic SQL Injection
(including the droves of SQL Injection bots that will find your site).
I have the new function return null whenever something goes wrong. This
allows you to check for the bad output after the function call and handle it
there, however you need to. Again, if you were to call this function from
different pages, each might have its own way of dealing with the bad output.

The Joomla documentation doesn't say if loadObjectList() returns something
like null or false if the query fails. If it does return null, you might
need to change all my "return null;"s to be something like "return false;",
so you don't confuse the two outputs.
Finally, I set a try-catch block around your database interaction. This is
good practice in production, as it will avoid nasty errors from hitting your
users. But it may be troublesome while you're developing the code (as it
will prevent you from seeing when something goes wrong). I'd recommend
commenting out the try-catch until you do publish the site to your
production environment.

As for speed, most of those 3 seconds for loading is likely network flight time (though, I assume you're running this locally) and the interaction with your database. There's not a lot you can do to speed up the traffic of sending the query and receiving the data, outside of environmental changes (eg. getting a faster hard drive on the server).

The one thing you can do is make sure your database schema is structured to facilitate speedy queries. For instance, if you're constantly querying against this "datefield", you might consider putting an index on it.

Depending on the data being returned, you can also tweak the cache. For instance, if users are frequently querying the same data, ensure that the cache is actually storing that data (rather than saying it's too big and dumping it or having the cache outright turned off).

If you expect a bunch of users to be using the page, you might also want to put stricter limits on the range of dates that can be queried. Having someone try to dump your entire database could cause slowdowns for other users.

But a lot of this is up to details you haven't given us.


Clean up function in Javascript Event Listener I am trying to refactor a chess game I am currently creating so the code is more flexible and maintainable, while doing this, I came across an event listener which I have no idea on how to make "cleaner". Some of the variable names in the event listener are unintuitive but apart from that, I would like to know how to structure the event listener such that it uses multiple methods. I have previously asked a question on Code Review which is also about refactoring, and contains the code from which this function came from, here is the link: Object-Oriented JavaScript chess game. If you have any questions or clarifications needed about the code, feel free to ask.

  document.addEventListener('click', function(event){
        if(!hasClicked){
          if(event.clientX < 480 && event.clientY < 480 && board[Math.floor(event.clientY / 60)][Math.floor(event.clientX / 60)] != "vacant"){
            if(humanPlayer.indexOf(board[Math.floor(event.clientY / 60)][Math.floor(event.clientX / 60)]) != -1){
              canMove = true;
              isHighlightPossibleMoves = true;
              hasClicked = true;
              highlightPos = {x: Math.floor(event.clientX / 60), y: Math.floor(event.clientY / 60)};
              pieceMoves = processMoves({x: Math.floor(event.clientX / 60), y: Math.floor(event.clientY / 60)}, board);
            } else {
              hasClicked = true;
              highlightPos = {x: Math.floor(event.clientX / 60), y: Math.floor(event.clientY / 60)};
              canMove = false;
            }
          }
        } else {
          if(canMove){
            advancePosition = {x: Math.floor(event.clientX / 60), y: Math.floor(event.clientY / 60)};
            for(i = 0; i < pieceMoves.moves.length; i++){
              if(advancePosition.x == pieceMoves.moves[i].x && advancePosition.y == pieceMoves.moves[i].y){
                if(board[highlightPos.y][highlightPos.x] == blackKing || board[highlightPos.y][highlightPos.x] == whiteKing){
                  if(pieceMoves.moves[i].x - 2 == highlightPos.x || pieceMoves.moves[i].x + 2 == highlightPos.x){
                    isCastling = true;
                  } else {
                    isCastling = false;
                  }
                }
                if(isCastling){
                  board = chess.returnCastledBoard({x: highlightPos.x, y: highlightPos.y}, pieceMoves.moves[i]);
                  chess = new Chess(board);
                  isCastling = false;
                } else {
                  if(
                    board[highlightPos.y][highlightPos.x] == whiteKingSideCastle ||
                    board[highlightPos.y][highlightPos.x] == whiteQueenSideCastle ||
                    board[highlightPos.y][highlightPos.x] == blackKingSideCastle ||
                    board[highlightPos.y][highlightPos.x] == blackQueenSideCastle ||
                    board[highlightPos.y][highlightPos.x] == blackKing ||
                    board[highlightPos.y][highlightPos.x] == whiteKing
                  ){
                    board[highlightPos.y][highlightPos.x].hasClicked = true;
                  }
                  board = chess.updateBoard(highlightPos, advancePosition);
                  chess = new Chess(board);
                  break;
                }
              }
            }
          }
          hasClicked = false;
          canMove = false;
          highlightPos = undefined;
          pieceMoves = undefined;
          advancePosition = undefined;
        }
      });

````

Toward restructuring and optimization

The expressions Math.floor(event.clientX / 60) and Math.floor(event.clientY / 60) which represents element position are redundantly duplicated across a half of the entire posted function's content.The Extract function technique is reasonably applied and expressed with a separate function:

function getElPosition(e){
    return {x: Math.floor(e.clientX / 60),
            y: Math.floor(e.clientY / 60)};
}

hasClicked and highlightPos variables are assigned with same values in both exclusive branches of if/else conditional.Thus, the assignment statements are moved out as common ones.Furthermore, both highlightPos and advancePosition are essentially point to the target event element position returned by mentioned getElPosition function. Therefore, they could be just eliminated (see the full approach below).

The condition:

if (pieceMoves.moves[i].x - 2 == highlightPos.x || pieceMoves.moves[i].x + 2 == highlightPos.x) {
    isCastling = true;
} else {
    isCastling = false;
}

is just a verbose version of:

isCastling = (pieceMoves.moves[i].x - 2 == elPos.x || pieceMoves.moves[i].x + 2 == elPos.x);

Noisy condition:

if (
    board[highlightPos.y][highlightPos.x] == whiteKingSideCastle ||
    board[highlightPos.y][highlightPos.x] == whiteQueenSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackKingSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackQueenSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackKing ||
    board[highlightPos.y][highlightPos.x] == whiteKing
)

is replaced with flexible Array.includes feature:

if ([whiteKingSideCastle, whiteQueenSideCastle, blackKingSideCastle,
     blackQueenSideCastle, blackKing, whiteKing].includes(board[elPos.y][elPos.x]))

The can_move flag can be eliminated and replaced with check of pieceMoves.length.

board[highlightPos.y][highlightPos.x] indexing is repeated multiple times and worth to be extracted into a variable:

let boardItem = board[elPos.y][elPos.x];

See the full optimized approach:

function getElPosition(e){
    return {x: Math.floor(event.clientX / 60),
            y: Math.floor(event.clientY / 60)};
}

document.addEventListener('click', function (event) {
    let elPos = getElPosition(event);
    if (!hasClicked) {
        if (event.clientX < 480 && event.clientY < 480 && board[elPos.y][elPos.x] != "vacant") {
            if (humanPlayer.indexOf(board[elPos.y][elPos.x]) != -1) {
                isHighlightPossibleMoves = true;
                pieceMoves = processMoves(Object.assign({}, elPos), board);
            }
            hasClicked = true;
        }
    } else {
        if (pieceMoves && pieceMoves.length) {
            for (let i = 0, len = pieceMoves.moves.length; i < len; i++) {
                let boardItem = board[elPos.y][elPos.x];
                if (elPos.x == pieceMoves.moves[i].x && elPos.y == pieceMoves.moves[i].y) {
                    if (boardItem == blackKing || boardItem == whiteKing) {
                        isCastling = (pieceMoves.moves[i].x - 2 == elPos.x || pieceMoves.moves[i].x + 2 == elPos.x);
                    }
                    if (!isCastling) {
                        if ([whiteKingSideCastle, whiteQueenSideCastle, blackKingSideCastle,
                            blackQueenSideCastle, blackKing, whiteKing].includes(boardItem))
                        {
                            boardItem.hasClicked = true;
                        }
                        board = chess.updateBoard(elPos, elPos);
                        chess = new Chess(board);
                        break;
                    }
                    board = chess.returnCastledBoard({x: elPos.x, y: elPos.y}, pieceMoves.moves[i]);
                    chess = new Chess(board);
                    isCastling = false;
                }
            }
        }
        hasClicked = false;
        pieceMoves = undefined;
    }
});


How to improve my auto-reconnection Python script I need some feedback here. Due to a weird issue on my server system, the network card disconnects from the network regularly. I'll fix this at a later date. But I wanted to know if the below script can be improved.

It's comprised of two files. A python script (autoConnect.py), and a Bash script (checkOnline.sh).

autoConnect.py:

#!/usr/bin/python
from subprocess import call, Popen, PIPE, STDOUT
import time
cmd = './checkOnline.sh'
while True:
    p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True)
    if "0" in p.stdout.read():
        print time.ctime() + ": Offline"
        print "Attempting to reconnect..."
        print "Determining network profile..."
        cmdTwo = "wicd-cli -ySl | sed -n '2 p' | grep -i paws -c"
        pTwo = Popen(cmdTwo, shell=True, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True)
        if "1" in pTwo.stdout.read():
            print "Network profile is \"1\""
            defNum = 1
        else:
            print "Network profile is \"2\""
            defNum = 2
        print "Connecting to network..."
        p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True)
        while "0" in p.stdout.read():
            call("wicd-cli -yn " + defNum + " -c")
            p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, stderr=STDOUT, close_fds=True)
            time.sleep(3)
            if "0" in p.stdout.read():
                print "Failed to connect. Trying again..."
        print "Success, connected to network!"
    else:
        print time.ctime() + ": Online"
    time.sleep(5)

checkOnline.sh:

#!/bin/bash

host=google.com

(ping -w5 -c3 $host  > /dev/null 2>&1) && echo "1" || (echo "0" && exit 1)

The former is the main file to be run. checkOnline pings google.com 3 times for 5 seconds, if nothing is returned within 5 seconds, it will return 0. If it gets something, it returns 1 into stdout.

I wanted to know if this can be improved for what I want it to do (check if the network is online, search for networks, connect, repeat). As of right now, I'm away from my server's physical location and am unable to test it on the current system I'm using, but the script should be functional.

I'd move the contents of the script into a main function and then use the if __name__ == '__main__' boilerplate to start it.
I wouldn't use external tools like bash/sed/grep to do additional logic. I'd implement that logic in python.
I'd use the communicate method for POpen objects rather then reading the stdout attribute
Your POpens are all very similiar. I'd write a function that takes a command line executes it and returns the standard output
You check if you are connected in multiple places, write a am_connected() function which returns True or False and use it multiple times
I'd probably write a function for each external command you execute even if they are not repeated because it'll make your code cleaner.


Drag and drop selected array items in Javascript I've written a function that moves selected items from a list into a new position. I've found that in order to do this, I need to retain the original 'index structure' of the array. The way I've done this is, instead of splicing the array right away, I set the elements to null, then later traverse the list backwards and splice the null elements. I don't know if this is the best way of doing this, but I thought I would post it here to see if anyone else has an idea of how to better accomplish this.

https://jsfiddle.net/g407pjbk/1/

function drag_and_drop(list, before_index){
    const selected = [];

    for(let index in list){
        const item = list[index];

        if(item.selected){
            selected.push(item);

            // Set elements to null in order to retain indecies
            list[index] = null;
        }
    }

    // Insert selected items
    list.splice(before_index, 0, ...selected);

    // Backwards traverse and splice remaining null elements
    for(let i=list.length-1;i>=0;i--){
        if(list[i] == null)
            list.splice(i, 1);
    }

    return list;
}

So you don't need two loops here. You can just use one loop to

First, get the selected items
Remove the selected items from the actual array. So you can later put them in the index you want.

So to do that, first, you have to retrieve the indices where the selected obj is there. Then spice it. IN the loop

for (let index in list) {
    const item = list[index];

    if (item.selected) {
        selected.push(item);
        list.spice(index,1)
    }
}

So you don't have to run another loop to remove the null values. You can now simply just put the values using another spice. That saves you a loop.

You shouldn't be using for ... in for the reasons you have got in RoToRa's answer.

My approach: I prefer using just one filter. In that, you can do the same thing.

function drag_and_drop(list, before_index) {
  const selected = list.filter((obj,i) => obj.selected ? list.splice(i,1) : obj.selected)
  list.splice(before_index - selected.length , 0, ...selected)
  return list
}

So here, first we check if the object is selected and if it is then we'll just splice it off and if we didn't got a selected item then we can just return the selected item in the selected loop.

So now you have inside selected, the values which have item.selected as true and your list don't have them now because you did splice already in the filter, so you can just splice them with another splice.

Now the index is not anymore before_index it's two values more, because we took out two objects. So to balance that, we will do before_index - selected.length because that's the number of indexes we are going to drop.


Listing movies for torrenting Can I get some help on shortening the amount of code I have in this program?

import requests
import gzip
import sys
import urllib
from tabulate import tabulate
from bs4 import BeautifulSoup
from io import BytesIO

# firstly it will display a list of movies, the user will then type in the torrent they want to download

# download file
def get_file(href, file_name):
    try:
        req = requests.get('https://examplewebsite.com' + href)
    except Exception as e:
        print (e)

    response = req.text
    soup = BeautifulSoup(response)
    download = soup.find('a', {'class': 'siteButton giantButton    verifTorrentButton'})

    file_name = file_name + '.torrent'

    # this encodes the response information in a file

    response = urllib.urlopen(download.get('href'))
    buffer = BytesIO(response.read())

    gz = gzip.GzipFile(fileobj=buffer)

    with open(file_name,'wb') as f:
        f.truncate()
        f.write(gz.read())

def select_torrent():
    torrent = raw_input('>>')
    if torrent =='q' or torrent =='Q':
        sys.exit(0)
    else:
        return torrent
def test(text):
    if text=='q' or text=='Q':
        sys.exit(0)

# generates a list of movies
def find_movies(pages):
    total_movie_list = {}
    total_amount_of_movies = []
    total_movie_number = 0

    for i in xrange(pages):
        try:
            req = requests.get('https://example.website/movies/'+ str(i))
        except Exception as e:
            print (e)
        else:

            response = req.text
            soup = BeautifulSoup(response)

            #find elements in html file
            movies = soup.findAll('a', {'class': 'cellMainLink'})
            seed = soup.findAll('td',{'class':'green center'})
            leech = soup.findAll('td',{'class':'red lasttd center'})

            #put seed and leech data into list
            seed_list = [number.string for number in seed]
            leech_list = [number.string for number in leech]

            # generate dictionary of movies with their movie name and seed/leech data
            movie_temp = {i+total_movie_number: (movies[i].get('href'), movies[i].string,seed_list[i],leech_list[i]) for i in range(len(movies))}
            #put data into a table

            table = [[key,value[1],value[2],value[3]]for key,value in movie_temp.items()]
            #add onto current number of movies
            total_amount_of_movies += table
            #update movie list in order to download them
            total_movie_list.update(movie_temp)

            # add on to the last movie  number
            current_number_of_movies = len(movies)
            total_movie_number +=current_number_of_movies

    print(tabulate(total_amount_of_movies,headers=['number','name','seeder','leecher']))

    return total_movie_list

if __name__ == '__main__':
    #find movies, get input from user, then download movie torrent
    pages = raw_input('how many pages do you want(press q to quit)?:')
    test(pages)
    movie_list = find_movies(int(pages))
    print('\ndownload a file by typing the number(press q to quit)\n')
    torrent  = select_torrent()
    test(torrent)

    get_file(movie_list[int(torrent)][0], movie_list[int(torrent)][1])

Shortening

  Can I get some help on shortening the amount of code I have in this program?

Shortening shouldn't be a primary concern. But ok let's go.

In get_file,
if an error occurs while downloading the page,
you print the exception, and then execution continues,
and the program will crash on the next line that tries to access the downloaded response.
You could just as well remove the try-except,
the result will be actually better,
because that way the crash will show the real problem (download error).

The same argument goes for the try-except in find_movies too.

You use two libraries to download data: requests and urllib.
You only need either of those, so you can get rid of one import.

file_name = file_name + '.torrent' can be shortened to file_name += '.torrent'

Instead of this:

torrent = select_torrent()
test(torrent)

You can delete the select_torrent function and write this:

torrent = raw_input('>>')
test(torrent)

Making it better

Follow PEP8, the Python coding style guide.

test is a very poor name for what it does.

Some validation for the selected torrent would be good.


Handling an invalid SQL query exception This is my DAO object that takes the Cypher query as a string, and returns a JSON representation of the graph it returns.

Because it's using a Spring JdbcTemplate to access the database, the jdbc query will return any errors wrapped in a DataAccessException.

What I'm wanting to do here, is detect whether the error being thrown is due to an invalid query (in which case I want to return a message to the user saying so), or for now, any other error, in which case I'll file it as 'An unexpected error occurred'.

public String runQuery(String q) throws JsonGenerationException, JsonMappingException, UncategorizedSQLException, DataAccessException
 {
    Graph g = null;
            try {
                g = jdbcTemplate.query(q, new Neo4jGraphResultSetExtractor());
            } catch (DataAccessException e) {

                if (e instanceof UncategorizedSQLException)
                    {
                    //Invalid cypher query
                    throw (UncategorizedSQLException)e;
                    }

                else throw e;
            }

    json = g.toJson(); //throws JsonGenerationException, JsonMappingException
    return json;
}

When I call the DAO from my Business Layer:

public String submitCypherQuery(String query) {

    try {
        return neoConn.runQuery(query);
    } catch (ClassNotFoundException e) {

        return handleUnexpectedError(e);

    } catch (JsonGenerationException e) {
        return handleUnexpectedError(e);
    } catch (JsonMappingException e) {
        return handleUnexpectedError(e);
    } catch (IOException e) {
        return handleUnexpectedError(e);
    }
    catch (UncategorizedSQLException e)
    {
        return new ErrorMessage(errorInvalidQuery).toJson();
    }
    catch (DataAccessException e)
    {
        return handleUnexpectedError(e);
    }

}

Is this an OK way of doing this?

One question I have is that both UncategorizedSQLException, DataAccessException are unchecked exceptions, meaning that even though the DAO has declared them as being thrown - the compiler is fine with calling these methods without being surrounded in a try/catch.

This isn't especially useful - this is the exact case I want to force the developers to handle!

If you want to make the compiler spit out errors when the exceptions aren't caught, make your classes that derive from Exception, but not RuntimeException. Note that unless you have control over the source from whence they came, since you can't change the definition of classes, you have to create your own custom ones.

You've got a bunch of random newlines that just make your code look ugly. I'd suggest removing all of them in the code you've posted.

Rather than having separate catch blocks for each, you can put them all into
one. The general form is this:

catch (ExceptionTypeA | ExceptionTypeB name) { /* ... */ }

Also, the standard in Java is to put the opening brace on the same line as the statement it belongs to, so

catch (UncategorizedSQLException e)
{
    return new ErrorMessage(errorInvalidQuery).toJson();
}

should really be

catch (UncategorizedSQLException e) {
    return new ErrorMessage(errorInvalidQuery).toJson();
}

        } catch (DataAccessException e) {

            if (e instanceof UncategorizedSQLException)
                {
                //Invalid cypher query
                throw (UncategorizedSQLException)e;
                }

            else throw e;
        }

Ignoring the wacky indentation and unconventional brackets, there are two tips I wanna give:

catch statements are done in order, using (what's basically) an instanceof check that stops after the first match. This means that if you put a superclass before a subclass, that subclass will never get hit -- so just put the subclass first:

catch (UncatergorizedSQLException e) {
    throw e;
}
catch (DataAccessException e) {
     throw e;
}

Of course, you can sidestep that whole issue by getting rid of the try and declaring that your method throws those two exceptions -- since you're doing nothing but rethrowing, it's simpler and easier. Basically, since you've already declared that you're throwing, just remove the try and its associated catches.

Follow that, and your first method (with formatting) becomes just this:

public String runQuery(String q) throws JsonGenerationException, JsonMappingException,
                                        UncategorizedSQLException, DataAccessException {
    Graph g = jdbcTemplate.query(q, new Neo4jGraphResultSetExtractor());
    json = g.toJson(); //throws JsonGenerationException, JsonMappingException
    return json;
}

And your second becomes (changing the whitespace to follow the majority of the catches):

public String submitCypherQuery(String query) {
    try {
        return neoConn.runQuery(query);
    } catch (UncategorizedSQLException e) {
        return new ErrorMessage(errorInvalidQuery).toJson();
    } catch (DataAccessException | ClassNotFoundException | JsonGenerationException |
             IOException | JsonMappingException e) {
        return handleUnexpectedError(e);
    }
}


YUIDoc syntax for optionally static constructor In JavaScript, I wrote a class that can be optionally instantiated with the new keyword, but can also be called statically as well. What is the best way to demonstrate this behavior in comment syntax? I was thinking something like this:

// localize constant
var PI = Math.PI;

/**
 * Complex constructor
 * optionally instantiate with `new`
 *
 * @class Complex
 * @constructor
 * @static
 * @param r {Number} real part
 * @param i {Number} imaginary part
 * @param m {Number} optional, magnitude
 * @param t {Number} optional, argument
 * @return {Object} complex number
 */

function Complex(r, i, m, t) {
    if (!(this instanceof Complex)) {
        return new Complex(r, i, m, t);
    }

    if (typeof m === 'number') {
        this.m = Math.abs(m);
    } else {
        this.m = Math.sqrt(r * r + i * i);
    }

    if (typeof t === 'number') {
        this.t = (m >= 0 ? t : t + PI);
    } else {
        this.t = Math.atan2(i, r);
    }

    // limit argument between (-pi,pi]
    this.t = PI - (PI * 3 - (PI - (PI * 3 - this.t) % (PI * 2))) % (PI * 2);
    this.r = r;
    this.i = i;
}

Naming
As Bhathiya pointed out, you should be using actual, descriptive variable names rather than just a single character.
A descriptive variable name means everything, as it increases readability and understanding 10-fold, even if it means typing a few more characters each time you write the variable name.
And, if you really, really don't want to type out a full variable name each time, most text editors or IDEs come with auto-complete. This could help you type less.

I think that the best way to express that the class can be instantiated in comment form would be to write something like this:
@instantiate: (new) Complex(r, i, m, t);

Then, in your code, I recommend adding a comment to these lines:
if (!(this instanceof Complex)) {
    return new Complex(r, i, m, t);
}

That says that those lines allow for the object to be instantiated with new.
However, do you really need both options? Is there something in the full program that you are using that blocks you from using new? I can't think of any case where you would be blocked by using new, I think you should always use it because it is then very clear that Complex is a class.


Proper handling of exceptions in MVP In my DAL currently I'm handling exceptions as follows. Is it in an acceptable level?

Note : I'm using MVP in this Winforms application.

    public bool InsertAccount(IBankAccount ba)
    {
        string selectStatement = @"IF NOT EXISTS (SELECT ac_no FROM BankAccount WHERE ac_no=@ac_no) BEGIN INSERT INTO BankAccount (ac_no, emp_id, ac_name, bank_name, ac_type," +
                                  "ent_date, ent_by, remarks, file_no, status) VALUES (@ac_no, @emp_id, @ac_name, @bank_name, @ac_type, @ent_date, @ent_by, @remarks, @file_no, @status) END";

        using (SqlConnection sqlConnection = new SqlConnection(db.ConnectionString))
        {
            using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
            {
                try
                {
                    sqlConnection.Open();
                    sqlCommand.Parameters.Add("@ac_no", SqlDbType.Char).Value = ba.AccountNumber;
                    sqlCommand.Parameters.Add("@emp_id", SqlDbType.Int).Value = ba.EmployeeID;
                    sqlCommand.Parameters.Add("@ac_name", SqlDbType.VarChar).Value = ba.AccountName;
                    sqlCommand.Parameters.Add("@bank_name", SqlDbType.VarChar).Value = ba.BankName;
                    sqlCommand.Parameters.Add("@ac_type", SqlDbType.VarChar).Value = ba.AccountType;
                    sqlCommand.Parameters.Add("@ent_date", SqlDbType.DateTime).Value = ba.EnteredDate;
                    sqlCommand.Parameters.Add("@ent_by", SqlDbType.VarChar).Value = "ABCD";
                    sqlCommand.Parameters.Add("@remarks", SqlDbType.VarChar).Value = ba.Remarks;
                    sqlCommand.Parameters.Add("@file_no", SqlDbType.Int).Value = ba.FileNo;
                    sqlCommand.Parameters.Add("@status", SqlDbType.Bit).Value = ba.Active;

                    sqlCommand.ExecuteNonQuery();
                    return true;
                }
                catch (Exception e) { MessageBox.Show(("Error: " + e.Message)); }
                if (sqlConnection.State == System.Data.ConnectionState.Open) sqlConnection.Close();
                return false;
            }

        }
    }

Short Answer:
Having a call to MessageBox.Show() in any DAL code is A Bad Thing (TM).

Longer Answer:
In an ideal world, your DAL would not know anything about your presentation layer - not even that it is a Windows/Winforms application.  It really should know very little (if anything) about any code that invokes it.

I would have the above routine return nothing and simply let any exception propagate back up the call stack to the point where is can be dealt with.

If this is in the code that directly called it, then wrapped the call in a try/catch. There should be no need to check a return value on this function.

//...
try {
    InsertAccount(ba);
    // continue with rest of processing
}
catch (Exception ex){
    // deal with exception
}

Even Longer Answer:

A Matter Of Personal Preference (AMOPP), having MessageBox.Show() any where in one's code except for a service dedicated to showing messages is bad.  Any code that needs to show a message should invoke a service to show the message.

This means that

We can more easily unit test the code.  A request to show a message can be mocked and tested. Actually showing a MessageBox blocks the test.
We can manage how the messages should be shown in a single place. Say that we want all the messages to show as toast notifications instead of message boxes; we need only change in one place.